Skip to content

Commit 6315323

Browse files
committed
Fix /dev/tty check
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.
1 parent bd5a895 commit 6315323

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

notes

+2-2
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ open_note() {
204204

205205
note_path=$( get_full_note_path "$note_path" )
206206

207-
if [[ -z "/dev/tty" ]]; then
208-
$EDITOR "$note_path" < /dev/tty
207+
if bash -c "printf '' >/dev/tty" >/dev/null 2>/dev/null; then
208+
$EDITOR "$note_path" </dev/tty
209209
else
210210
$EDITOR "$note_path"
211211
fi

test/test-open.bats

+45-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ notes="./notes"
2424
run $notes open
2525

2626
assert_success
27-
assert_output "Opening $HOME/notes"
27+
assert_output "Opening $HOME/notes"
2828
}
2929

3030
@test "Opens the configured notes directory if set" {
@@ -132,3 +132,47 @@ notes="./notes"
132132
assert_success
133133
assert_output "Editing $NOTES_DIRECTORY/test-note.txt"
134134
}
135+
136+
@test "Gives a TTY input to EDITOR when available" {
137+
if ! bash -c "printf '' >/dev/tty" >/dev/null 2>/dev/null; then
138+
# If no working TTY is available, we can't test this, skip it
139+
skip
140+
fi
141+
142+
function checkTtyEdit() {
143+
if [ -t 0 ]; then
144+
echo "Editing $* with TTY"
145+
else
146+
echo "No TTY to edit $*"
147+
fi
148+
}
149+
export -f checkTtyEdit
150+
export EDITOR="checkTtyEdit"
151+
152+
touch $NOTES_DIRECTORY/note.md
153+
154+
run bash -c "$notes find | $notes open test-note"
155+
156+
assert_success
157+
assert_output "Editing $NOTES_DIRECTORY/note.md with TTY"
158+
}
159+
160+
@test "Opens editor with no TTY input if none is available" {
161+
function checkTtyEdit() {
162+
if [ -t 0 ]; then
163+
echo "Editing $* with TTY"
164+
else
165+
echo "No TTY to edit $*"
166+
fi
167+
}
168+
export -f checkTtyEdit
169+
export EDITOR="checkTtyEdit"
170+
171+
touch $NOTES_DIRECTORY/note.md
172+
173+
# Setsid removes the controlling TTY:
174+
run setsid bash -c "$notes find | $notes open test-note"
175+
176+
assert_success
177+
assert_output "No TTY to edit $NOTES_DIRECTORY/note.md"
178+
}

0 commit comments

Comments
 (0)