Re: [PATCH 7/8] Provide a build time default-editor setting
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:47:39
Subsystem:
kernel build + files below scripts/ (unless maintained elsewhere), the rest · Maintainers:
Nathan Chancellor, Nicolas Schier, Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH 7/8] Provide a build time default-editor setting · Jonathan Nieder <hidden>
- 2016-06-15 · Re: [PATCH 7/8] Provide a build time default-editor setting · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH 7/8] Provide a build time default-editor setting · Jonathan Nieder <hidden>
- 2016-06-15 · Re: [PATCH 7/8] Provide a build time default-editor setting · Junio C Hamano <hidden>
- 2016-06-15 · [PATCH 7/8] Provide a build time default-editor setting · Jonathan Nieder <hidden>
Junio C Hamano wrote:
Jonathan Nieder [off-list ref] writes:
quoted
+test_expect_success 'does editor have a simple name (no slashes, etc)?' ' + + editor=$(TERM=vt100 git var GIT_EDITOR) && + test -n "$editor" && + simple=t && + case "$editor" in + */* | core_editor | [A-Z]*)Hmm, what are the latter two cases designed to catch?
Both are meant to allow the test to work without too many changes. The core_editor case is a little pedantic, since it is unlikely to actually come up in practice. With a default editor of core_editor, the initial loop will overwrite e-core_editor.sh (to be used through the core.editor configuration) with e-core_editor.sh (to be used as a fallback editor) before renaming it to core_editor. I missed some other cases: If editor is .git, e-GIT_EDITOR.sh, etc, the mv will still misbehave. The [A-Z]* test is to avoid changing the loop around line 86: | unset EDITOR VISUAL GIT_EDITOR | git config --unset-all core.editor | for i in "$editor" EDITOR VISUAL core_editor GIT_EDITOR | do | echo "Edited by $i" >expect | case "$i" in | core_editor) | git config core.editor ./e-core_editor.sh | ;; | [A-Z]*) | eval "$i=./e-$i.sh" | export $i | ;; | esac | test_expect_success "Using $i (override)" ' | git --exec-path=. commit --amend && | git show -s --pretty=oneline | | sed -e "s/^[0-9a-f]* //" >actual && | diff actual expect | ' | done which I do not think is worth making more complicated. Maybe it would be better to just check for an editor consisting only of alphabetical characters. Perhaps something like the following: -- %< -- From: Jonathan Nieder <redacted> Subject: [PATCH] Provide a build time default-editor setting 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 <redacted> Signed-off-by: Ben Walton <redacted> Signed-off-by: Jonathan Nieder <redacted> --- Makefile | 17 +++++++++++++++++ editor.c | 6 +++++- t/t7005-editor.sh | 27 ++++++++++++++++++++------- 3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..625866c 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 4f98b72..2aac807 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 b647957..13c37de 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh@@ -4,7 +4,22 @@ 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' ' + + editor=$(TERM=vt100 git var GIT_EDITOR) && + test -n "$editor" + +' + +if ! test -z "$(printf '%s\n' "$editor" | sed '/^[a-z]*$/d')" +then + say 'skipping editor tests, default editor name too complicated' + test_done +fi + +for i in GIT_EDITOR core_editor EDITOR VISUAL "$editor" do cat >e-$i.sh <<-EOF #!$SHELL_PATH
@@ -12,15 +27,13 @@ do EOF chmod +x e-$i.sh done -unset vi -mv e-vi.sh vi -unset EDITOR VISUAL GIT_EDITOR +mv "e-$editor.sh" "$editor" test_expect_success setup ' msg="Hand edited" && echo "$msg" >expect && - git add vi && + git add "$editor" && test_tick && git commit -m "$msg" && git show -s --pretty=oneline |
@@ -44,7 +57,7 @@ test_expect_success 'dumb should error out when falling back on vi' ' TERM=vt100 export TERM -for i in vi EDITOR VISUAL core_editor GIT_EDITOR +for i in "$editor" EDITOR VISUAL core_editor GIT_EDITOR do echo "Edited by $i" >expect unset EDITOR VISUAL GIT_EDITOR
@@ -68,7 +81,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 "$editor" EDITOR VISUAL core_editor GIT_EDITOR do echo "Edited by $i" >expect case "$i" in
--
1.6.5.2