{"thread":{"id":"35372","subject":"[PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message","startedAt":"2013-11-20T22:31:59Z","lastAt":"2013-12-05T23:10:03Z","messageCount":7,"participants":["Jens Lehmann","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"230866","messageId":"528D385F.2070906@web.de","threadId":"35372","inReplyTo":null,"subject":"[PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-20T22:31:59Z","receivedAt":"2013-11-20T22:31:59Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When using the '-v' option of \"git commit\" the diff added to the commit\nmessage temporarily for editing is stripped off after the user exited the\neditor by searching for \"\\ndiff --git \" and truncating the commmit message\nthere if it is found.\n\nBut this approach has two problems:\n\n- when the commit message itself contains a line starting with\n  \"diff --git\" it will be truncated there prematurely; and\n\n- when the \"diff.submodule\" setting is set to \"log\", the diff may\n  start with \"Submodule <hash1>..<hash2>\", which will be left in\n  the commit message while it shouldn't.\n\nFix that by introducing a special scissor separator line starting with the\ncomment character ('#' or the core.commentChar config if set) followed by\ntwo lines describing what it is for. The scissor line - which will not be\ntranslated - is used to reliably detect the start of the diff so it can be\nchopped off from the commit message, no matter what the user enters there.\n\nTurn a known test failure fixed by this change into a successful test;\nalso add one for a diff starting with a submodule log and another one for\nproper handling of the comment char.\n\nReported-by: Ari Pollak <ari@debian.org>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\n\nChanges since v2:\n\n- Honor the core.commentChar setting and add a test for that.\n\n- Fix the submodule test to set the editor in a portable way.\n\n- Only print scissor and description lines when not going to stdout\n  (otherwise a \"git status -v\" prints that on stdout too, which\n  doesn't make much sense). Now we definitely do not have to care\n  about coloring these lines either.\n\n- Nicer formatting of the commit message.\n\n\n builtin/commit.c          |  9 +++------\n t/t7507-commit-verbose.sh | 28 +++++++++++++++++++++++++++-\n wt-status.c               | 23 +++++++++++++++++++++--\n wt-status.h               |  1 +\n 4 files changed, 52 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..fedb45a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tconst char *index_file, *reflog_msg;\n-\tchar *nl, *p;\n+\tchar *nl;\n \tunsigned char sha1[20];\n \tstruct ref_lock *ref_lock;\n \tstruct commit_list *parents = NULL, **pptr = &parents;\n@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n\n \t/* Truncate the message just before the diff, if any. */\n-\tif (verbose) {\n-\t\tp = strstr(sb.buf, \"\\ndiff --git \");\n-\t\tif (p != NULL)\n-\t\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n-\t}\n+\tif (verbose)\n+\t\twt_status_truncate_message_at_cut_line(&sb);\n\n \tif (cleanup_mode != CLEANUP_NONE)\n \t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex da5bd3b..2ddf28c 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' '\n \tcheck_message diff\n '\n\n-test_expect_failure 'diff in message is retained with -v' '\n+test_expect_success 'diff in message is retained with -v' '\n \tgit commit --amend -F diff -v &&\n \tcheck_message diff\n '\n\n+test_expect_success 'submodule log is stripped out too with -v' '\n+\tgit config diff.submodule log &&\n+\tgit submodule add ./. sub &&\n+\tgit commit -m \"sub added\" &&\n+\t(\n+\t\tcd sub &&\n+\t\techo \"more\" >>file &&\n+\t\tgit commit -a -m \"submodule commit\"\n+\t) &&\n+\t(\n+\t\tGIT_EDITOR=cat &&\n+\t\texport GIT_EDITOR &&\n+\t\ttest_must_fail git commit -a -v 2>err\n+\t) &&\n+\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n+'\n+\n+test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n+\t(\n+\t\tGIT_EDITOR=cat &&\n+\t\texport GIT_EDITOR &&\n+\t\ttest_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n+\t) &&\n+\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n+'\n+\n test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex b4e44ba..734f94b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -16,6 +16,9 @@\n #include \"column.h\"\n #include \"strbuf.h\"\n\n+static char wt_status_cut_line[] = /* 'X' is replaced with comment_line_char */\n+\"X ------------------------ >8 ------------------------\\n\";\n+\n static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n \tGIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */\n@@ -767,6 +770,15 @@ conclude:\n \tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n }\n\n+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n+{\n+\tconst char *p;\n+\n+\tp = strstr(buf->buf, wt_status_cut_line);\n+\tif (p && (p == buf->buf || p[-1] == '\\n'))\n+\t\tstrbuf_setlen(buf, p - buf->buf);\n+}\n+\n static void wt_status_print_verbose(struct wt_status *s)\n {\n \tstruct rev_info rev;\n@@ -787,10 +799,17 @@ static void wt_status_print_verbose(struct wt_status *s)\n \t * If we're not going to stdout, then we definitely don't\n \t * want color, since we are going to the commit message\n \t * file (and even the \"auto\" setting won't work, since it\n-\t * will have checked isatty on stdout).\n+\t * will have checked isatty on stdout). But we then do want\n+\t * to insert the scissor line here to reliably remove the\n+\t * diff before committing.\n \t */\n-\tif (s->fp != stdout)\n+\tif (s->fp != stdout) {\n \t\trev.diffopt.use_color = 0;\n+\t\twt_status_cut_line[0] = comment_line_char;\n+\t\tfprintf(s->fp, wt_status_cut_line);\n+\t\tfprintf(s->fp, _(\"%c Do not touch the line above.\\n\"), comment_line_char);\n+\t\tfprintf(s->fp, _(\"%c Everything below will be removed.\\n\"), comment_line_char);\n+\t}\n \trun_diff_index(&rev, 1);\n }\n\ndiff --git a/wt-status.h b/wt-status.h\nindex 6c29e6f..30a4812 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -91,6 +91,7 @@ struct wt_status_state {\n \tunsigned char cherry_pick_head_sha1[20];\n };\n\n+void wt_status_truncate_message_at_cut_line(struct strbuf *);\n void wt_status_prepare(struct wt_status *s);\n void wt_status_print(struct wt_status *s);\n void wt_status_collect(struct wt_status *s);\n-- \n1.8.5.rc2.7.g9470f78.dirty\n"},{"id":"230869","messageId":"xmqqpppu65fs.fsf@gitster.dls.corp.google.com","threadId":"35372","inReplyTo":"528D385F.2070906@web.de","subject":"Re: [PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-20T23:04:23Z","receivedAt":"2013-11-20T23:04:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> Changes since v2:\n>\n> - Honor the core.commentChar setting and add a test for that.\n>\n> - Fix the submodule test to set the editor in a portable way.\n>\n> - Only print scissor and description lines when not going to stdout\n>   (otherwise a \"git status -v\" prints that on stdout too, which\n>   doesn't make much sense). Now we definitely do not have to care\n>   about coloring these lines either.\n\nHmph.  It makes me suspect that we were drunk when we decided to let\n\"git status -v\" to keep showing diff when we declared that \"git\nstatus\" is different from \"git commit --dry-run\", but it is too late\nto fix it now, I think.\n\n> - Nicer formatting of the commit message.\n\nCertainly a lot easier to read, at least to me.\n\n> @@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \t}\n>\n>  \t/* Truncate the message just before the diff, if any. */\n\nI wonder if this comment still is valid, but it probably is OK.\n\n> -\tif (verbose) {\n> -\t\tp = strstr(sb.buf, \"\\ndiff --git \");\n> -\t\tif (p != NULL)\n> -\t\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n> -\t}\n> +\tif (verbose)\n> +\t\twt_status_truncate_message_at_cut_line(&sb);\n\n> diff --git a/wt-status.c b/wt-status.c\n> index b4e44ba..734f94b 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -16,6 +16,9 @@\n>  #include \"column.h\"\n>  #include \"strbuf.h\"\n>\n> +static char wt_status_cut_line[] = /* 'X' is replaced with comment_line_char */\n> +\"X ------------------------ >8 ------------------------\\n\";\n> +\n>  static char default_wt_status_colors[][COLOR_MAXLEN] = {\n>  \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n>  \tGIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */\n> @@ -767,6 +770,15 @@ conclude:\n>  \tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n>  }\n>\n> +void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n> +{\n> +\tconst char *p;\n> +\n> +\tp = strstr(buf->buf, wt_status_cut_line);\n> +\tif (p && (p == buf->buf || p[-1] == '\\n'))\n> +\t\tstrbuf_setlen(buf, p - buf->buf);\n> +}\n\nPerhaps it may happen that all the current callers have called\nwt_status_print_verbose() to cause wt_status_cut_line[0] to hold\ncomment_line_char, but relying on that calling sequence somehow\nmakes me feel uneasy.\n\nPerhaps cut_line[] should only have \"--- >8 ---\" part and both\nprinting side (below) and finding side (this one) should check these\nseparately?  That is:\n\n\tp = buf->buf;\n\twhile (p && *p) {\n\t\tp = strchr(p, comment_line_char);\n                if (!p)\n\t\t\tbreak;\n\t\tif (strstr(p + 1, cut_line) == p + 1)\n\t\t\tbreak;\n\t\tp++;\n                continue;\n\t}\n        if (p && *p && (p == buf->buf || p[-1] == '\\n'))\n\t\tstrbuf_setlen(buf, p - buf->buf);\n\nor something (the above is deliberately less-efficient-than-ideal,\nbecause I want to keep the code structure in such a way that we can\nlater turn comment_line_char to a string[] that can hold \"//\" to\nallow a multi-char comment introducer more easily)?\n\n>  static void wt_status_print_verbose(struct wt_status *s)\n>  {\n>  \tstruct rev_info rev;\n> @@ -787,10 +799,17 @@ static void wt_status_print_verbose(struct wt_status *s)\n>  \t * If we're not going to stdout, then we definitely don't\n>  \t * want color, since we are going to the commit message\n>  \t * file (and even the \"auto\" setting won't work, since it\n> -\t * will have checked isatty on stdout).\n> +\t * will have checked isatty on stdout). But we then do want\n> +\t * to insert the scissor line here to reliably remove the\n> +\t * diff before committing.\n>  \t */\n> -\tif (s->fp != stdout)\n> +\tif (s->fp != stdout) {\n>  \t\trev.diffopt.use_color = 0;\n> +\t\twt_status_cut_line[0] = comment_line_char;\n> +\t\tfprintf(s->fp, wt_status_cut_line);\n> +\t\tfprintf(s->fp, _(\"%c Do not touch the line above.\\n\"), comment_line_char);\n> +\t\tfprintf(s->fp, _(\"%c Everything below will be removed.\\n\"), comment_line_char);\n> +\t}\n\nI didn't bother with my \"how about this\" version, but we may want to\nuse strbuf_add_commented_lines() to help i18n/l10n folks.  Depending\non the l10n, this message may want to become more or less than 2\nlines.\n\n>  \trun_diff_index(&rev, 1);\n>  }\n>\n> diff --git a/wt-status.h b/wt-status.h\n> index 6c29e6f..30a4812 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -91,6 +91,7 @@ struct wt_status_state {\n>  \tunsigned char cherry_pick_head_sha1[20];\n>  };\n>\n> +void wt_status_truncate_message_at_cut_line(struct strbuf *);\n>  void wt_status_prepare(struct wt_status *s);\n>  void wt_status_print(struct wt_status *s);\n>  void wt_status_collect(struct wt_status *s);\n"},{"id":"230916","messageId":"528E7A6E.8080603@web.de","threadId":"35372","inReplyTo":"xmqqpppu65fs.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-21T21:26:06Z","receivedAt":"2013-11-21T21:26:06Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 21.11.2013 00:04, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>> diff --git a/wt-status.c b/wt-status.c\n>> index b4e44ba..734f94b 100644\n>> --- a/wt-status.c\n>> +++ b/wt-status.c\n>> @@ -16,6 +16,9 @@\n>>  #include \"column.h\"\n>>  #include \"strbuf.h\"\n>>\n>> +static char wt_status_cut_line[] = /* 'X' is replaced with comment_line_char */\n>> +\"X ------------------------ >8 ------------------------\\n\";\n>> +\n>>  static char default_wt_status_colors[][COLOR_MAXLEN] = {\n>>  \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n>>  \tGIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */\n>> @@ -767,6 +770,15 @@ conclude:\n>>  \tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n>>  }\n>>\n>> +void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n>> +{\n>> +\tconst char *p;\n>> +\n>> +\tp = strstr(buf->buf, wt_status_cut_line);\n>> +\tif (p && (p == buf->buf || p[-1] == '\\n'))\n>> +\t\tstrbuf_setlen(buf, p - buf->buf);\n>> +}\n> \n> Perhaps it may happen that all the current callers have called\n> wt_status_print_verbose() to cause wt_status_cut_line[0] to hold\n> comment_line_char, but relying on that calling sequence somehow\n> makes me feel uneasy.\n\nI initialized the place to be occupied by the comment_line_char\nin wt_status_cut_line with 'X' on purpose to notice such a\nproblem. But I'd be also fine with setting wt_status_cut_line[0]\nagain here just to be sure. But please also see below ...\n\n> Perhaps cut_line[] should only have \"--- >8 ---\" part and both\n> printing side (below) and finding side (this one) should check these\n> separately?\n\n... ok ...\n\n> That is:\n> \n> \tp = buf->buf;\n> \twhile (p && *p) {\n> \t\tp = strchr(p, comment_line_char);\n>                 if (!p)\n> \t\t\tbreak;\n> \t\tif (strstr(p + 1, cut_line) == p + 1)\n> \t\t\tbreak;\n> \t\tp++;\n>                 continue;\n> \t}\n>         if (p && *p && (p == buf->buf || p[-1] == '\\n'))\n> \t\tstrbuf_setlen(buf, p - buf->buf);\n> \n> or something (the above is deliberately less-efficient-than-ideal,\n> because I want to keep the code structure in such a way that we can\n> later turn comment_line_char to a string[] that can hold \"//\" to\n> allow a multi-char comment introducer more easily)?\n\nHmm, I'm a bit reluctant to go that far to optimize this patch for\nanother one that might materialize later. But what about this:\n\n\tstruct strbuf cut_line = STRBUF_INIT;\n\tstrbuf_addf(&cut_line, \"%c %s\", comment_line_char, wt_status_cut_line);\n\tp = strstr(buf->buf, cut_line.buf);\n\tif (p && (p == buf->buf || p[-1] == '\\n'))\n\t\tstrbuf_setlen(buf, p - buf->buf);\n\tstrbuf_release(&cut_line);\n\nThat is shorter can easily be adapted to a comment line string later.\nAnd even though it's slightly less performant should not be a problem\nhere as this happens only once after invoking an editor for user input.\n\n>>  static void wt_status_print_verbose(struct wt_status *s)\n>>  {\n>>  \tstruct rev_info rev;\n>> @@ -787,10 +799,17 @@ static void wt_status_print_verbose(struct wt_status *s)\n>>  \t * If we're not going to stdout, then we definitely don't\n>>  \t * want color, since we are going to the commit message\n>>  \t * file (and even the \"auto\" setting won't work, since it\n>> -\t * will have checked isatty on stdout).\n>> +\t * will have checked isatty on stdout). But we then do want\n>> +\t * to insert the scissor line here to reliably remove the\n>> +\t * diff before committing.\n>>  \t */\n>> -\tif (s->fp != stdout)\n>> +\tif (s->fp != stdout) {\n>>  \t\trev.diffopt.use_color = 0;\n>> +\t\twt_status_cut_line[0] = comment_line_char;\n>> +\t\tfprintf(s->fp, wt_status_cut_line);\n>> +\t\tfprintf(s->fp, _(\"%c Do not touch the line above.\\n\"), comment_line_char);\n>> +\t\tfprintf(s->fp, _(\"%c Everything below will be removed.\\n\"), comment_line_char);\n>> +\t}\n> \n> I didn't bother with my \"how about this\" version, but we may want to\n> use strbuf_add_commented_lines() to help i18n/l10n folks.  Depending\n> on the l10n, this message may want to become more or less than 2\n> lines.\n\nMakes sense, will change that (maybe using strbuf_commented_addf()\ninstead) for v4.\n"},{"id":"230919","messageId":"xmqqsiup2y3u.fsf@gitster.dls.corp.google.com","threadId":"35372","inReplyTo":"528E7A6E.8080603@web.de","subject":"Re: [PATCH v3] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-21T22:23:17Z","receivedAt":"2013-11-21T22:23:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> But what about this:\n>\n> \tstruct strbuf cut_line = STRBUF_INIT;\n> \tstrbuf_addf(&cut_line, \"%c %s\", comment_line_char, wt_status_cut_line);\n> \tp = strstr(buf->buf, cut_line.buf);\n> \tif (p && (p == buf->buf || p[-1] == '\\n'))\n> \t\tstrbuf_setlen(buf, p - buf->buf);\n> \tstrbuf_release(&cut_line);\n>\n> That is shorter can easily be adapted to a comment line string later.\n\nSure, that would work fine.\n\nThanks.\n"},{"id":"231627","messageId":"52A0D78E.4030509@web.de","threadId":"35372","inReplyTo":"xmqqsiup2y3u.fsf@gitster.dls.corp.google.com","subject":"[PATCH v4] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-12-05T19:44:14Z","receivedAt":"2013-12-05T19:44:14Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When using the '-v' option of \"git commit\" the diff added to the commit\nmessage temporarily for editing is stripped off after the user exited the\neditor by searching for \"\\ndiff --git \" and truncating the commmit message\nthere if it is found.\n\nBut this approach has two problems:\n\n- when the commit message itself contains a line starting with\n  \"diff --git\" it will be truncated there prematurely; and\n\n- when the \"diff.submodule\" setting is set to \"log\", the diff may\n  start with \"Submodule <hash1>..<hash2>\", which will be left in\n  the commit message while it shouldn't.\n\nFix that by introducing a special scissor separator line starting with the\ncomment character ('#' or the core.commentChar config if set) followed by\ntwo lines describing what it is for. The scissor line - which will not be\ntranslated - is used to reliably detect the start of the diff so it can be\nchopped off from the commit message, no matter what the user enters there.\n\nTurn a known test failure fixed by this change into a successful test;\nalso add one for a diff starting with a submodule log and another one for\nproper handling of the comment char.\n\nReported-by: Ari Pollak <ari@debian.org>\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nChanges to v3:\n\n- separating comment_line_char from the cut_line\n\n- using strbuf_add_commented_lines() for the comment\n\nAll issues raised should be addressed with this version.\n\n\n builtin/commit.c          |  9 +++------\n t/t7507-commit-verbose.sh | 28 +++++++++++++++++++++++++++-\n wt-status.c               | 29 +++++++++++++++++++++++++++--\n wt-status.h               |  1 +\n 4 files changed, 58 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..fedb45a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf author_ident = STRBUF_INIT;\n \tconst char *index_file, *reflog_msg;\n-\tchar *nl, *p;\n+\tchar *nl;\n \tunsigned char sha1[20];\n \tstruct ref_lock *ref_lock;\n \tstruct commit_list *parents = NULL, **pptr = &parents;\n@@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n\n \t/* Truncate the message just before the diff, if any. */\n-\tif (verbose) {\n-\t\tp = strstr(sb.buf, \"\\ndiff --git \");\n-\t\tif (p != NULL)\n-\t\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n-\t}\n+\tif (verbose)\n+\t\twt_status_truncate_message_at_cut_line(&sb);\n\n \tif (cleanup_mode != CLEANUP_NONE)\n \t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex da5bd3b..2ddf28c 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' '\n \tcheck_message diff\n '\n\n-test_expect_failure 'diff in message is retained with -v' '\n+test_expect_success 'diff in message is retained with -v' '\n \tgit commit --amend -F diff -v &&\n \tcheck_message diff\n '\n\n+test_expect_success 'submodule log is stripped out too with -v' '\n+\tgit config diff.submodule log &&\n+\tgit submodule add ./. sub &&\n+\tgit commit -m \"sub added\" &&\n+\t(\n+\t\tcd sub &&\n+\t\techo \"more\" >>file &&\n+\t\tgit commit -a -m \"submodule commit\"\n+\t) &&\n+\t(\n+\t\tGIT_EDITOR=cat &&\n+\t\texport GIT_EDITOR &&\n+\t\ttest_must_fail git commit -a -v 2>err\n+\t) &&\n+\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n+'\n+\n+test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n+\t(\n+\t\tGIT_EDITOR=cat &&\n+\t\texport GIT_EDITOR &&\n+\t\ttest_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n+\t) &&\n+\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n+'\n+\n test_done\ndiff --git a/wt-status.c b/wt-status.c\nindex b4e44ba..99c3d1c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -16,6 +16,9 @@\n #include \"column.h\"\n #include \"strbuf.h\"\n\n+static char cut_line[] =\n+\"------------------------ >8 ------------------------\\n\";\n+\n static char default_wt_status_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n \tGIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */\n@@ -767,6 +770,18 @@ conclude:\n \tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n }\n\n+void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n+{\n+\tconst char *p;\n+\tstruct strbuf pattern = STRBUF_INIT;\n+\n+\tstrbuf_addf(&pattern, \"%c %s\", comment_line_char, cut_line);\n+\tp = strstr(buf->buf, pattern.buf);\n+\tif (p && (p == buf->buf || p[-1] == '\\n'))\n+\t\tstrbuf_setlen(buf, p - buf->buf);\n+\tstrbuf_release(&pattern);\n+}\n+\n static void wt_status_print_verbose(struct wt_status *s)\n {\n \tstruct rev_info rev;\n@@ -787,10 +802,20 @@ static void wt_status_print_verbose(struct wt_status *s)\n \t * If we're not going to stdout, then we definitely don't\n \t * want color, since we are going to the commit message\n \t * file (and even the \"auto\" setting won't work, since it\n-\t * will have checked isatty on stdout).\n+\t * will have checked isatty on stdout). But we then do want\n+\t * to insert the scissor line here to reliably remove the\n+\t * diff before committing.\n \t */\n-\tif (s->fp != stdout)\n+\tif (s->fp != stdout) {\n+\t\tconst char *explanation = _(\"Do not touch the line above.\\nEverything below will be removed.\");\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n \t\trev.diffopt.use_color = 0;\n+\t\tfprintf(s->fp, \"%c %s\", comment_line_char, cut_line);\n+\t\tstrbuf_add_commented_lines(&buf, explanation, strlen(explanation));\n+\t\tfprintf(s->fp, buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n \trun_diff_index(&rev, 1);\n }\n\ndiff --git a/wt-status.h b/wt-status.h\nindex 6c29e6f..30a4812 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -91,6 +91,7 @@ struct wt_status_state {\n \tunsigned char cherry_pick_head_sha1[20];\n };\n\n+void wt_status_truncate_message_at_cut_line(struct strbuf *);\n void wt_status_prepare(struct wt_status *s);\n void wt_status_print(struct wt_status *s);\n void wt_status_collect(struct wt_status *s);\n-- \n1.8.5.4.g9a09ec7\n"},{"id":"231633","messageId":"xmqqk3fj3vxb.fsf@gitster.dls.corp.google.com","threadId":"35372","inReplyTo":"52A0D78E.4030509@web.de","subject":"Re: [PATCH v4] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-05T20:05:52Z","receivedAt":"2013-12-05T20:05:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> When using the '-v' option of \"git commit\" the diff added to the commit\n> message temporarily for editing is stripped off after the user exited the\n> editor by searching for \"\\ndiff --git \" and truncating the commmit message\n> there if it is found.\n>\n> But this approach has two problems:\n>\n> - when the commit message itself contains a line starting with\n>   \"diff --git\" it will be truncated there prematurely; and\n>\n> - when the \"diff.submodule\" setting is set to \"log\", the diff may\n>   start with \"Submodule <hash1>..<hash2>\", which will be left in\n>   the commit message while it shouldn't.\n>\n> Fix that by introducing a special scissor separator line starting with the\n> comment character ('#' or the core.commentChar config if set) followed by\n> two lines describing what it is for. The scissor line - which will not be\n> translated - is used to reliably detect the start of the diff so it can be\n> chopped off from the commit message, no matter what the user enters there.\n>\n> Turn a known test failure fixed by this change into a successful test;\n> also add one for a diff starting with a submodule log and another one for\n> proper handling of the comment char.\n>\n> Reported-by: Ari Pollak <ari@debian.org>\n> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n> ---\n>\n> Changes to v3:\n>\n> - separating comment_line_char from the cut_line\n>\n> - using strbuf_add_commented_lines() for the comment\n>\n> All issues raised should be addressed with this version.\n\nNicely done.\n\nThanks, will replace and queue.\n\n>  builtin/commit.c          |  9 +++------\n>  t/t7507-commit-verbose.sh | 28 +++++++++++++++++++++++++++-\n>  wt-status.c               | 29 +++++++++++++++++++++++++++--\n>  wt-status.h               |  1 +\n>  4 files changed, 58 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 6ab4605..fedb45a 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1505,7 +1505,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tstruct strbuf author_ident = STRBUF_INIT;\n>  \tconst char *index_file, *reflog_msg;\n> -\tchar *nl, *p;\n> +\tchar *nl;\n>  \tunsigned char sha1[20];\n>  \tstruct ref_lock *ref_lock;\n>  \tstruct commit_list *parents = NULL, **pptr = &parents;\n> @@ -1601,11 +1601,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \t}\n>\n>  \t/* Truncate the message just before the diff, if any. */\n> -\tif (verbose) {\n> -\t\tp = strstr(sb.buf, \"\\ndiff --git \");\n> -\t\tif (p != NULL)\n> -\t\t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n> -\t}\n> +\tif (verbose)\n> +\t\twt_status_truncate_message_at_cut_line(&sb);\n>\n>  \tif (cleanup_mode != CLEANUP_NONE)\n>  \t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index da5bd3b..2ddf28c 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -65,9 +65,35 @@ test_expect_success 'diff in message is retained without -v' '\n>  \tcheck_message diff\n>  '\n>\n> -test_expect_failure 'diff in message is retained with -v' '\n> +test_expect_success 'diff in message is retained with -v' '\n>  \tgit commit --amend -F diff -v &&\n>  \tcheck_message diff\n>  '\n>\n> +test_expect_success 'submodule log is stripped out too with -v' '\n> +\tgit config diff.submodule log &&\n> +\tgit submodule add ./. sub &&\n> +\tgit commit -m \"sub added\" &&\n> +\t(\n> +\t\tcd sub &&\n> +\t\techo \"more\" >>file &&\n> +\t\tgit commit -a -m \"submodule commit\"\n> +\t) &&\n> +\t(\n> +\t\tGIT_EDITOR=cat &&\n> +\t\texport GIT_EDITOR &&\n> +\t\ttest_must_fail git commit -a -v 2>err\n> +\t) &&\n> +\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n> +'\n> +\n> +test_expect_success 'verbose diff is stripped out with set core.commentChar' '\n> +\t(\n> +\t\tGIT_EDITOR=cat &&\n> +\t\texport GIT_EDITOR &&\n> +\t\ttest_must_fail git -c core.commentchar=\";\" commit -a -v 2>err\n> +\t) &&\n> +\ttest_i18ngrep \"Aborting commit due to empty commit message.\" err\n> +'\n> +\n>  test_done\n> diff --git a/wt-status.c b/wt-status.c\n> index b4e44ba..99c3d1c 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -16,6 +16,9 @@\n>  #include \"column.h\"\n>  #include \"strbuf.h\"\n>\n> +static char cut_line[] =\n> +\"------------------------ >8 ------------------------\\n\";\n> +\n>  static char default_wt_status_colors[][COLOR_MAXLEN] = {\n>  \tGIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n>  \tGIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */\n> @@ -767,6 +770,18 @@ conclude:\n>  \tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n>  }\n>\n> +void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n> +{\n> +\tconst char *p;\n> +\tstruct strbuf pattern = STRBUF_INIT;\n> +\n> +\tstrbuf_addf(&pattern, \"%c %s\", comment_line_char, cut_line);\n> +\tp = strstr(buf->buf, pattern.buf);\n> +\tif (p && (p == buf->buf || p[-1] == '\\n'))\n> +\t\tstrbuf_setlen(buf, p - buf->buf);\n> +\tstrbuf_release(&pattern);\n> +}\n> +\n>  static void wt_status_print_verbose(struct wt_status *s)\n>  {\n>  \tstruct rev_info rev;\n> @@ -787,10 +802,20 @@ static void wt_status_print_verbose(struct wt_status *s)\n>  \t * If we're not going to stdout, then we definitely don't\n>  \t * want color, since we are going to the commit message\n>  \t * file (and even the \"auto\" setting won't work, since it\n> -\t * will have checked isatty on stdout).\n> +\t * will have checked isatty on stdout). But we then do want\n> +\t * to insert the scissor line here to reliably remove the\n> +\t * diff before committing.\n>  \t */\n> -\tif (s->fp != stdout)\n> +\tif (s->fp != stdout) {\n> +\t\tconst char *explanation = _(\"Do not touch the line above.\\nEverything below will be removed.\");\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\n>  \t\trev.diffopt.use_color = 0;\n> +\t\tfprintf(s->fp, \"%c %s\", comment_line_char, cut_line);\n> +\t\tstrbuf_add_commented_lines(&buf, explanation, strlen(explanation));\n> +\t\tfprintf(s->fp, buf.buf);\n> +\t\tstrbuf_release(&buf);\n> +\t}\n>  \trun_diff_index(&rev, 1);\n>  }\n>\n> diff --git a/wt-status.h b/wt-status.h\n> index 6c29e6f..30a4812 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -91,6 +91,7 @@ struct wt_status_state {\n>  \tunsigned char cherry_pick_head_sha1[20];\n>  };\n>\n> +void wt_status_truncate_message_at_cut_line(struct strbuf *);\n>  void wt_status_prepare(struct wt_status *s);\n>  void wt_status_print(struct wt_status *s);\n>  void wt_status_collect(struct wt_status *s);\n"},{"id":"231643","messageId":"xmqqzjoe3nec.fsf@gitster.dls.corp.google.com","threadId":"35372","inReplyTo":"52A0D78E.4030509@web.de","subject":"Re: [PATCH v4] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-05T23:10:03Z","receivedAt":"2013-12-05T23:10:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> +\t\tfprintf(s->fp, \"%c %s\", comment_line_char, cut_line);\n> +\t\tstrbuf_add_commented_lines(&buf, explanation, strlen(explanation));\n> +\t\tfprintf(s->fp, buf.buf);\n\nThis is better done with:\n\n\tfputs(buf.buf, s->fp);\n\nAlready locally tweaked while applying, so no need to resend only\nfor this change.\n"}]}