Merge branch 'jn/editor-pager'
authorJunio C Hamano <gitster@pobox.com>
Sat, 21 Nov 2009 07:48:52 +0000 (23:48 -0800)
committerJunio C Hamano <gitster@pobox.com>
Sat, 21 Nov 2009 07:48:52 +0000 (23:48 -0800)
* jn/editor-pager:
Provide a build time default-pager setting
Provide a build time default-editor setting
am -i, git-svn: use "git var GIT_PAGER"
add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
Teach git var about GIT_PAGER
Teach git var about GIT_EDITOR
Suppress warnings from "git var -l"
Do not use VISUAL editor on dumb terminals
Handle more shell metacharacters in editor names

19 files changed:
Documentation/config.txt
Documentation/git-commit.txt
Documentation/git-send-email.txt
Documentation/git-var.txt
Makefile
cache.h
contrib/fast-import/git-p4
editor.c
git-add--interactive.perl
git-am.sh
git-send-email.perl
git-sh-setup.sh
git-svn.perl
ident.c
pager.c
t/t7005-editor.sh
t/t7501-commit.sh
t/test-lib.sh
var.c
index cb73d7571f25deffb061441577ce1ae531fdf5d9..c9b8db5cf701fa39fc518dd196c4acfb57cac0c4 100644 (file)
@@ -387,9 +387,7 @@ core.editor::
        Commands such as `commit` and `tag` that lets you edit
        messages by launching an editor uses the value of this
        variable when it is set, and the environment variable
-       `GIT_EDITOR` is not set.  The order of preference is
-       `GIT_EDITOR` environment, `core.editor`, `VISUAL` and
-       `EDITOR` environment variables and then finally `vi`.
+       `GIT_EDITOR` is not set.  See linkgit:git-var[1].
 
 core.pager::
        The command that git will use to paginate output.  Can
index 0578a40d841348b5ae484e1fe15ce637ecc4b830..3ea80c820fb404d7b00436f9820fe8701f056ebf 100644 (file)
@@ -323,7 +323,7 @@ ENVIRONMENT AND CONFIGURATION VARIABLES
 The editor used to edit the commit log message will be chosen from the
 GIT_EDITOR environment variable, the core.editor configuration variable, the
 VISUAL environment variable, or the EDITOR environment variable (in that
-order).
+order).  See linkgit:git-var[1] for details.
 
 HOOKS
 -----
index 767cf4d4bdcdfbb6e76b3680b0e89af2186d632a..c85d7f4385b91ce296c0247e3c9b93059abff19d 100644 (file)
@@ -60,8 +60,8 @@ The --bcc option must be repeated for each user you want on the bcc list.
 The --cc option must be repeated for each user you want on the cc list.
 
 --compose::
-       Use $GIT_EDITOR, core.editor, $VISUAL, or $EDITOR to edit an
-       introductory message for the patch series.
+       Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
+       to edit an introductory message for the patch series.
 +
 When '--compose' is used, git send-email will use the From, Subject, and
 In-Reply-To headers specified in the message. If the body of the message
index e2f4c0901bcb4bcc5361e400ff40d70062c77ae6..ef6aa81872eb9e67eaf8183cfd5359d97e6c772b 100644 (file)
@@ -36,6 +36,20 @@ 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'.
+
+GIT_PAGER::
+    Text viewer for use by git commands (e.g., 'less').  The value
+    is meant to be interpreted by the shell.  The order of preference
+    is the `$GIT_PAGER` environment variable, then `core.pager`
+    configuration, then `$PAGER`, and then finally 'less'.
+
 Diagnostics
 -----------
 You don't exist. Go away!::
index 4e596dcd0bfe01c2d325a02ac5d7b39b67df2912..f8906618e89bfa0e06fccea8280bae2a4e53d84f 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -204,6 +204,18 @@ 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_PAGER to a sensible pager command (defaults to "less") if
+# you want to use something different.  The value will be interpreted by the
+# shell at runtime when it is used.
+#
+# 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
@@ -1374,6 +1386,22 @@ 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
+
+ifdef DEFAULT_PAGER
+DEFAULT_PAGER_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_PAGER)))"
+DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ))
+
+BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'
+endif
+
 ALL_CFLAGS += $(BASIC_CFLAGS)
 ALL_LDFLAGS += $(BASIC_LDFLAGS)
 
