From eab58f1e8e5ef86b5075ce6dfcd6d3f1b3b888b3 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 30 Oct 2009 20:24:04 -0500 Subject: Handle more shell metacharacters in editor names Pass the editor name to the shell if it contains any susv3 shell special character (globs, redirections, variable substitutions, escapes, etc). This way, the meaning of some characters will not meaninglessly change when others are added, and git commands implemented in C and in shell scripts will interpret editor names in the same way. This does not make the GIT_EDITOR setting any more expressive, since one could always use single quotes to force the editor to be passed to the shell. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- editor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'editor.c') diff --git a/editor.c b/editor.c index 4d469d076b..941c0b2ab6 100644 --- a/editor.c +++ b/editor.c @@ -28,7 +28,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en const char *args[6]; struct strbuf arg0 = STRBUF_INIT; - if (strcspn(editor, "$ \t'") != len) { + if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) { /* there are specials */ strbuf_addf(&arg0, "%s \"$@\"", editor); args[i++] = "sh"; -- cgit v1.3-5-g9baa From d33738d7d3ed42a956c74d0125eb2b3abda451b7 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Nov 2009 17:56:07 -0600 Subject: Do not use VISUAL editor on dumb terminals 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 Signed-off-by: Junio C Hamano --- 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(-) (limited to 'editor.c') diff --git a/editor.c b/editor.c index 941c0b2ab6..3f1375114c 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 b647957d75..a95fe19d8c 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 d2de57679f..a603f6d21a 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 f2ca536472..ec3336aba5 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 () { -- cgit v1.3-5-g9baa From 44fcb4977cbae67f4698306ccfe982420ceebcbf Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 11 Nov 2009 18:01:27 -0600 Subject: Teach git var about GIT_EDITOR Expose the command used by launch_editor() for scripts to use. This should allow one to avoid searching for a proper editor separately in each command. git_editor(void) uses the logic to decide which editor to use that used to live in launch_editor(). The function returns NULL if there is no suitable editor; the caller is expected to issue an error message when appropriate. launch_editor() uses git_editor() and gives the error message the same way as before when EDITOR is not set. "git var GIT_EDITOR" gives the editor name, or an error message when there is no appropriate one. "git var -l" gives GIT_EDITOR=name only if there is an appropriate editor. Originally-submitted-by: Johannes Sixt Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/git-var.txt | 8 ++++++++ cache.h | 1 + editor.c | 14 ++++++++++++-- var.c | 16 +++++++++++++++- 4 files changed, 36 insertions(+), 3 deletions(-) (limited to 'editor.c') diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt index e2f4c0901b..89e4b4fa0c 100644 --- a/Documentation/git-var.txt +++ b/Documentation/git-var.txt @@ -36,6 +36,14 @@ GIT_AUTHOR_IDENT:: GIT_COMMITTER_IDENT:: The person who put a piece of code into git. +GIT_EDITOR:: + Text editor for use by git commands. The value is meant to be + interpreted by the shell when it is used. Examples: `~/bin/vi`, + `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe" + --nofork`. The order of preference is the `$GIT_EDITOR` + environment variable, then `core.editor` configuration, then + `$VISUAL`, then `$EDITOR`, and then finally 'vi'. + Diagnostics ----------- You don't exist. Go away!:: diff --git a/cache.h b/cache.h index 96840c7af7..311cfe1219 100644 --- a/cache.h +++ b/cache.h @@ -750,6 +750,7 @@ extern const char *git_author_info(int); extern const char *git_committer_info(int); extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int); extern const char *fmt_name(const char *name, const char *email); +extern const char *git_editor(void); struct checkout { const char *base_dir; diff --git a/editor.c b/editor.c index 3f1375114c..70618f1066 100644 --- a/editor.c +++ b/editor.c @@ -2,7 +2,7 @@ #include "strbuf.h" #include "run-command.h" -int launch_editor(const char *path, struct strbuf *buffer, const char *const *env) +const char *git_editor(void) { const char *editor = getenv("GIT_EDITOR"); const char *terminal = getenv("TERM"); @@ -16,11 +16,21 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en editor = getenv("EDITOR"); if (!editor && terminal_is_dumb) - return error("terminal is dumb, but EDITOR unset"); + return NULL; if (!editor) editor = "vi"; + return editor; +} + +int launch_editor(const char *path, struct strbuf *buffer, const char *const *env) +{ + const char *editor = git_editor(); + + if (!editor) + return error("Terminal is dumb, but EDITOR unset"); + if (strcmp(editor, ":")) { size_t len = strlen(editor); int i = 0; diff --git a/var.c b/var.c index dacbaab259..b502487e53 100644 --- a/var.c +++ b/var.c @@ -8,6 +8,16 @@ static const char var_usage[] = "git var [-l | ]"; +static const char *editor(int flag) +{ + const char *pgm = git_editor(); + + if (!pgm && flag & IDENT_ERROR_ON_NO_NAME) + die("Terminal is dumb, but EDITOR unset"); + + return pgm; +} + struct git_var { const char *name; const char *(*read)(int); @@ -15,14 +25,18 @@ struct git_var { static struct git_var git_vars[] = { { "GIT_COMMITTER_IDENT", git_committer_info }, { "GIT_AUTHOR_IDENT", git_author_info }, + { "GIT_EDITOR", editor }, { "", NULL }, }; static void list_vars(void) { struct git_var *ptr; + const char *val; + for (ptr = git_vars; ptr->read; ptr++) - printf("%s=%s\n", ptr->name, ptr->read(0)); + if ((val = ptr->read(0))) + printf("%s=%s\n", ptr->name, val); } static const char *read_var(const char *var) -- cgit v1.3-5-g9baa From 8f4b576ad14943a7b14cb8937eb321b7bfd91ee7 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 30 Oct 2009 20:44:41 -0500 Subject: Provide a build time default-editor setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provide a DEFAULT_EDITOR knob to allow setting the fallback editor to use instead of vi (when VISUAL, EDITOR, and GIT_EDITOR are unset). The value can be set at build time according to a system’s policy. For example, on Debian systems, the default editor should be the 'editor' command. Signed-off-by: Jonathan Nieder Signed-off-by: Ben Walton Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 17 +++++++++++++++++ editor.c | 6 +++++- t/t7005-editor.sh | 37 +++++++++++++++++++++++++------------ 3 files changed, 47 insertions(+), 13 deletions(-) (limited to 'editor.c') diff --git a/Makefile b/Makefile index 268aede566..625866c737 100644 --- a/Makefile +++ b/Makefile @@ -200,6 +200,14 @@ all:: # memory allocators with the nedmalloc allocator written by Niall Douglas. # # Define NO_REGEX if you have no or inferior regex support in your C library. +# +# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you +# want to use something different. The value will be interpreted by the shell +# if necessary when it is used. Examples: +# +# DEFAULT_EDITOR='~/bin/vi', +# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR', +# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork' GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1363,6 +1371,15 @@ BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \ $(COMPAT_CFLAGS) LIB_OBJS += $(COMPAT_OBJS) +# Quote for C + +ifdef DEFAULT_EDITOR +DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))" +DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ)) + +BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)' +endif + ALL_CFLAGS += $(BASIC_CFLAGS) ALL_LDFLAGS += $(BASIC_LDFLAGS) diff --git a/editor.c b/editor.c index 70618f1066..615f5754d6 100644 --- a/editor.c +++ b/editor.c @@ -2,6 +2,10 @@ #include "strbuf.h" #include "run-command.h" +#ifndef DEFAULT_EDITOR +#define DEFAULT_EDITOR "vi" +#endif + const char *git_editor(void) { const char *editor = getenv("GIT_EDITOR"); @@ -19,7 +23,7 @@ const char *git_editor(void) return NULL; if (!editor) - editor = "vi"; + editor = DEFAULT_EDITOR; return editor; } diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh index a95fe19d8c..5257f4d261 100755 --- a/t/t7005-editor.sh +++ b/t/t7005-editor.sh @@ -4,7 +4,21 @@ test_description='GIT_EDITOR, core.editor, and stuff' . ./test-lib.sh -for i in GIT_EDITOR core_editor EDITOR VISUAL vi +unset EDITOR VISUAL GIT_EDITOR + +test_expect_success 'determine default editor' ' + + vi=$(TERM=vt100 git var GIT_EDITOR) && + test -n "$vi" + +' + +if ! expr "$vi" : '^[a-z]*$' >/dev/null +then + vi= +fi + +for i in GIT_EDITOR core_editor EDITOR VISUAL $vi do cat >e-$i.sh <<-EOF #!$SHELL_PATH @@ -12,19 +26,18 @@ do EOF chmod +x e-$i.sh done -unset vi -mv e-vi.sh vi -unset EDITOR VISUAL GIT_EDITOR + +if ! test -z "$vi" +then + mv e-$vi.sh $vi +fi test_expect_success setup ' - msg="Hand edited" && + msg="Hand-edited" && + test_commit "$msg" && echo "$msg" >expect && - git add vi && - test_tick && - git commit -m "$msg" && - git show -s --pretty=oneline | - sed -e "s/^[0-9a-f]* //" >actual && + git show -s --format=%s > actual && diff actual expect ' @@ -54,7 +67,7 @@ test_expect_success 'dumb should prefer EDITOR to VISUAL' ' TERM=vt100 export TERM -for i in vi EDITOR VISUAL core_editor GIT_EDITOR +for i in $vi EDITOR VISUAL core_editor GIT_EDITOR do echo "Edited by $i" >expect unset EDITOR VISUAL GIT_EDITOR @@ -78,7 +91,7 @@ done unset EDITOR VISUAL GIT_EDITOR git config --unset-all core.editor -for i in vi EDITOR VISUAL core_editor GIT_EDITOR +for i in $vi EDITOR VISUAL core_editor GIT_EDITOR do echo "Edited by $i" >expect case "$i" in -- cgit v1.3-5-g9baa