{"thread":{"id":"39554","subject":"[PATCH] commit: cope with scissors lines in commit message","startedAt":"2015-06-08T01:40:39Z","lastAt":"2015-06-09T00:28:34Z","messageCount":5,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"263166","messageId":"1433727639-5927-1-git-send-email-szeder@ira.uka.de","threadId":"39554","inReplyTo":null,"subject":"[PATCH] commit: cope with scissors lines in commit message","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-06-08T01:40:39Z","receivedAt":"2015-06-08T01:40:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The diff and submodule shortlog appended to the commit message template\nby 'git commit --verbose' are not stripped when the commit message\ncontains an indented scissors line.\n\nWhen cleaning up a commit message with 'git commit --verbose' or\n'--cleanup=scissors' the code is careful and triggers only on a pure\nscissors line, i.e. a line containing nothing but a comment character, a\nspace, and the scissors cut.  This is good, because people can embed\nscissor lines in the commit message while using 'git commit --verbose',\nand the text they write after their indented scissors line doesn't get\ndeleted.\n\nWhile doing so, however, the cleanup function only looks at the first\nline matching the scissors pattern and if it doesn't start at the\nbeginning of the line, then the function just returns without performing\nany cleanup.  This is bad, because a \"real\" scissors line added by 'git\ncommit --verbose' might follow, and in that case the diff and submodule\nshortlog get included in the commit message.\n\nDon't bail out if a scissors line doesn't start at the beginning of the\nline, but keep looking for a non-indented scissors line to fix this.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n t/t7502-commit.sh | 25 +++++++++++++++++++++++++\n wt-status.c       | 12 ++++++++----\n 2 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 2e0d557243..77db3a31c3 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -239,6 +239,31 @@ EOF\n \n '\n \n+test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors line in commit message)' '\n+\techo >>negative &&\n+\tcat >text <<EOF &&\n+\n+  # to be kept\n+\n+  # ------------------------ >8 ------------------------\n+  # to be kept, too\n+# ------------------------ >8 ------------------------\n+to be removed\n+# ------------------------ >8 ------------------------\n+to be removed, too\n+EOF\n+\n+\tcat >expect <<EOF &&\n+  # to be kept\n+\n+  # ------------------------ >8 ------------------------\n+  # to be kept, too\n+EOF\n+\tgit commit --cleanup=scissors -e -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'cleanup commit messages (strip option,-F)' '\n \n \techo >>negative &&\ndiff --git a/wt-status.c b/wt-status.c\nindex c56c78fb6f..e6d171a0cb 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -822,13 +822,17 @@ conclude:\n \n void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n {\n-\tconst char *p;\n+\tconst char *p = buf->buf;\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+\twhile ((p = strstr(p, pattern.buf))) {\n+\t\tif (p == buf->buf || p[-1] == '\\n') {\n+\t\t\tstrbuf_setlen(buf, p - buf->buf);\n+\t\t\tbreak;\n+\t\t}\n+\t\tp++;\n+\t}\n \tstrbuf_release(&pattern);\n }\n \n-- \n2.4.2.423.gad3a03f\n"},{"id":"263229","messageId":"xmqqoakqciv1.fsf@gitster.dls.corp.google.com","threadId":"39554","inReplyTo":"1433727639-5927-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] commit: cope with scissors lines in commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T15:36:34Z","receivedAt":"2015-06-08T15:36:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n> The diff and submodule shortlog appended to the commit message template\n> by 'git commit --verbose' are not stripped when the commit message\n> contains an indented scissors line.\n>\n> When cleaning up a commit message with 'git commit --verbose' or\n> '--cleanup=scissors' the code is careful and triggers only on a pure\n> scissors line, i.e. a line containing nothing but a comment character, a\n> space, and the scissors cut.  This is good, because people can embed\n> scissor lines in the commit message while using 'git commit --verbose',\n> and the text they write after their indented scissors line doesn't get\n> deleted.\n>\n> While doing so, however, the cleanup function only looks at the first\n> line matching the scissors pattern and if it doesn't start at the\n> beginning of the line, then the function just returns without performing\n> any cleanup.  This is bad, because a \"real\" scissors line added by 'git\n> commit --verbose' might follow, and in that case the diff and submodule\n> shortlog get included in the commit message.\n\nYikes; this is not just \"bad\" but simply \"wrong\".  Thanks for\nnoticing.\n\n>  void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n>  {\n> -\tconst char *p;\n> +\tconst char *p = buf->buf;\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> +\twhile ((p = strstr(p, pattern.buf))) {\n> +\t\tif (p == buf->buf || p[-1] == '\\n') {\n> +\t\t\tstrbuf_setlen(buf, p - buf->buf);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tp++;\n> +\t}\n\nI however wonder if we should make strstr() do more work for us.\n\n\tstrbuf_addf(&pattern, \"\\n%c %s\", comment_line_char, cut_line);\n\tif (starts_with(buf->buf, pattern.buf + 1))\n\t\tstrbuf_setlen(buf, 0);\n\telse if ((p = strstr(buf->buf, pattern.buf)) != NULL)\n        \tstrbuf_setlen(buf, p - buf->buf + 1);\n\tstrbuf_release(&pattern);\n\nperhaps?\n"},{"id":"263246","messageId":"20150608183019.Horde.Kp4b2pClPLqAJp3LacT_fA1@webmail.informatik.kit.edu","threadId":"39554","inReplyTo":"xmqqoakqciv1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] commit: cope with scissors lines in commit message","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-06-08T16:30:19Z","receivedAt":"2015-06-08T16:30:19Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Junio C Hamano <gitster@pobox.com>:\n\n> SZEDER Gábor <szeder@ira.uka.de> writes:\n>\n>> The diff and submodule shortlog appended to the commit message template\n>> by 'git commit --verbose' are not stripped when the commit message\n>> contains an indented scissors line.\n>>\n>> When cleaning up a commit message with 'git commit --verbose' or\n>> '--cleanup=scissors' the code is careful and triggers only on a pure\n>> scissors line, i.e. a line containing nothing but a comment character, a\n>> space, and the scissors cut.  This is good, because people can embed\n>> scissor lines in the commit message while using 'git commit --verbose',\n>> and the text they write after their indented scissors line doesn't get\n>> deleted.\n>>\n>> While doing so, however, the cleanup function only looks at the first\n>> line matching the scissors pattern and if it doesn't start at the\n>> beginning of the line, then the function just returns without performing\n>> any cleanup.  This is bad, because a \"real\" scissors line added by 'git\n>> commit --verbose' might follow, and in that case the diff and submodule\n>> shortlog get included in the commit message.\n>\n> Yikes; this is not just \"bad\" but simply \"wrong\".  Thanks for\n> noticing.\n\nGreat, the other day I scored a \"Gaaah\" from Peff, now a \"Yikes\" from  \nyou...  Doin' good! ;)\n\n>>  void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\n>>  {\n>> -\tconst char *p;\n>> +\tconst char *p = buf->buf;\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>> +\twhile ((p = strstr(p, pattern.buf))) {\n>> +\t\tif (p == buf->buf || p[-1] == '\\n') {\n>> +\t\t\tstrbuf_setlen(buf, p - buf->buf);\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\tp++;\n>> +\t}\n>\n> I however wonder if we should make strstr() do more work for us.\n>\n> \tstrbuf_addf(&pattern, \"\\n%c %s\", comment_line_char, cut_line);\n> \tif (starts_with(buf->buf, pattern.buf + 1))\n> \t\tstrbuf_setlen(buf, 0);\n> \telse if ((p = strstr(buf->buf, pattern.buf)) != NULL)\n>         \tstrbuf_setlen(buf, p - buf->buf + 1);\n> \tstrbuf_release(&pattern);\n>\n> perhaps?\n\nThough this has one more if statement, it doesn't add a loop and  \neliminates the if (... || ...).\nGood, will try to reroll in the evening.\n\nGábor\n"},{"id":"263321","messageId":"1433809461-9823-1-git-send-email-szeder@ira.uka.de","threadId":"39554","inReplyTo":"20150608183019.Horde.Kp4b2pClPLqAJp3LacT_fA1@webmail.informatik.kit.edu","subject":"[PATCH v2] commit: cope with scissors lines in commit message","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-06-09T00:24:21Z","receivedAt":"2015-06-09T00:24:21Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The diff and submodule shortlog appended to the commit message template\nby 'git commit --verbose' are not stripped when the commit message\ncontains an indented scissors line.\n\nWhen cleaning up a commit message with 'git commit --verbose' or\n'--cleanup=scissors' the code is careful and triggers only on a pure\nscissors line, i.e. a line containing nothing but a comment character, a\nspace, and the scissors cut.  This is good, because people can embed\nscissor lines in the commit message while using 'git commit --verbose',\nand the text they write after their indented scissors line doesn't get\ndeleted.\n\nWhile doing so, however, the cleanup function only looks at the first\nline matching the scissors pattern and if it doesn't start at the\nbeginning of the line, then the function just returns without performing\nany cleanup.  This is wrong, because a \"real\" scissors line added by\n'git commit --verbose' might follow, and in that case the diff and\nsubmodule shortlog get included in the commit message.\n\nFix this by changing the scissors pattern to match only at the beginning\nof the line, yet be careful to catch scissors on the first line as well.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\nChanges besides incorporating Junio's suggestion and updating the commit\nmessage accordingly:\n\n * Instead of adding a new test, modify the existing one to check\n   handling indented scissors lines.\n * Add a test to check scissors on the first line\n \n t/t7502-commit.sh | 24 +++++++++++++++++++++++-\n wt-status.c       |  9 +++++----\n 2 files changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 2e0d557243..b39e313ac2 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' '\n \tcat >text <<EOF &&\n \n # to be kept\n+\n+  # ------------------------ >8 ------------------------\n+# to be kept, too\n # ------------------------ >8 ------------------------\n to be removed\n+# ------------------------ >8 ------------------------\n+to be removed, too\n+EOF\n+\n+\tcat >expect <<EOF &&\n+# to be kept\n+\n+  # ------------------------ >8 ------------------------\n+# to be kept, too\n EOF\n-\techo \"# to be kept\" >expect &&\n \tgit commit --cleanup=scissors -e -F text -a &&\n \tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n \ttest_cmp expect actual\n+'\n \n+test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' '\n+\n+\techo >>negative &&\n+\tcat >text <<EOF &&\n+# ------------------------ >8 ------------------------\n+to be removed\n+EOF\n+\tgit commit --cleanup=scissors -e -F text -a --allow-empty-message &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\ttest_must_be_empty actual\n '\n \n test_expect_success 'cleanup commit messages (strip option,-F)' '\ndiff --git a/wt-status.c b/wt-status.c\nindex c56c78fb6f..eaed4fed32 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\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_addf(&pattern, \"\\n%c %s\", comment_line_char, cut_line);\n+\tif (starts_with(buf->buf, pattern.buf + 1))\n+\t\tstrbuf_setlen(buf, 0);\n+\telse if ((p = strstr(buf->buf, pattern.buf)))\n+\t\tstrbuf_setlen(buf, p - buf->buf + 1);\n \tstrbuf_release(&pattern);\n }\n \n-- \n2.4.3.384.g605df7b\n"},{"id":"263322","messageId":"1433809714-10865-1-git-send-email-szeder@ira.uka.de","threadId":"39554","inReplyTo":"1433809461-9823-1-git-send-email-szeder@ira.uka.de","subject":"[PATCH v3] commit: cope with scissors lines in commit message","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-06-09T00:28:34Z","receivedAt":"2015-06-09T00:28:34Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The diff and submodule shortlog appended to the commit message template\nby 'git commit --verbose' are not stripped when the commit message\ncontains an indented scissors line.\n\nWhen cleaning up a commit message with 'git commit --verbose' or\n'--cleanup=scissors' the code is careful and triggers only on a pure\nscissors line, i.e. a line containing nothing but a comment character, a\nspace, and the scissors cut.  This is good, because people can embed\nscissors lines in the commit message while using 'git commit --verbose',\nand the text they write after their indented scissors line doesn't get\ndeleted.\n\nWhile doing so, however, the cleanup function only looks at the first\nline matching the scissors pattern and if it doesn't start at the\nbeginning of the line, then the function just returns without performing\nany cleanup.  This is wrong, because a \"real\" scissors line added by\n'git commit --verbose' might follow, and in that case the diff and\nsubmodule shortlog get included in the commit message.\n\nFix this by changing the scissors pattern to match only at the beginning\nof the line, yet be careful to catch scissors on the first line as well.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\nFixed a typo in the commit message.\n \n t/t7502-commit.sh | 24 +++++++++++++++++++++++-\n wt-status.c       |  9 +++++----\n 2 files changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 2e0d557243..b39e313ac2 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' '\n \tcat >text <<EOF &&\n \n # to be kept\n+\n+  # ------------------------ >8 ------------------------\n+# to be kept, too\n # ------------------------ >8 ------------------------\n to be removed\n+# ------------------------ >8 ------------------------\n+to be removed, too\n+EOF\n+\n+\tcat >expect <<EOF &&\n+# to be kept\n+\n+  # ------------------------ >8 ------------------------\n+# to be kept, too\n EOF\n-\techo \"# to be kept\" >expect &&\n \tgit commit --cleanup=scissors -e -F text -a &&\n \tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n \ttest_cmp expect actual\n+'\n \n+test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' '\n+\n+\techo >>negative &&\n+\tcat >text <<EOF &&\n+# ------------------------ >8 ------------------------\n+to be removed\n+EOF\n+\tgit commit --cleanup=scissors -e -F text -a --allow-empty-message &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\ttest_must_be_empty actual\n '\n \n test_expect_success 'cleanup commit messages (strip option,-F)' '\ndiff --git a/wt-status.c b/wt-status.c\nindex c56c78fb6f..eaed4fed32 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf)\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_addf(&pattern, \"\\n%c %s\", comment_line_char, cut_line);\n+\tif (starts_with(buf->buf, pattern.buf + 1))\n+\t\tstrbuf_setlen(buf, 0);\n+\telse if ((p = strstr(buf->buf, pattern.buf)))\n+\t\tstrbuf_setlen(buf, p - buf->buf + 1);\n \tstrbuf_release(&pattern);\n }\n \n-- \n2.4.3.384.g605df7b\n"}]}