commit -v: strip diffs and submodule shortlogs from the commit message

When using the '-v' option of "git commit" the diff added to the commit
message temporarily for editing is stripped off after the user exited the
editor by searching for "\ndiff --git " and truncating the commmit message
there if it is found.

But this approach has two problems:

- when the commit message itself contains a line starting with
  "diff --git" it will be truncated there prematurely; and

- when the "diff.submodule" setting is set to "log", the diff may
  start with "Submodule <hash1>..<hash2>", which will be left in
  the commit message while it shouldn't.

Fix that by introducing a special scissor separator line starting with the
comment character ('#' or the core.commentChar config if set) followed by
two lines describing what it is for. The scissor line - which will not be
translated - is used to reliably detect the start of the diff so it can be
chopped off from the commit message, no matter what the user enters there.

Turn a known test failure fixed by this change into a successful test;
also add one for a diff starting with a submodule log and another one for
proper handling of the comment char.

Reported-by: Ari Pollak <ari@debian.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605cf5c2e6ef5efb37518c9c215bf8895f97..fedb45af8c65d28c3b43b3b74362e589d81a95ee 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
        struct strbuf sb = STRBUF_INIT;
        struct strbuf author_ident = STRBUF_INIT;
        const char *index_file, *reflog_msg;
-       char *nl, *p;
+       char *nl;
        unsigned char sha1[20];
        struct ref_lock *ref_lock;
        struct commit_list *parents = NULL, **pptr = &parents;
@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
        }
 
        /* Truncate the message just before the diff, if any. */
-       if (verbose) {
-               p = strstr(sb.buf, "\ndiff --git ");
-               if (p != NULL)
-                       strbuf_setlen(&sb, p - sb.buf + 1);
-       }
+       if (verbose)
+               wt_status_truncate_message_at_cut_line(&sb);
 
        if (cleanup_mode != CLEANUP_NONE)
                stripspace(&sb, cleanup_mode == CLEANUP_ALL);

diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index da5bd3b5a585667dad97481df27049f0aa7415eb..2ddf28c984de99cb17884f76d68a708076f57e12 100755 (executable)
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' '
        check_message diff
 '
 
-test_expect_failure 'diff in message is retained with -v' '
+test_expect_success 'diff in message is retained with -v' '
        git commit --amend -F diff -v &&
        check_message diff
 '
 
+test_expect_success 'submodule log is stripped out too with -v' '
+       git config diff.submodule log &&
+       git submodule add ./. sub &&
+       git commit -m "sub added" &&
+       (
+               cd sub &&
+               echo "more" >>file &&
+               git commit -a -m "submodule commit"
+       ) &&
+       (
+               GIT_EDITOR=cat &&
+               export GIT_EDITOR &&
+               test_must_fail git commit -a -v 2>err
+       ) &&
+       test_i18ngrep "Aborting commit due to empty commit message." err
+'
+
+test_expect_success 'verbose diff is stripped out with set core.commentChar' '
+       (
+               GIT_EDITOR=cat &&
+               export GIT_EDITOR &&
+               test_must_fail git -c core.commentchar=";" commit -a -v 2>err
+       ) &&
+       test_i18ngrep "Aborting commit due to empty commit message." err
+'
+
 test_done

diff --git a/wt-status.c b/wt-status.c
index b4e44baa2917dfcf33824c3d6fc08756886b9ae1..85390b813f12e2392b73dd6dffe8d1b41e838614 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,9 @@
 #include "column.h"
 #include "strbuf.h"
 
+static char cut_line[] =
+"------------------------ >8 ------------------------\n";
+
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
        GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
        GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
@@ -767,6 +770,18 @@ conclude:
        status_printf_ln(s, GIT_COLOR_NORMAL, "");
 }
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)
+{
+       const char *p;
+       struct strbuf pattern = STRBUF_INIT;
+
+       strbuf_addf(&pattern, "%c %s", comment_line_char, cut_line);
+       p = strstr(buf->buf, pattern.buf);
+       if (p && (p == buf->buf || p[-1] == '\n'))
+               strbuf_setlen(buf, p - buf->buf);
+       strbuf_release(&pattern);
+}
+
 static void wt_status_print_verbose(struct wt_status *s)
 {
        struct rev_info rev;
@@ -787,10 +802,20 @@ static void wt_status_print_verbose(struct wt_status *s)
         * If we're not going to stdout, then we definitely don't
         * want color, since we are going to the commit message
         * file (and even the "auto" setting won't work, since it
-        * will have checked isatty on stdout).
+        * will have checked isatty on stdout). But we then do want
+        * to insert the scissor line here to reliably remove the
+        * diff before committing.
         */
-       if (s->fp != stdout)
+       if (s->fp != stdout) {
+               const char *explanation = _("Do not touch the line above.\nEverything below will be removed.");
+               struct strbuf buf = STRBUF_INIT;
+
                rev.diffopt.use_color = 0;
+               fprintf(s->fp, "%c %s", comment_line_char, cut_line);
+               strbuf_add_commented_lines(&buf, explanation, strlen(explanation));
+               fputs(buf.buf, s->fp);
+               strbuf_release(&buf);
+       }
        run_diff_index(&rev, 1);
 }
 

diff --git a/wt-status.h b/wt-status.h
index 6c29e6f5e57f84eb7acdc3cf1ee621cc717fd5a9..30a481258366733ad5d6a65e3a9c3ae1ee74e833 100644 (file)
--- a/wt-status.h
+++ b/wt-status.h
@@ -91,6 +91,7 @@ struct wt_status_state {
        unsigned char cherry_pick_head_sha1[20];
 };
 
+void wt_status_truncate_message_at_cut_line(struct strbuf *);
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);