Skip to content

Commit

Permalink
Fix /dev/tty check
Browse files Browse the repository at this point in the history
Previously this was bugged, incorrectly using -z so that the check never
passed, and $EDITOR was always run without having the TTY injected as
an input. This causes problems with command-line editors like Vim, which
need an interactive input (and output) to be usable.

Meanwhile, attempting to inject /dev/tty as the input in all cases
causes problems in CI, because there is no TTY available in GitHub
Actions, and presumably in other scripted environments.

Fixing this with -e does not work, as /dev/tty exists but unopenably in
many environments. Instead, we use printf in a subshell to test whether
/dev/tty is actually usable, and then only use it if it is.
  • Loading branch information
pimterry committed Sep 7, 2021
1 parent bd5a895 commit 6315323
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
4 changes: 2 additions & 2 deletions notes
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ open_note() {

note_path=$( get_full_note_path "$note_path" )

if [[ -z "/dev/tty" ]]; then
$EDITOR "$note_path" < /dev/tty
if bash -c "printf '' >/dev/tty" >/dev/null 2>/dev/null; then
$EDITOR "$note_path" </dev/tty
else
$EDITOR "$note_path"
fi
Expand Down
46 changes: 45 additions & 1 deletion test/test-open.bats
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ notes="./notes"
run $notes open

assert_success
assert_output "Opening $HOME/notes"
assert_output "Opening $HOME/notes"
}

@test "Opens the configured notes directory if set" {
Expand Down Expand Up @@ -132,3 +132,47 @@ notes="./notes"
assert_success
assert_output "Editing $NOTES_DIRECTORY/test-note.txt"
}

@test "Gives a TTY input to EDITOR when available" {
if ! bash -c "printf '' >/dev/tty" >/dev/null 2>/dev/null; then
# If no working TTY is available, we can't test this, skip it
skip
fi

function checkTtyEdit() {
if [ -t 0 ]; then
echo "Editing $* with TTY"
else
echo "No TTY to edit $*"
fi
}
export -f checkTtyEdit
export EDITOR="checkTtyEdit"

touch $NOTES_DIRECTORY/note.md

run bash -c "$notes find | $notes open test-note"

assert_success
assert_output "Editing $NOTES_DIRECTORY/note.md with TTY"
}

@test "Opens editor with no TTY input if none is available" {
function checkTtyEdit() {
if [ -t 0 ]; then
echo "Editing $* with TTY"
else
echo "No TTY to edit $*"
fi
}
export -f checkTtyEdit
export EDITOR="checkTtyEdit"

touch $NOTES_DIRECTORY/note.md

# Setsid removes the controlling TTY:
run setsid bash -c "$notes find | $notes open test-note"

assert_success
assert_output "No TTY to edit $NOTES_DIRECTORY/note.md"
}

0 comments on commit 6315323

Please sign in to comment.