Re: [PATCH/RFC 2/2] Provide a build time default-editor setting

Provide a DEFAULT_EDITOR knob to allow the fallback editor (to
use instead of vi if VISUAL, EDITOR, and GIT_EDITOR are unset) to
be set at build time according to a system’s policy.  For
example, on Debian systems, the default editor should be the
'editor' command.

The contrib/fast-import/git-p4 script still uses vi, since it is
not modified by the Makefile currently, and making it require
build-time modification would create too much trouble for people
deploying that script.

This change makes t7005-editor into a mess.  Any ideas for fixing
this?

Signed-off-by: Jonathan Nieder <redacted>
---
 Makefile                  |   10 ++++++++++
 editor.c                  |    2 +-
 git-add--interactive.perl |    3 ++-
 git-sh-setup.sh           |    6 ++++--
 git-svn.perl              |    5 +++--
 t/Makefile                |    2 ++
 t/t7005-editor.sh         |   29 ++++++++++++++++++++++-------
 7 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index fc1a461..fae8647 100644
--- a/Makefile
+++ b/Makefile

@@ -203,6 +203,9 @@ all::

 #
 # Define DEFAULT_PAGER to the path of a sensible pager (defaults to "less") if
 # you want to use something different.
+#
+# Define DEFAULT_EDITOR to a sensible editor command (defaults to "vi") if you
+# want to use something different.

 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
       @$(SHELL_PATH) ./GIT-VERSION-GEN

@@ -1301,6 +1304,11 @@ ifndef DEFAULT_PAGER

       DEFAULT_PAGER = less
 endif
 BASIC_CFLAGS += -DDEFAULT_PAGER='"$(DEFAULT_PAGER)"'
+ifndef DEFAULT_EDITOR
+       DEFAULT_EDITOR = vi
+endif
+export DEFAULT_EDITOR
+BASIC_CFLAGS += -DDEFAULT_EDITOR='"$(DEFAULT_EDITOR)"'

 ifdef USE_NED_ALLOCATOR
       COMPAT_CFLAGS += -DUSE_NED_ALLOCATOR -DOVERRIDE_STRDUP -DNDEBUG -DREPLACE_SYSTEM_ALLOCATOR -Icompat/nedmalloc

@@ -1435,6 +1443,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh

           -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
           -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
           -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+           -e 's|DEFAULT_EDITOR:=vi|DEFAULT_EDITOR:=$(DEFAULT_EDITOR)|' \
           -e $(BROKEN_PATH_FIX) \
           $@.sh >$@+ && \
       chmod +x $@+ && \

@@ -1459,6 +1468,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl

           -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
           -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
           -e 's/@@DEFAULT_PAGER@@/$(DEFAULT_PAGER)/g' \
+           -e 's/@@DEFAULT_EDITOR@@/$(DEFAULT_EDITOR)/g' \
           $@.perl >$@+ && \
       chmod +x $@+ && \
       mv $@+ $@

diff --git a/editor.c b/editor.c
index 4d469d0..93b8cbb 100644
--- a/editor.c
+++ b/editor.c

@@ -19,7 +19,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en

               return error("Terminal is dumb but no VISUAL nor EDITOR defined.");

       if (!editor)
