{"thread":{"id":"35316","subject":"Bug? diff.submodule=log adds text to commit -v message","startedAt":"2013-11-10T21:49:20Z","lastAt":"2013-11-19T21:01:24Z","messageCount":23,"participants":["Ari Pollak","Jens Lehmann","Jeff King","Johannes Sixt","Junio C Hamano","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"230375","messageId":"loom.20131110T222043-57@post.gmane.org","threadId":"35316","inReplyTo":null,"subject":"Bug? diff.submodule=log adds text to commit -v message","fromName":"Ari Pollak","fromEmail":"ari@debian.org","sentAt":"2013-11-10T21:49:20Z","receivedAt":"2013-11-10T21:49:20Z","isPatch":false,"sender":{"key":"ari@debian.org","avatar":null},"body":"Hi,\n\nI'm using git 1.8.4.2, and I've set the \"diff.submodule = log\" option \nglobally. If I change the revision that a submodule is set to, then run\n\"git commit -av\", The submodule shortlog is appended to the log message without \nany #s before it, so the log messages get included in my own log message. \nThis seems like a bug and not a feature, as diffs aren't normally included in \nthe commit message with -v.\n\nCheers,\nAri\n"},{"id":"230426","messageId":"528140F5.6090700@web.de","threadId":"35316","inReplyTo":"loom.20131110T222043-57@post.gmane.org","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-11T20:41:25Z","receivedAt":"2013-11-11T20:41:25Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Hi Ari,\n\nAm 10.11.2013 22:49, schrieb Ari Pollak:\n> I'm using git 1.8.4.2, and I've set the \"diff.submodule = log\" option \n> globally. If I change the revision that a submodule is set to, then run\n> \"git commit -av\", The submodule shortlog is appended to the log message without \n> any #s before it, so the log messages get included in my own log message. \n> This seems like a bug and not a feature, as diffs aren't normally included in \n> the commit message with -v.\n\nThanks for your report, I can reproduce that here. But first I think\nthis is unrelated to the \"diff.submodule = log\" setting, as without it\nyou'll just see the submodule commit hash diff instead of the shortlog\n(which is perfectly consistent with what I'd expect from this setting).\nAnd secondly what you describe looks like documented behavior, the man\npage of \"git commit\" states:\n\n -v, --verbose\n Show unified diff between the HEAD commit and what would be\n committed at the bottom of the commit message template. Note that\n this diff output doesn't have its lines prefixed with #.\n\nAnd after adding a modified file the log message also shows the diff of\nthat file (and without leading \"# \"s too), so I doubt that diffs aren't\nnormally included in the commit message with -v. What am I missing?\n\n\nThanks\nJens\n"},{"id":"230431","messageId":"loom.20131111T214646-550@post.gmane.org","threadId":"35316","inReplyTo":"528140F5.6090700@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Ari Pollak","fromEmail":"ari@debian.org","sentAt":"2013-11-11T20:48:18Z","receivedAt":"2013-11-11T20:48:18Z","isPatch":false,"sender":{"key":"ari@debian.org","avatar":null},"body":"Jens Lehmann writes:\n> And after adding a modified file the log message also shows the diff of\n> that file (and without leading \"# \"s too), so I doubt that diffs aren't\n> normally included in the commit message with -v. What am I missing?\n\nAh, it is true that -v normally does not prefix the diffs with #, but there \nmust be some filter in place after I save & quit my editor when a normal file \ndiff is present, since that does not get included in the final commit \nmessage. But the submodule log does get included, which does not seem \nintentional.\n\nCheers,\nAri\n"},{"id":"230440","messageId":"52814C35.6040205@web.de","threadId":"35316","inReplyTo":"loom.20131111T214646-550@post.gmane.org","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-11T21:29:25Z","receivedAt":"2013-11-11T21:29:25Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 11.11.2013 21:48, schrieb Ari Pollak:\n> Jens Lehmann writes:\n>> And after adding a modified file the log message also shows the diff of\n>> that file (and without leading \"# \"s too), so I doubt that diffs aren't\n>> normally included in the commit message with -v. What am I missing?\n> \n> Ah, it is true that -v normally does not prefix the diffs with #, but there \n> must be some filter in place after I save & quit my editor when a normal file \n> diff is present, since that does not get included in the final commit \n> message. But the submodule log does get included, which does not seem \n> intentional.\n\nOk, now this makes sense. \"git commit\" strips off the diff added by\n-v by skipping everything starting with \"\\ndiff --git \". But that\nlogic fails when the \"diff.submodule = log\" setting adds a shortlog\ninstead of a regular diff, as that starts with \"\\nSubmodule \".\n\nThe diff below fixes the problem you describe for me. (But I do not\nconsider it a worthwhile fix in its current form because a line\nstarting with \"Submodule \" might appear in a perfectly normal commit\nmessage, while \"diff --git \" most probably won't).\n\nAnd while testing this issue I noticed another problem: When using\n\"git commit -a\" not only the staged commits of a submodule get\ncommitted, but also the unstaged commits. Will look into that too.\n\n-------------------------8<---------------------\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..ff6e171 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1600,11 +1600,14 @@ int cmd_commit(int argc, const char **argv, const char *\n die(_(\"could not read commit message: %s\"), strerror(saved_errno\n }\n\n- /* Truncate the message just before the diff, if any. */\n+ /* Truncate the message just before the diff or submodule shortlog */\n if (verbose) {\n p = strstr(sb.buf, \"\\ndiff --git \");\n if (p != NULL)\n strbuf_setlen(&sb, p - sb.buf + 1);\n+ p = strstr(sb.buf, \"\\nSubmodule \");\n+ if (p != NULL)\n+ strbuf_setlen(&sb, p - sb.buf + 1);\n }\n\n if (cleanup_mode != CLEANUP_NONE)\n"},{"id":"230441","messageId":"52814D73.4010706@web.de","threadId":"35316","inReplyTo":"52814C35.6040205@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-11T21:34:43Z","receivedAt":"2013-11-11T21:34:43Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 11.11.2013 22:29, schrieb Jens Lehmann:\n> And while testing this issue I noticed another problem: When using\n> \"git commit -a\" not only the staged commits of a submodule get\n> committed, but also the unstaged commits. Will look into that too.\n\nOk, scrap that. This is exactly what is expected.\n"},{"id":"230455","messageId":"20131111232412.GA14924@sigill.intra.peff.net","threadId":"35316","inReplyTo":"52814C35.6040205@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-11T23:24:12Z","receivedAt":"2013-11-11T23:24:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 11, 2013 at 10:29:25PM +0100, Jens Lehmann wrote:\n\n> The diff below fixes the problem you describe for me. (But I do not\n> consider it a worthwhile fix in its current form because a line\n> starting with \"Submodule \" might appear in a perfectly normal commit\n> message, while \"diff --git \" most probably won't).\n\nYeah, this fix makes me nervous for that reason. \"commit -v\" has always\nbeen a little bit flaky in that respect, as it is simply guessing at\nthe beginning of the diff text it added earlier. In addition to false\nnegatives, it also has false positives, stripping out people's diffs\nthat they meant to include in the commit message.\n\nThe \"right\" way to fix this is to change the format to use some more\nrobust marker, like:\n\n # Everything below this line is a diff that will be removed.\n\nI do not know offhand if anybody's commit-template generating or parsing\nscripts would be broken, but I doubt the fallout would be that big. When\nlast we discussed this (AFAICT), we did not yet have 0b38227 (commit:\nFix stripping of patch in verbose mode., 2008-11-12), which meant that\nit would affect _everybody_. Nowadays it would only affect users of\n\"-v\", which is presumably a much smaller population.\n\n-Peff\n"},{"id":"230482","messageId":"5281DCC5.2000209@kdbg.org","threadId":"35316","inReplyTo":"52814C35.6040205@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-11-12T07:46:13Z","receivedAt":"2013-11-12T07:46:13Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.11.2013 22:29, schrieb Jens Lehmann:\n> The diff below fixes the problem you describe for me. (But I do not\n> consider it a worthwhile fix in its current form because a line\n> starting with \"Submodule \" might appear in a perfectly normal commit\n> message, while \"diff --git \" most probably won't).\n\nAnd on top of that, \"Submodule \" originates from a translatable string,\ndoesn't it?\n\n-- Hannes\n"},{"id":"230539","messageId":"5282A90A.4030900@web.de","threadId":"35316","inReplyTo":"5281DCC5.2000209@kdbg.org","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-12T22:17:46Z","receivedAt":"2013-11-12T22:17:46Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 12.11.2013 08:46, schrieb Johannes Sixt:\n> Am 11.11.2013 22:29, schrieb Jens Lehmann:\n>> The diff below fixes the problem you describe for me. (But I do not\n>> consider it a worthwhile fix in its current form because a line\n>> starting with \"Submodule \" might appear in a perfectly normal commit\n>> message, while \"diff --git \" most probably won't).\n> \n> And on top of that, \"Submodule \" originates from a translatable string,\n> doesn't it?\n\nThis would also be true for the marker line that Peff proposed:\n\n # Everything below this line is a diff that will be removed.\n\nBut I suspect that would be ok if the marker would be both added\nand searched for in its translated form. Or is it possible that\nthe locale changes between those two steps?\n"},{"id":"230540","messageId":"xmqqy54tfeje.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"5282A90A.4030900@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-12T22:20:21Z","receivedAt":"2013-11-12T22:20:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Am 12.11.2013 08:46, schrieb Johannes Sixt:\n>> Am 11.11.2013 22:29, schrieb Jens Lehmann:\n>>> The diff below fixes the problem you describe for me. (But I do not\n>>> consider it a worthwhile fix in its current form because a line\n>>> starting with \"Submodule \" might appear in a perfectly normal commit\n>>> message, while \"diff --git \" most probably won't).\n>> \n>> And on top of that, \"Submodule \" originates from a translatable string,\n>> doesn't it?\n>\n> This would also be true for the marker line that Peff proposed:\n>\n> # Everything below this line is a diff that will be removed.\n>\n> But I suspect that would be ok if the marker would be both added\n> and searched for in its translated form. Or is it possible that\n> the locale changes between those two steps?\n\nIf we were introducing a divider line for machine consumption, I do\nnot think it is wise to let that line even translated...\n"},{"id":"230571","messageId":"5283C701.8090400@web.de","threadId":"35316","inReplyTo":"xmqqy54tfeje.fsf@gitster.dls.corp.google.com","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-13T18:37:53Z","receivedAt":"2013-11-13T18:37:53Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 12.11.2013 23:20, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> Am 12.11.2013 08:46, schrieb Johannes Sixt:\n>>> Am 11.11.2013 22:29, schrieb Jens Lehmann:\n>>>> The diff below fixes the problem you describe for me. (But I do not\n>>>> consider it a worthwhile fix in its current form because a line\n>>>> starting with \"Submodule \" might appear in a perfectly normal commit\n>>>> message, while \"diff --git \" most probably won't).\n>>>\n>>> And on top of that, \"Submodule \" originates from a translatable string,\n>>> doesn't it?\n>>\n>> This would also be true for the marker line that Peff proposed:\n>>\n>> # Everything below this line is a diff that will be removed.\n>>\n>> But I suspect that would be ok if the marker would be both added\n>> and searched for in its translated form. Or is it possible that\n>> the locale changes between those two steps?\n> \n> If we were introducing a divider line for machine consumption, I do\n> not think it is wise to let that line even translated...\n\nOk, but then it won't mean much to readers who don't understand\nEnglish. I assume prefixing all diff lines with \"# \" is out of\nthe question because of backwards compatibility, so what about\nusing a descriptive text together with a scissor line? The former\ncan be be translated (and won't make it into the commit message\nbecause it starts with a \"#\") while the latter serves as a robust\ndivider line:\n\n# Everything below the following line is a diff that will be removed.\n# --------------------------------8<--------------------------------\n"},{"id":"230575","messageId":"xmqq7gccdq67.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"5283C701.8090400@web.de","subject":"Re: Bug? diff.submodule=log adds text to commit -v message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-13T20:04:16Z","receivedAt":"2013-11-13T20:04:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n>> If we were introducing a divider line for machine consumption, I do\n>> not think it is wise to let that line even translated...\n>\n> Ok, but then it won't mean much to readers who don't understand\n> English. I assume prefixing all diff lines with \"# \" is out of\n> the question because of backwards compatibility, so what about\n> using a descriptive text together with a scissor line? The former\n> can be be translated (and won't make it into the commit message\n> because it starts with a \"#\") while the latter serves as a robust\n> divider line:\n>\n> # Everything below the following line is a diff that will be removed.\n> # --------------------------------8<--------------------------------\n\nYeah, or swap them around if you are trying to protect the part\nabove the divider from getting contaminated by the noise.\n"},{"id":"230714","messageId":"5287F735.3030306@web.de","threadId":"35316","inReplyTo":"xmqq7gccdq67.fsf@gitster.dls.corp.google.com","subject":"[RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-16T22:52:37Z","receivedAt":"2013-11-16T22:52:37Z","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. But this approach has two problems: when the commit\nmessage itself contains a line starting with \"diff --git\" it will be\ntruncated there prematurely. And when the \"diff.submodule\" setting is set\nto \"log\", the diff may start with \"Submodule ..\", which will\nbe left in the commit message while it shouldn't.\n\nFix that by introducing a special scissor separator line starting with the\ncomment character '#' followed by a line describing what it is for. The\nscissor line 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.\nTurn a known test failure fixed by this change into a successful test and\nadd another one for a diff starting with a submodule log.\n\nReported-by: Ari Pollak \nSigned-off-by: Jens Lehmann \n---\n\n\nAm 13.11.2013 21:04, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>>> If we were introducing a divider line for machine consumption, I do\n>>> not think it is wise to let that line even translated...\n>>\n>> Ok, but then it won't mean much to readers who don't understand\n>> English. I assume prefixing all diff lines with \"# \" is out of\n>> the question because of backwards compatibility, so what about\n>> using a descriptive text together with a scissor line? The former\n>> can be be translated (and won't make it into the commit message\n>> because it starts with a \"#\") while the latter serves as a robust\n>> divider line:\n>>\n>> # Everything below the following line is a diff that will be removed.\n>> # --------------------------------8<--------------------------------\n> \n> Yeah, or swap them around if you are trying to protect the part\n> above the divider from getting contaminated by the noise.\n\nOk, did that. I couldn't find another user of the verbose setting,\nso emitting the scissor line together with the description inside\nwt_status_print_verbose() should be ok (or did I miss something\nhere?). But currently these lines use the hardcoded '#' instead of\nthe status.displaycommentprefix configuration and ignores coloring,\nlooks like I need to fix this in the next iteration.\n\n\n builtin/commit.c | 6 +++---\n t/t7507-commit-verbose.sh | 15 ++++++++++++++-\n wt-status.c | 4 ++++\n wt-status.h | 2 ++\n 4 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..091a6e7 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tp = strstr(sb.buf, wt_status_diff_divider);\n+\t\tif ((p != NULL) && (p > sb.buf) && (p[-1] == '\\n'))\n+\t\t\tstrbuf_setlen(&sb, p - sb.buf);\n \t}\n\n \tif (cleanup_mode != CLEANUP_NONE)\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex da5bd3b..09c1150 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -65,9 +65,22 @@ 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+\tGIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&\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..a3f7115 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -16,6 +16,8 @@\n #include \"column.h\"\n #include \"strbuf.h\"\n\n+const char wt_status_diff_divider[] = \"# -----------------------------------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@@ -791,6 +793,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n \t */\n \tif (s->fp != stdout)\n \t\trev.diffopt.use_color = 0;\n+\tfprintf(s->fp, wt_status_diff_divider);\n+\tfprintf(s->fp, _(\"# The diff below will be removed when keeping the previous line.\\n\"));\n \trun_diff_index(&rev, 1);\n }\n\ndiff --git a/wt-status.h b/wt-status.h\nindex 6c29e6f..cd2709f 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -91,6 +91,8 @@ struct wt_status_state {\n \tunsigned char cherry_pick_head_sha1[20];\n };\n\n+const char wt_status_diff_divider[];\n+\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.1.g537c6ea\n"},{"id":"230719","messageId":"CAPig+cTiwA7wg2eeCcfw8d=-+_QoxGiWVq9sABNdUTCka=5fbg@mail.gmail.com","threadId":"35316","inReplyTo":"5287F735.3030306@web.de","subject":"Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-11-17T00:22:29Z","receivedAt":"2013-11-17T00:22:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Nov 16, 2013 at 5:52 PM, Jens Lehmann wrote:\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. But this approach has two problems: when the commit\n> message itself contains a line starting with \"diff --git\" it will be\n> truncated there prematurely. And when the \"diff.submodule\" setting is set\n> to \"log\", the diff may start with \"Submodule ..\", which will\n> be left in the commit message while it shouldn't.\n>\n> Fix that by introducing a special scissor separator line starting with the\n> comment character '#' followed by a line describing what it is for. The\n> scissor line 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> Turn a known test failure fixed by this change into a successful test and\n> add another one for a diff starting with a submodule log.\n>\n> Reported-by: Ari Pollak \n> Signed-off-by: Jens Lehmann \n> ---\n>\n> builtin/commit.c | 6 +++---\n> t/t7507-commit-verbose.sh | 15 ++++++++++++++-\n> wt-status.c | 4 ++++\n> wt-status.h | 2 ++\n> 4 files changed, 23 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 6ab4605..091a6e7 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>\n> /* Truncate the message just before the diff, if any. */\n> if (verbose) {\n> - p = strstr(sb.buf, \"\\ndiff --git \");\n> - if (p != NULL)\n> - strbuf_setlen(&sb, p - sb.buf + 1);\n> + p = strstr(sb.buf, wt_status_diff_divider);\n\nWould it make sense to use the more flexible is_scissors_line() from\nbuiltin/mailinfo.c here?\n\n> + if ((p != NULL) && (p > sb.buf) && (p[-1] == '\\n'))\n> + strbuf_setlen(&sb, p - sb.buf);\n> }\n>\n> if (cleanup_mode != CLEANUP_NONE)\n> diff --git a/wt-status.c b/wt-status.c\n> index b4e44ba..a3f7115 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -16,6 +16,8 @@\n> #include \"column.h\"\n> #include \"strbuf.h\"\n>\n> +const char wt_status_diff_divider[] = \"# -----------------------------------8<-----------------------------------\\n\";\n> +\n> static char default_wt_status_colors[][COLOR_MAXLEN] = {\n> GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */\n> GIT_COLOR_GREEN, /* WT_STATUS_UPDATED */\n> @@ -791,6 +793,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n> */\n> if (s->fp != stdout)\n> rev.diffopt.use_color = 0;\n> + fprintf(s->fp, wt_status_diff_divider);\n> + fprintf(s->fp, _(\"# The diff below will be removed when keeping the previous line.\\n\"));\n> run_diff_index(&rev, 1);\n> }\n>\n> diff --git a/wt-status.h b/wt-status.h\n> index 6c29e6f..cd2709f 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -91,6 +91,8 @@ struct wt_status_state {\n> unsigned char cherry_pick_head_sha1[20];\n> };\n>\n> +const char wt_status_diff_divider[];\n> +\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> --\n"},{"id":"230724","messageId":"20131117085357.GB17016@sigill.intra.peff.net","threadId":"35316","inReplyTo":"CAPig+cTiwA7wg2eeCcfw8d=-+_QoxGiWVq9sABNdUTCka=5fbg@mail.gmail.com","subject":"Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-17T08:53:57Z","receivedAt":"2013-11-17T08:53:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 16, 2013 at 07:22:29PM -0500, Eric Sunshine wrote:\n\n> > /* Truncate the message just before the diff, if any. */\n> > if (verbose) {\n> > - p = strstr(sb.buf, \"\\ndiff --git \");\n> > - if (p != NULL)\n> > - strbuf_setlen(&sb, p - sb.buf + 1);\n> > + p = strstr(sb.buf, wt_status_diff_divider);\n> \n> Would it make sense to use the more flexible is_scissors_line() from\n> builtin/mailinfo.c here?\n\nI don't think so. We are not trying to be friendly to a remote source\nwhich has given us an arbitrarily-written scissor line. Rather the\nopposite: we are trying to be very strict only to break on the line we\nhave included ourselves.\n\n-Peff\n"},{"id":"230727","messageId":"20131117090935.GC17016@sigill.intra.peff.net","threadId":"35316","inReplyTo":"5287F735.3030306@web.de","subject":"Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-17T09:09:36Z","receivedAt":"2013-11-17T09:09:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 6ab4605..091a6e7 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tp = strstr(sb.buf, wt_status_diff_divider);\n> +\t\tif ((p != NULL) && (p > sb.buf) && (p[-1] == '\\n'))\n> +\t\t\tstrbuf_setlen(&sb, p - sb.buf);\n\nI think your check for a preceding newline is too strict. If I delete\neverything before the scissor line (e.g., because I am trying to abort\nthe commit), we should still remove the diff. With your patch, we do\nnot, and a commit message consisting solely of the diff.\n\nSo I think you want:\n\n if (p && (p == sb.buf || p[-1] == '\\n'))\n\n> +\tfprintf(s->fp, _(\"# The diff below will be removed when keeping the previous line.\\n\"));\n\nI found this hard to parse, I think because of the \"keeping\" (why would\nI not keep it?), and because you are talking about lines above and\nbelow. It is not as accurate to say:\n\n # ------------------ >8 --------------------\n # Everything below this line will be removed.\n\nbecause it is technically the line above that is the cutoff. But I think\nthe meaning is clear, and it is simpler to parse.\n\nI do think it would be simpler with a single line. I know handling the\ni18n was a question there, but I think we should be fine as long as we\ncheck for the exact bytes we wrote. Surely gettext can do something\nlike:\n\n magic = _(\"# Everything below this line will be removed\");\n fprintf(fh, \"%s\", magic);\n ...\n p = strstr(magic);\n\nI don't know what guarantees on string lifetime gettext gives us, but\nthe worst case is that we simply strdup the result.\n\nI suppose it's possible that the translated string could have utf8 with\nmultiple representations, and the user's editor normalizes the text in a\ndifferent way than we wrote it when it saves the result. I don't know if\nthat is worth caring about or not; it seems kind of insane.\n\n-Peff\n"},{"id":"230730","messageId":"5288B49D.8080401@web.de","threadId":"35316","inReplyTo":"20131117090935.GC17016@sigill.intra.peff.net","subject":"Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-17T12:20:45Z","receivedAt":"2013-11-17T12:20:45Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 17.11.2013 10:09, schrieb Jeff King:\n> \n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> index 6ab4605..091a6e7 100644\n>> --- a/builtin/commit.c\n>> +++ b/builtin/commit.c\n>> @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tp = strstr(sb.buf, wt_status_diff_divider);\n>> +\t\tif ((p != NULL) && (p > sb.buf) && (p[-1] == '\\n'))\n>> +\t\t\tstrbuf_setlen(&sb, p - sb.buf);\n> \n> I think your check for a preceding newline is too strict. If I delete\n> everything before the scissor line (e.g., because I am trying to abort\n> the commit), we should still remove the diff. With your patch, we do\n> not, and a commit message consisting solely of the diff.\n> \n> So I think you want:\n> \n> if (p && (p == sb.buf || p[-1] == '\\n'))\n\nThanks for catching this, will do so in v2.\n\n>> +\tfprintf(s->fp, _(\"# The diff below will be removed when keeping the previous line.\\n\"));\n> \n> I found this hard to parse, I think because of the \"keeping\" (why would\n> I not keep it?), and because you are talking about lines above and\n> below. It is not as accurate to say:\n> \n> # ------------------ >8 --------------------\n> # Everything below this line will be removed.\n> \n> because it is technically the line above that is the cutoff. But I think\n> the meaning is clear, and it is simpler to parse.\n\nOk.\n\n> I do think it would be simpler with a single line. I know handling the\n> i18n was a question there, but I think we should be fine as long as we\n> check for the exact bytes we wrote. Surely gettext can do something\n> like:\n> \n> magic = _(\"# Everything below this line will be removed\");\n> fprintf(fh, \"%s\", magic);\n> ...\n> p = strstr(magic);\n> \n> I don't know what guarantees on string lifetime gettext gives us, but\n> the worst case is that we simply strdup the result.\n> \n> I suppose it's possible that the translated string could have utf8 with\n> multiple representations, and the user's editor normalizes the text in a\n> different way than we wrote it when it saves the result. I don't know if\n> that is worth caring about or not; it seems kind of insane.\n\nI don't have any strong feelings about this one. I'd be fine with\ndropping the scissor line and taking the translated string as divider\nline. What do others think?\n"},{"id":"230753","messageId":"xmqqk3g5d7gy.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"20131117090935.GC17016@sigill.intra.peff.net","subject":"Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-18T16:01:49Z","receivedAt":"2013-11-18T16:01:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I found this hard to parse, I think because of the \"keeping\" (why would\n> I not keep it?), and because you are talking about lines above and\n> below. It is not as accurate to say:\n>\n> # ------------------ >8 --------------------\n> # Everything below this line will be removed.\n>\n> because it is technically the line above that is the cutoff. But I think\n> the meaning is clear, and it is simpler to parse.\n>\n> I do think it would be simpler with a single line. I know handling the\n> i18n was a question there, but I think we should be fine as long as we\n> check for the exact bytes we wrote. Surely gettext can do something\n> like:\n>\n> magic = _(\"# Everything below this line will be removed\");\n> fprintf(fh, \"%s\", magic);\n> ...\n> p = strstr(magic);\n>\n> I don't know what guarantees on string lifetime gettext gives us, but\n> the worst case is that we simply strdup the result.\n>\n> I suppose it's possible that the translated string could have utf8 with\n> multiple representations, and the user's editor normalizes the text in a\n> different way than we wrote it when it saves the result. I don't know if\n> that is worth caring about or not; it seems kind of insane.\n\nI agree with your rewording suggestion. It might make it even more\nrobust to do something like\n\n const char cut_here[] = \"# --- cut here --- >8 --- cut here ---\";\n\n fprintf(fh, \"%s\\n\", cut_here);\n fputs(_(\"# Everything below this line will be removed\\n\"), fh);\n ...\n p = strstr(cut_here);\n\ni.e. a real marker line that will never be translated, with an\nexplanation immediately below that can be translated.\n"},{"id":"230799","messageId":"528BB23D.4020001@web.de","threadId":"35316","inReplyTo":"xmqqk3g5d7gy.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-19T18:47:25Z","receivedAt":"2013-11-19T18:47:25Z","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. But this approach has two problems: when the commit\nmessage itself contains a line starting with \"diff --git\" it will be\ntruncated there prematurely. And when the \"diff.submodule\" setting is set\nto \"log\", the diff may start with \"Submodule ..\", which will\nbe left in the commit message while it shouldn't.\n\nFix that by introducing a special scissor separator line starting with the\ncomment character '#' followed by a line describing what it is for. The\nscissor line - which will not be translated - is used to reliably detect\nthe start of the diff so it can be chopped off from the commit message, no\nmatter what the user enters there. Turn a known test failure fixed by this\nchange into a successful test and add another one for a diff starting with\na submodule log.\n\nReported-by: Ari Pollak \nSigned-off-by: Jens Lehmann \n---\n\nAm 18.11.2013 17:01, schrieb Junio C Hamano:\n> Jeff King writes:\n> \n>> I found this hard to parse, I think because of the \"keeping\" (why would\n>> I not keep it?), and because you are talking about lines above and\n>> below. It is not as accurate to say:\n>>\n>> # ------------------ >8 --------------------\n>> # Everything below this line will be removed.\n>>\n>> because it is technically the line above that is the cutoff. But I think\n>> the meaning is clear, and it is simpler to parse.\n\nOk, changed in this version.\n\n> I agree with your rewording suggestion. It might make it even more\n> robust to do something like\n> \n> const char cut_here[] = \"# --- cut here --- >8 --- cut here ---\";\n> \n> fprintf(fh, \"%s\\n\", cut_here);\n> fputs(_(\"# Everything below this line will be removed\\n\"), fh);\n> ...\n> p = strstr(cut_here);\n> \n> i.e. a real marker line that will never be translated, with an\n> explanation immediately below that can be translated.\n\nWhich is what my last version already did :-) But it didn't mention\nthat in the commit message, which it does now).\n\nAlso the too strict check for the scissor line is fixed according to\nPeff's proposal, it will now handle an empty commit message correctly\ntoo.\n\n\n builtin/commit.c | 6 +++---\n t/t7507-commit-verbose.sh | 15 ++++++++++++++-\n wt-status.c | 4 ++++\n wt-status.h | 2 ++\n 4 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..b6d6655 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tp = strstr(sb.buf, wt_status_diff_divider);\n+\t\tif (p && (p == sb.buf || p[-1] == '\\n'))\n+\t\t\tstrbuf_setlen(&sb, p - sb.buf);\n \t}\n\n \tif (cleanup_mode != CLEANUP_NONE)\ndiff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\nindex da5bd3b..09c1150 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -65,9 +65,22 @@ 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+\tGIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&\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..a499bd2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -16,6 +16,8 @@\n #include \"column.h\"\n #include \"strbuf.h\"\n\n+const char wt_status_diff_divider[] = \"# -----------------------------------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@@ -791,6 +793,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n \t */\n \tif (s->fp != stdout)\n \t\trev.diffopt.use_color = 0;\n+\tfprintf(s->fp, wt_status_diff_divider);\n+\tfprintf(s->fp, _(\"# Everything below this line will be removed.\\n\"));\n \trun_diff_index(&rev, 1);\n }\n\ndiff --git a/wt-status.h b/wt-status.h\nindex 6c29e6f..cd2709f 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -91,6 +91,8 @@ struct wt_status_state {\n \tunsigned char cherry_pick_head_sha1[20];\n };\n\n+const char wt_status_diff_divider[];\n+\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.gb1d0fd2\n"},{"id":"230800","messageId":"xmqqsius8b39.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"528BB23D.4020001@web.de","subject":"Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-19T19:07:06Z","receivedAt":"2013-11-19T19:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann 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. But this approach has two problems: when the commit\n> message itself contains a line starting with \"diff --git\" it will be\n> truncated there prematurely. And when the \"diff.submodule\" setting is set\n> to \"log\", the diff may start with \"Submodule ..\", which will\n> be left in the commit message while it shouldn't.\n>\n> Fix that by introducing a special scissor separator line starting with the\n> comment character '#' followed by a line describing what it is for. The\n> scissor line - which will not be translated - is used to reliably detect\n> the start of the diff so it can be chopped off from the commit message, no\n> matter what the user enters there. Turn a known test failure fixed by this\n> change into a successful test and add another one for a diff starting with\n> a submodule log.\n>\n> Reported-by: Ari Pollak \n> Signed-off-by: Jens Lehmann \n> ---\n>\n> Am 18.11.2013 17:01, schrieb Junio C Hamano:\n>> Jeff King writes:\n>> \n>>> I found this hard to parse, I think because of the \"keeping\" (why would\n>>> I not keep it?), and because you are talking about lines above and\n>>> below. It is not as accurate to say:\n>>>\n>>> # ------------------ >8 --------------------\n>>> # Everything below this line will be removed.\n>>>\n>>> because it is technically the line above that is the cutoff. But I think\n>>> the meaning is clear, and it is simpler to parse.\n>\n> Ok, changed in this version.\n>\n>> I agree with your rewording suggestion. It might make it even more\n>> robust to do something like\n>> \n>> const char cut_here[] = \"# --- cut here --- >8 --- cut here ---\";\n>> \n>> fprintf(fh, \"%s\\n\", cut_here);\n>> fputs(_(\"# Everything below this line will be removed\\n\"), fh);\n>> ...\n>> p = strstr(cut_here);\n>> \n>> i.e. a real marker line that will never be translated, with an\n>> explanation immediately below that can be translated.\n>\n> Which is what my last version already did :-) But it didn't mention\n> that in the commit message, which it does now).\n>\n> Also the too strict check for the scissor line is fixed according to\n> Peff's proposal, it will now handle an empty commit message correctly\n> too.\n\nThanks.\n\nI have a feeling that the translatable message has to say two\nthings, though.\n\n - Any junk below that cut marker will be removed;\n\n - Do not touch the cut marker, or you will mess up the commit log\n message.\n\nNobody seems to have brought up the latter in the discussion, but I\nhave a feeling that the users can do almost anything to break us and\nit is better to be defensive for the users.\n\nPerhaps phrase it something like this (the message is to be\ntranslated, but not the marker)?\n\n\t# ------------------------ >8 ------------------------\n # Do not touch the cut-marker line above; everything below\n # will be removed.\n\n>\n> builtin/commit.c | 6 +++---\n> t/t7507-commit-verbose.sh | 15 ++++++++++++++-\n> wt-status.c | 4 ++++\n> wt-status.h | 2 ++\n> 4 files changed, 23 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 6ab4605..b6d6655 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\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\tp = strstr(sb.buf, wt_status_diff_divider);\n> +\t\tif (p && (p == sb.buf || p[-1] == '\\n'))\n> +\t\t\tstrbuf_setlen(&sb, p - sb.buf);\n> \t}\n>\n> \tif (cleanup_mode != CLEANUP_NONE)\n> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh\n> index da5bd3b..09c1150 100755\n> --- a/t/t7507-commit-verbose.sh\n> +++ b/t/t7507-commit-verbose.sh\n> @@ -65,9 +65,22 @@ 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> +\tGIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&\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..a499bd2 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -16,6 +16,8 @@\n> #include \"column.h\"\n> #include \"strbuf.h\"\n>\n> +const char wt_status_diff_divider[] = \"# -----------------------------------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> @@ -791,6 +793,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n> \t */\n> \tif (s->fp != stdout)\n> \t\trev.diffopt.use_color = 0;\n> +\tfprintf(s->fp, wt_status_diff_divider);\n> +\tfprintf(s->fp, _(\"# Everything below this line will be removed.\\n\"));\n> \trun_diff_index(&rev, 1);\n> }\n>\n> diff --git a/wt-status.h b/wt-status.h\n> index 6c29e6f..cd2709f 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -91,6 +91,8 @@ struct wt_status_state {\n> \tunsigned char cherry_pick_head_sha1[20];\n> };\n>\n> +const char wt_status_diff_divider[];\n> +\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":"230802","messageId":"xmqqob5g89y0.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"528BB23D.4020001@web.de","subject":"Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-19T19:31:51Z","receivedAt":"2013-11-19T19:31:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> diff --git a/wt-status.h b/wt-status.h\n> index 6c29e6f..cd2709f 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -91,6 +91,8 @@ struct wt_status_state {\n> \tunsigned char cherry_pick_head_sha1[20];\n> };\n>\n> +const char wt_status_diff_divider[];\n\nThis gives me:\n\n./wt-status.h:94:12: error: array 'wt_status_diff_divider' assumed to have one element [-Werror]\ncc1: all warnings being treated as errors\n\nwhich is a bit unfortunate.\n\nRegardless of that, from the API design standpoint, I think it may\nbe much better not to expose this particular implementation element\n(i.e. the array) to the caller, but instead to export a helper\nfunction that takes a pointer to a piece of memory and let callers\nask an \"I have this line---is it the status cut mark?\" question.\n"},{"id":"230803","messageId":"xmqqk3g489g4.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"xmqqob5g89y0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-19T19:42:35Z","receivedAt":"2013-11-19T19:42:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jens Lehmann writes:\n>\n>> diff --git a/wt-status.h b/wt-status.h\n>> index 6c29e6f..cd2709f 100644\n>> --- a/wt-status.h\n>> +++ b/wt-status.h\n>> @@ -91,6 +91,8 @@ struct wt_status_state {\n>> \tunsigned char cherry_pick_head_sha1[20];\n>> };\n>>\n>> +const char wt_status_diff_divider[];\n>\n> This gives me:\n>\n> ./wt-status.h:94:12: error: array 'wt_status_diff_divider' assumed to have one element [-Werror]\n> cc1: all warnings being treated as errors\n>\n> which is a bit unfortunate.\n>\n> Regardless of that, from the API design standpoint, I think it may\n> be much better not to expose this particular implementation element\n> (i.e. the array) to the caller, but instead to export a helper\n> function that takes a pointer to a piece of memory and let callers\n> ask an \"I have this line---is it the status cut mark?\" question.\n\nThat is, something like this, perhaps.\n\n builtin/commit.c | 9 +++------\n t/t7507-commit-verbose.sh | 15 ++++++++++++++-\n wt-status.c | 15 +++++++++++++++\n wt-status.h | 1 +\n 4 files changed, 33 insertions(+), 7 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..09c1150 100755\n--- a/t/t7507-commit-verbose.sh\n+++ b/t/t7507-commit-verbose.sh\n@@ -65,9 +65,22 @@ 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+\tGIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&\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..492506a 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 const char wt_status_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,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@@ -791,6 +803,9 @@ static void wt_status_print_verbose(struct wt_status *s)\n \t */\n \tif (s->fp != stdout)\n \t\trev.diffopt.use_color = 0;\n+\tfprintf(s->fp, wt_status_cut_line);\n+\tfprintf(s->fp, _(\"# Do not touch the line above.\\n\"));\n+\tfprintf(s->fp, _(\"# Everything below will be removed.\\n\"));\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"},{"id":"230804","messageId":"xmqqd2lw871i.fsf@gitster.dls.corp.google.com","threadId":"35316","inReplyTo":"528BB23D.4020001@web.de","subject":"Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-19T20:34:33Z","receivedAt":"2013-11-19T20:34:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Am 18.11.2013 17:01, schrieb Junio C Hamano:\n>> Jeff King writes:\n>> \n>>> I found this hard to parse, I think because of the \"keeping\" (why would\n>>> I not keep it?), and because you are talking about lines above and\n>>> below. It is not as accurate to say:\n>>>\n>>> # ------------------ >8 --------------------\n>>> # Everything below this line will be removed.\n>>>\n>>> because it is technically the line above that is the cutoff. But I think\n>>> the meaning is clear, and it is simpler to parse.\n>\n> Ok, changed in this version.\n>\n>> I agree with your rewording suggestion....\n>\n> Which is what my last version already did :-) But it didn't mention\n> that in the commit message, which it does now).\n\nOh, another thing. Does this interact with the core.commentChar in\nany way, and if so how?\n"},{"id":"230805","messageId":"528BD1A4.4060903@web.de","threadId":"35316","inReplyTo":"xmqqd2lw871i.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 ] commit -v: strip diffs and submodule shortlogs from the commit message","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-19T21:01:24Z","receivedAt":"2013-11-19T21:01:24Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 19.11.2013 21:34, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> Am 18.11.2013 17:01, schrieb Junio C Hamano:\n>>> Jeff King writes:\n>>>\n>>>> I found this hard to parse, I think because of the \"keeping\" (why would\n>>>> I not keep it?), and because you are talking about lines above and\n>>>> below. It is not as accurate to say:\n>>>>\n>>>> # ------------------ >8 --------------------\n>>>> # Everything below this line will be removed.\n>>>>\n>>>> because it is technically the line above that is the cutoff. But I think\n>>>> the meaning is clear, and it is simpler to parse.\n>>\n>> Ok, changed in this version.\n>>\n>>> I agree with your rewording suggestion....\n>>\n>> Which is what my last version already did :-) But it didn't mention\n>> that in the commit message, which it does now).\n> \n> Oh, another thing. Does this interact with the core.commentChar in\n> any way, and if so how?\n\nIf I understand that setting correctly the hardcoded '#' characters\nmust be replaced with the core.commentChar when set. Will tackle that\nin the next iteration.\n"}]}