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

[David Roundy <roundyd@physics.oregonstate.edu>]

Any chance this will be exported as plumbing? I know it's pretty
high-level, but it'd be handy to have be able to write `git editor
$FILENAME` and just have it do the right thing.  This would also mean
that the perl scripts below could be simplified.

Same goes for pager, of course...

David

On Thu, Oct 29, 2009 at 3:50 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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 <jrnieder@gmail.com>
> ---
>  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 <normalperson@yhbt.net>';
>  $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
>



-- 
David Roundy