-               editor = "vi";
+               editor = DEFAULT_EDITOR;

       if (strcmp(editor, ":")) {
               size_t len = strlen(editor);

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 69aeaf0..c3d932c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -1,6 +1,7 @@

 #!/usr/bin/perl -w

 use strict;
+use constant DEFAULT_EDITOR => '@@DEFAULT_EDITOR@@';
 use Git;

 binmode(STDOUT, ":raw");

@@ -988,7 +989,7 @@ EOF

       close $fh;

       my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
-               || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+               || $ENV{VISUAL} || $ENV{EDITOR} || DEFAULT_EDITOR;
       system('sh', '-c', $editor.' "$@"', $editor, $hunkfile);

       if ($? != 0) {

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c41c2f7..d053d56 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh

@@ -99,19 +99,21 @@ set_reflog_action() {

 }

 git_editor() {
+       : "${DEFAULT_EDITOR:=vi}"
       : "${GIT_EDITOR:=$(git config core.editor)}"
       : "${GIT_EDITOR:=${VISUAL:-${EDITOR}}}"
       case "$GIT_EDITOR,$TERM" in
       ,dumb)
               echo >&2 "No editor specified in GIT_EDITOR, core.editor, VISUAL,"
-               echo >&2 "or EDITOR. Tried to fall back to vi but terminal is dumb."
+               echo >&2 "or EDITOR. Tried to fall back to $DEFAULT_EDITOR" \
+                       "but terminal is dumb."
               echo >&2 "Please set one of these variables to an appropriate"
               echo >&2 "editor or run $0 with options that will not cause an"
               echo >&2 "editor to be invoked (e.g., -m or -F for git-commit)."
               exit 1
               ;;
       esac
-       eval "${GIT_EDITOR:=vi}" '"$@"'
+       eval "${GIT_EDITOR:=$DEFAULT_EDITOR}" '"$@"'
 }

 is_bare_repository () {

diff --git a/git-svn.perl b/git-svn.perl
index c270b23..b98d378 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -3,12 +3,13 @@

 # License: GPL v2 or later
 use warnings;
 use strict;
-use vars qw/   $AUTHOR $VERSION $DEFAULT_PAGER
+use vars qw/   $AUTHOR $VERSION $DEFAULT_PAGER $DEFAULT_EDITOR
               $sha1 $sha1_short $_revision $_repository
               $_q $_authors $_authors_prog %users/;
 $AUTHOR = 'Eric Wong [off-list ref]';
 $VERSION = '@@GIT_VERSION@@';
 $DEFAULT_PAGER = '@@DEFAULT_PAGER@@';
+$DEFAULT_EDITOR = '@@DEFAULT_EDITOR@@';

 # From which subdir have we been invoked?
 my $cmd_dir_prefix = eval {

@@ -1322,7 +1323,7 @@ sub get_commit_entry {

       close $log_fh or croak $!;

       if ($_edit || ($type eq 'tree')) {
-               my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
+               my $editor = $ENV{VISUAL} || $ENV{EDITOR} || $DEFAULT_EDITOR;
               # TODO: strip out spaces, comments, like git-commit.sh
               system($editor, $commit_editmsg);
       }

diff --git a/t/Makefile b/t/Makefile
index bd09390..9174bbb 100644
--- a/t/Makefile
+++ b/t/Makefile

@@ -9,6 +9,8 @@

 SHELL_PATH ?= $(SHELL)
 TAR ?= $(TAR)
 RM ?= rm -f
+DEFAULT_EDITOR ?= vi
+export DEFAULT_EDITOR

 # Shell quote;
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))

diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b647957..2b76f72 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh

@@ -4,7 +4,18 @@ test_description='GIT_EDITOR, core.editor, and stuff'

 . ./test-lib.sh

-for i in GIT_EDITOR core_editor EDITOR VISUAL vi
+: ${DEFAULT_EDITOR=vi}
+
+unset EDITOR VISUAL GIT_EDITOR
+
+case "$DEFAULT_EDITOR" in
+*/* | [A-Z]*)
+       DEFAULT_EDITOR=
+       ;;
+esac
+
+for i in GIT_EDITOR core_editor EDITOR VISUAL \
+       ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"}
 do
       cat >e-$i.sh <<-EOF
       #!$SHELL_PATH

@@ -12,15 +23,17 @@ do

       EOF
       chmod +x e-$i.sh
 done
-unset vi
-mv e-vi.sh vi
-unset EDITOR VISUAL GIT_EDITOR
+
+if test -n "$DEFAULT_EDITOR"
+then
+       mv "e-$DEFAULT_EDITOR.sh" "$DEFAULT_EDITOR"
+fi

 test_expect_success setup '

       msg="Hand edited" &&
       echo "$msg" >expect &&
-       git add vi &&
+       git add "e-VISUAL.sh" &&
       test_tick &&
       git commit -m "$msg" &&
       git show -s --pretty=oneline |

@@ -44,7 +57,8 @@ 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 ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
+       EDITOR VISUAL core_editor GIT_EDITOR
 do
       echo "Edited by $i" >expect
       unset EDITOR VISUAL GIT_EDITOR

@@ -68,7 +82,8 @@ 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 ${DEFAULT_EDITOR:+"$DEFAULT_EDITOR"} \
+       EDITOR VISUAL core_editor GIT_EDITOR
 do
       echo "Edited by $i" >expect
       case "$i" in
--
1.6.5.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html