Refuse to use $VISUAL and fall back to $EDITOR if TERM is unset or set to "dumb". Traditionally, VISUAL is set to a screen editor and EDITOR to a line-based editor, which should be more useful in that situation. vim, for example, is happy to assume a terminal supports ANSI sequences even if TERM is dumb (e.g., when running from a text editor like Acme). git already refuses to fall back to vi on a dumb terminal if GIT_EDITOR, core.editor, VISUAL, and EDITOR are unset, but without this patch, that check is suppressed by VISUAL=vi. Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> --- Fixes broken sign-off, patch unchanged. I am personally most interested in this for usage from a text editor, but vim does not set TERM=dumb like it probably ought to. A more realistic everyday example might be "ssh user@domain git commit". editor.c | 12 ++++++------ t/t7005-editor.sh | 10 ++++++++++ t/t7501-commit.sh | 8 ++++---- t/test-lib.sh | 8 ++++---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/editor.c b/editor.c index 941c0b2..3f13751 100644 --- a/editor.c +++ b/editor.c @@ -4,19 +4,19 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *env) { - const char *editor, *terminal; + const char *editor = getenv("GIT_EDITOR"); + const char *terminal = getenv("TERM"); + int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb"); - editor = getenv("GIT_EDITOR"); if (!editor && editor_program) editor = editor_program; - if (!editor) + if (!editor && !terminal_is_dumb) editor = getenv("VISUAL"); if (!editor) editor = getenv("EDITOR"); - terminal = getenv("TERM"); - if (!editor && (!terminal || !strcmp(terminal, "dumb"))) - return error("Terminal is dumb but no VISUAL nor EDITOR defined."); + if (!editor && terminal_is_dumb) + return error("terminal is dumb, but EDITOR unset"); if (!editor) editor = "vi"; diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh index b647957..a95fe19 100755 --- a/t/t7005-editor.sh +++ b/t/t7005-editor.sh @@ -42,6 +42,16 @@ test_expect_success 'dumb should error out when falling back on vi' ' fi ' +test_expect_success 'dumb should prefer EDITOR to VISUAL' ' + + EDITOR=./e-EDITOR.sh && + VISUAL=./e-VISUAL.sh && + export EDITOR VISUAL && + git commit --amend && + test "$(git show -s --format=%s)" = "Edited by EDITOR" + +' + TERM=vt100 export TERM for i in vi EDITOR VISUAL core_editor GIT_EDITOR diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index d2de576..a603f6d 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -86,7 +86,7 @@ chmod 755 editor test_expect_success \ "amend commit" \ - "VISUAL=./editor git commit --amend" + "EDITOR=./editor git commit --amend" test_expect_success \ "passing -m and -F" \ @@ -107,7 +107,7 @@ chmod 755 editor test_expect_success \ "editing message from other commit" \ "echo 'hula hula' >file && \ - VISUAL=./editor git commit -c HEAD^ -a" + EDITOR=./editor git commit -c HEAD^ -a" test_expect_success \ "message from stdin" \ @@ -141,10 +141,10 @@ EOF test_expect_success \ 'editor not invoked if -F is given' ' echo "moo" >file && - VISUAL=./editor git commit -a -F msg && + EDITOR=./editor git commit -a -F msg && git show -s --pretty=format:"%s" | grep -q good && echo "quack" >file && - echo "Another good message." | VISUAL=./editor git commit -a -F - && + echo "Another good message." | EDITOR=./editor git commit -a -F - && git show -s --pretty=format:"%s" | grep -q good ' # We could just check the head sha1, but checking each commit makes it diff --git a/t/test-lib.sh b/t/test-lib.sh index f2ca536..ec3336a 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -30,7 +30,7 @@ TZ=UTC TERM=dumb export LANG LC_ALL PAGER TERM TZ EDITOR=: -VISUAL=: +unset VISUAL unset GIT_EDITOR unset AUTHOR_DATE unset AUTHOR_EMAIL @@ -58,7 +58,7 @@ GIT_MERGE_VERBOSITY=5 export GIT_MERGE_VERBOSITY export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME -export EDITOR VISUAL +export EDITOR GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u} # Protect ourselves from common misconfiguration to export @@ -207,8 +207,8 @@ trap 'die' EXIT test_set_editor () { FAKE_EDITOR="$1" export FAKE_EDITOR - VISUAL='"$FAKE_EDITOR"' - export VISUAL + EDITOR='"$FAKE_EDITOR"' + export EDITOR } test_tick () { -- 1.6.5.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html