diff --git a/cache.h b/cache.h
index 71a731dbc988a8a22d0a2e1f8f3a5223f4c5d67a..a9047491e650dc0f6267b578fabeb33cafd20885 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -751,6 +751,8 @@ 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);
+extern const char *git_pager(void);
 
 struct checkout {
        const char *base_dir;
index e710219ca52af3c90c28bb90273d062a26543864..48059d0aa74fe5f4c14bd74f599d449335ef8519 100755 (executable)
@@ -729,13 +729,10 @@ class P4Submit(Command):
             tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
             tmpFile.close()
             mtime = os.stat(fileName).st_mtime
-            defaultEditor = "vi"
-            if platform.system() == "Windows":
-                defaultEditor = "notepad"
             if os.environ.has_key("P4EDITOR"):
                 editor = os.environ.get("P4EDITOR")
             else:
-                editor = os.environ.get("EDITOR", defaultEditor);
+                editor = read_pipe("git var GIT_EDITOR")
             system(editor + " " + fileName)
 
             response = "y"
index 4d469d076bcd58df3af16d98adc9120e34765944..615f5754d66ba7ea611a4837b1ff802f9f9de598 100644 (file)
--- a/editor.c
+++ b/editor.c
@@ -2,24 +2,38 @@
 #include "strbuf.h"
 #include "run-command.h"
 
-int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+#ifndef DEFAULT_EDITOR
+#define DEFAULT_EDITOR "vi"
+#endif
+
+const char *git_editor(void)
 {
-       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 NULL;
+
+       if (!editor)
+               editor = DEFAULT_EDITOR;
+
+       return editor;
+}
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+{
+       const char *editor = git_editor();
 
        if (!editor)
-               editor = "vi";
+               return error("Terminal is dumb, but EDITOR unset");
 
        if (strcmp(editor, ":")) {
                size_t len = strlen(editor);
@@ -28,7 +42,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";
index 8ce1ec92c2b04e1271b3ee1a41a2ac60d2f8aad4..f813ffdaa1526aa68b4ed8e7d4d04b48e270496a 100755 (executable)
@@ -990,8 +990,7 @@ sub edit_hunk_manually {
 EOF
        close $fh;
 
-       my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
-               || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+       chomp(my $editor = run_cmd_pipe(qw(git var GIT_EDITOR)));
        system('sh', '-c', $editor.' "$@"', $editor, $hunkfile);
 
        if ($? != 0) {
index c132f50da5f3416e6177dd7010e6db3660bdb073..26494877f44f10be8f105932ad9845ac56e49c05 100755 (executable)
--- a/git-am.sh
+++ b/git-am.sh
@@ -649,7 +649,10 @@ do
                [eE]*) git_editor "$dotest/final-commit"
                       action=again ;;
                [vV]*) action=again
-                      LESS=-S ${PAGER:-less} "$dotest/patch" ;;
+                      : ${GIT_PAGER=$(git var GIT_PAGER)}
+                      : ${LESS=-FRSX}
+                      export LESS
+                      $GIT_PAGER "$dotest/patch" ;;
                *)     action=again ;;
                esac
            done
index a0279de687064c762a4ee24dfe2ed1922afab53a..4f5da4ecf253d79dd558e3c962c194fe3da46296 100755 (executable)
@@ -162,7 +162,8 @@ sub format_2822_time {
 
 # Handle interactive edition of files.
 my $multiedit;
-my $editor = $ENV{GIT_EDITOR} || Git::config(@repo, "core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+my $editor = Git::command_oneline('var', 'GIT_EDITOR');
+
 sub do_edit {
        if (defined($multiedit) && !$multiedit) {
                map {
index c41c2f7439724adc3dd13b92c86b25d254fc23b8..99cceeb858dcd2c83a040b0dd3117744e177e316 100755 (executable)
@@ -99,19 +99,12 @@ set_reflog_action() {
 }
 
 git_editor() {
-       : "${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 "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}" '"$@"'
+       if test -z "${GIT_EDITOR:+set}"
+       then
+               GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
+       fi
+
+       eval "$GIT_EDITOR" '"$@"'
 }
 
 is_bare_repository () {
index ab0a8dd0990ac030db046efdcdae74978a2d8c46..2746895ae620a37c4b84419caf221db8f476e197 100755 (executable)
@@ -1332,9 +1332,8 @@ sub get_commit_entry {
        close $log_fh or croak $!;
 
        if ($_edit || ($type eq 'tree')) {
-               my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
-               # TODO: strip out spaces, comments, like git-commit.sh
-               system($editor, $commit_editmsg);
+               chomp(my $editor = command_oneline(qw(var GIT_EDITOR)));
+               system('sh', '-c', $editor.' "$@"', $editor, $commit_editmsg);
        }
        rename $commit_editmsg, $commit_msg or croak $!;
        {
@@ -5219,10 +5218,8 @@ sub git_svn_log_cmd {
 
 # adapted from pager.c
 sub config_pager {
-       $pager ||= $ENV{GIT_PAGER} || $ENV{PAGER};
-       if (!defined $pager) {
-               $pager = 'less';
-       } elsif (length $pager == 0 || $pager eq 'cat') {
+       chomp(my $pager = command_oneline(qw(var GIT_PAGER)));
+       if ($pager eq 'cat') {
                $pager = undef;
        }
        $ENV{GIT_PAGER_IN_USE} = defined($pager);
diff --git a/ident.c b/ident.c
index 99f1c85ea5f5c83247f7affd3d801ff5c14393a6..26409b2a1b191765706265c2aa6d2ae163ba5bab 100644 (file)
--- a/ident.c
+++ b/ident.c
@@ -205,7 +205,7 @@ const char *fmt_ident(const char *name, const char *email,
                if ((warn_on_no_name || error_on_no_name) &&
                    name == git_default_name && env_hint) {
                        fprintf(stderr, env_hint, au_env, co_env);
-                       env_hint = NULL; /* warn only once, for "git var -l" */
+                       env_hint = NULL; /* warn only once */
                }
                if (error_on_no_name)
                        die("empty ident %s <%s> not allowed", name, email);
diff --git a/pager.c b/pager.c
index 86facec7b417b26a7dbd9b3c1338149fcecd5588..92c03f654abd0333bd0dd48b4aebf9ae42ac4de5 100644 (file)
--- a/pager.c
+++ b/pager.c
@@ -2,6 +2,10 @@
 #include "run-command.h"
 #include "sigchain.h"
 
+#ifndef DEFAULT_PAGER
+#define DEFAULT_PAGER "less"
+#endif
+
 /*
  * This is split up from the rest of git so that we can do
  * something different on Windows.
@@ -44,12 +48,14 @@ static void wait_for_pager_signal(int signo)
        raise(signo);
 }
 
-void setup_pager(void)
+const char *git_pager(void)
 {
-       const char *pager = getenv("GIT_PAGER");
+       const char *pager;
 
        if (!isatty(1))
-               return;
+               return NULL;
+
+       pager = getenv("GIT_PAGER");
        if (!pager) {
                if (!pager_program)
                        git_config(git_default_config, NULL);
@@ -58,8 +64,18 @@ void setup_pager(void)
        if (!pager)
                pager = getenv("PAGER");
        if (!pager)
-               pager = "less";
+               pager = DEFAULT_PAGER;
        else if (!*pager || !strcmp(pager, "cat"))
+               pager = NULL;
+
+       return pager;
+}
+
+void setup_pager(void)
+{
+       const char *pager = git_pager();
+
+       if (!pager)
                return;
 
        spawned_pager = 1; /* means we are emitting to terminal */
index b647957d75fa0d0ce4d88c7c3c7243f31af38b4a..5257f4d261c2060b881d2649034232f76f4ed9b7 100755 (executable)
@@ -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
 
 '
@@ -42,9 +55,19 @@ 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
+for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
 do
        echo "Edited by $i" >expect
        unset EDITOR VISUAL GIT_EDITOR
@@ -68,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
index d2de57679f9ee347a5e77e751ae7767f9980f84e..a603f6d21a61df197256ea91bfb38c1f4e45c5e8 100755 (executable)
@@ -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
index f2ca5364722e9c85a23bdfdcf1e24122fd5e3a0f..ec3336aba5a65a468bc6ce71f33a9cca76dbfe0f 100644 (file)
@@ -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 () {
diff --git a/var.c b/var.c
index 125c0d1676abae6de29de14061d1a24fd842712d..d9892f85ce954883427ac4f61d9de9d591c0827c 100644 (file)
--- a/var.c
+++ b/var.c
@@ -8,6 +8,25 @@
 
 static const char var_usage[] = "git var [-l | <variable>]";
 
+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;
+}
+
+static const char *pager(int flag)
+{
+       const char *pgm = git_pager();
+
+       if (!pgm)
+               pgm = "cat";
+       return pgm;
+}
+
 struct git_var {
        const char *name;
        const char *(*read)(int);
@@ -15,14 +34,19 @@ struct git_var {
 static struct git_var git_vars[] = {
        { "GIT_COMMITTER_IDENT", git_committer_info },
        { "GIT_AUTHOR_IDENT",   git_author_info },
+       { "GIT_EDITOR", editor },
+       { "GIT_PAGER", pager },
        { "", 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(IDENT_WARN_ON_NO_NAME));
+               if ((val = ptr->read(0)))
+                       printf("%s=%s\n", ptr->name, val);
 }
 
 static const char *read_var(const char *var)