{"thread":{"id":"21392","subject":"[PATCH] commit: More generous accepting of RFC-2822 footer lines.","startedAt":"2009-10-27T23:45:20Z","lastAt":"2009-11-04T15:11:14Z","messageCount":11,"participants":["David Brown","Shawn O. Pearce","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"126069","messageId":"20091027234520.GA11433@quaoar.codeaurora.org","threadId":"21392","inReplyTo":null,"subject":"[PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"David Brown","fromEmail":"davidb@codeaurora.org","sentAt":"2009-10-27T23:45:20Z","receivedAt":"2009-10-27T23:45:20Z","isPatch":true,"sender":{"key":"davidb@codeaurora.org","avatar":"https://gravatar.com/avatar/1bacedec21621bd4efa4bc8ecb05c507f0cf0641ecdaa50943c2ed21c8ef22d8?d=mp&s=160"},"body":"From: David Brown \n\n'git commit -s' will insert a blank line before the Signed-off-by\nline at the end of the message, unless this last line is a\nSigned-off-by line itself. Common use has other trailing lines\nat the ends of commit text, in the style of RFC2822 headers.\n\nBe more generous in considering lines to be part of this footer.\nThis may occasionally leave out the blank line for cases where\nthe commit text happens to start with a word ending in a colon,\nbut this results in less fixups than the extra blank lines with\nAcked-by, or other custom footers.\n\nSigned-off-by: David Brown \n---\n builtin-commit.c | 17 ++++++++++++++++-\n t/t7501-commit.sh | 19 +++++++++++++++++++\n 2 files changed, 35 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 200ffda..f081e80 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -414,6 +414,21 @@ static void determine_author_info(void)\n \tauthor_date = date;\n }\n \n+static int is_rfc2822_footer(const char *line)\n+{\n+\tint ch;\n+\n+\twhile ((ch = *line++)) {\n+\t\tif (ch == ':')\n+\t\t\treturn 1;\n+\t\tif ((33 <= ch && ch <= 57) ||\n+\t\t (59 <= ch && ch <= 126))\n+\t\t\tcontinue;\n+\t\tbreak;\n+\t}\n+\treturn 0;\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct wt_status *s)\n {\n@@ -489,7 +504,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n \t\t\t; /* do nothing */\n \t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (prefixcmp(sb.buf + i, sign_off_header))\n+\t\t\tif (!is_rfc2822_footer(sb.buf + i))\n \t\t\t\tstrbuf_addch(&sb, '\\n');\n \t\t\tstrbuf_addbuf(&sb, &sob);\n \t\t}\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex e2ef532..05542b4 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -247,6 +247,25 @@ $existing\" &&\n \n '\n \n+test_expect_success 'signoff gap' '\n+\n+\techo 3 >positive &&\n+\tgit add positive &&\n+\talt=\"Alt-RFC-822-Header: Value\" &&\n+\tgit commit -s -m \"welcome\n+\n+$alt\" &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\t(\n+\t\techo welcome\n+\t\techo\n+\t\techo $alt\n+\t\tgit var GIT_COMMITTER_IDENT |\n+\t\tsed -e \"s/>.*/>/\" -e \"s/^/Signed-off-by: /\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'multiple -m' '\n \n \t>negative &&\n-- \n1.6.5.1\n"},{"id":"126071","messageId":"20091028000511.GK10505@spearce.org","threadId":"21392","inReplyTo":"20091027234520.GA11433@quaoar.codeaurora.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-10-28T00:05:11Z","receivedAt":"2009-10-28T00:05:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"David Brown wrote:\n> From: David Brown \n> \n> 'git commit -s' will insert a blank line before the Signed-off-by\n> line at the end of the message, unless this last line is a\n> Signed-off-by line itself. Common use has other trailing lines\n> at the ends of commit text, in the style of RFC2822 headers.\n> \n> Be more generous in considering lines to be part of this footer.\n> This may occasionally leave out the blank line for cases where\n> the commit text happens to start with a word ending in a colon,\n> but this results in less fixups than the extra blank lines with\n> Acked-by, or other custom footers.\n\nThe nasty perl I use in Gerrit's commit-msg hook is a bit more\nexpressive. Basically the rule is we insert a blank line before\nthe new footer unless all lines in the last paragraph (so all text\nafter the last \"\\n\\n\" sequence) match the regex \"^[a-zA-Z0-9-]+:\".\n \n> +test_expect_success 'signoff gap' '\n> +\n> +\techo 3 >positive &&\n> +\tgit add positive &&\n> +\talt=\"Alt-RFC-822-Header: Value\" &&\n> +\tgit commit -s -m \"welcome\n> +\n> +$alt\" &&\n\nI wonder if we shouldn't also have a test case for the message:\n\n\tmsg=\"test\n\nthis is a test that\nfixes: 42.\n\"\n\nas the result would be expected to be:\n\n\texp=\"test\n\nthis is a test that\nfixes: 42.\n\nSigned-off-by A. U. Thor <...>\n\"\n\nBut:\n\n\tmsg=\"test\n\nthis is a test\n\nfixes: 42\n\"\n\nwould produce:\n\n\texp=\"test\n\nthis is a test\n\nfixes: 42\nSigned-off-by A. U. Thor <...>\n\"\n\n-- \nShawn.\n"},{"id":"126091","messageId":"7vk4yguh00.fsf@alter.siamese.dyndns.org","threadId":"21392","inReplyTo":"20091028000511.GK10505@spearce.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-28T07:14:55Z","receivedAt":"2009-10-28T07:14:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> David Brown wrote:\n>> From: David Brown \n>> \n>> 'git commit -s' will insert a blank line before the Signed-off-by\n>> line at the end of the message, unless this last line is a\n>> Signed-off-by line itself. Common use has other trailing lines\n>> at the ends of commit text, in the style of RFC2822 headers.\n>> \n>> Be more generous in considering lines to be part of this footer.\n>> This may occasionally leave out the blank line for cases where\n>> the commit text happens to start with a word ending in a colon,\n>> but this results in less fixups than the extra blank lines with\n>> Acked-by, or other custom footers.\n>\n> The nasty perl I use in Gerrit's commit-msg hook is a bit more\n> expressive. Basically the rule is we insert a blank line before\n> the new footer unless all lines in the last paragraph (so all text\n> after the last \"\\n\\n\" sequence) match the regex \"^[a-zA-Z0-9-]+:\".\n\nTogether with your suggestion for tests, the above makes quite a lot of\nsense to me.\n\nThere is one thing to be careful about.\n\nWhen deciding to omit adding a new S-o-b, we deliberately check only the\nlast S-o-b to see if it matches what we are trying to add. This is so\nthat a message from you, that has my patch that was reviewed and touched\nup by you with your sign-off, i.e.\n\n\tS-o-b: Junio\n S-o-b: Shawn\n\nwill not be prevented to have another sign-off by me, so that I can\ncertify that I know that your change I received from you in the patch is\nkosher. IOW, this is not a \"duplicate\" check. The order of S-o-b:\nmatters as it records the flow of the patch.\n"},{"id":"126143","messageId":"20091028142328.GA13343@huya.quicinc.com","threadId":"21392","inReplyTo":"7vk4yguh00.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"David Brown","fromEmail":"davidb@quicinc.com","sentAt":"2009-10-28T14:23:28Z","receivedAt":"2009-10-28T14:23:28Z","isPatch":true,"sender":{"key":"git@davidb.org","avatar":"https://gravatar.com/avatar/94c86a2938470a74c2eac5e2b69afc0871f79a660295c02219597aba8cb101c1?d=mp&s=160"},"body":"On Wed, Oct 28, 2009 at 12:14:55AM -0700, Junio C Hamano wrote:\n\n> When deciding to omit adding a new S-o-b, we deliberately check only the\n> last S-o-b to see if it matches what we are trying to add. This is so\n> that a message from you, that has my patch that was reviewed and touched\n> up by you with your sign-off, i.e.\n\nThis is good to know. I'll leave the existing last-SoB test in\nplace then, and just use the sophisticated check for a block of\nRFC2822 footers to determine if there should be a blank line.\n\nJeff also pointed out that I should probably also allow lines\nstarting with whitespace to be considered header lines.\n\nDavid\n"},{"id":"126160","messageId":"20091028171344.GA22290@quaoar.codeaurora.org","threadId":"21392","inReplyTo":"20091027234520.GA11433@quaoar.codeaurora.org","subject":"[PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"David Brown","fromEmail":"davidb@codeaurora.org","sentAt":"2009-10-28T17:13:44Z","receivedAt":"2009-10-28T17:13:44Z","isPatch":true,"sender":{"key":"davidb@codeaurora.org","avatar":"https://gravatar.com/avatar/1bacedec21621bd4efa4bc8ecb05c507f0cf0641ecdaa50943c2ed21c8ef22d8?d=mp&s=160"},"body":"From: David Brown \n\n'git commit -s' will insert a blank line before the Signed-off-by\nline at the end of the message, unless this last line is a\nSigned-off-by line itself. Common use has other trailing lines\nat the ends of commit text, in the style of RFC2822 headers.\n\nBe more generous in considering lines to be part of this footer.\nIf the last paragraph of the commit message reasonably resembles\nRFC-2822 formatted lines, don't insert that blank line.\n\nThe new Signed-off-by line is still only suppressed when the\nauthor's existing Signed-off-by is the last line of the message.\n\nSigned-off-by: David Brown \n---\n builtin-commit.c | 43 ++++++++++++++++++++++++++++++++++++++++++-\n t/t7501-commit.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 83 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 200ffda..c395cbf 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -414,6 +414,47 @@ static void determine_author_info(void)\n \tauthor_date = date;\n }\n \n+static int ends_rfc2822_footer(struct strbuf *sb)\n+{\n+\tint ch;\n+\tint hit = 0;\n+\tint i, j, k;\n+\tint len = sb->len;\n+\tint first = 1;\n+\tconst char *buf = sb->buf;\n+\n+\tfor (i = len - 1; i > 0; i--) {\n+\t\tif (hit && buf[i] == '\\n')\n+\t\t\tbreak;\n+\t\thit = (buf[i] == '\\n');\n+\t}\n+\n+\twhile (i < len - 1 && buf[i] == '\\n')\n+\t\ti++;\n+\n+\tfor (; i < len; i = k) {\n+\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n+\t\t\t; /* do nothing */\n+\t\tk++;\n+\n+\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\t\tcontinue;\n+\n+\t\tfirst = 0;\n+\n+\t\tfor (j = 0; i + j < len; j++) {\n+\t\t\tch = buf[i + j];\n+\t\t\tif (ch == ':')\n+\t\t\t\tbreak;\n+\t\t\tif (isalnum(ch) ||\n+\t\t\t (ch == '-'))\n+\t\t\t\tcontinue;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t struct wt_status *s)\n {\n@@ -489,7 +530,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n \t\t\t; /* do nothing */\n \t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (prefixcmp(sb.buf + i, sign_off_header))\n+\t\t\tif (!ends_rfc2822_footer(&sb))\n \t\t\t\tstrbuf_addch(&sb, '\\n');\n \t\t\tstrbuf_addbuf(&sb, &sob);\n \t\t}\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex e2ef532..d2de576 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -247,6 +247,47 @@ $existing\" &&\n \n '\n \n+test_expect_success 'signoff gap' '\n+\n+\techo 3 >positive &&\n+\tgit add positive &&\n+\talt=\"Alt-RFC-822-Header: Value\" &&\n+\tgit commit -s -m \"welcome\n+\n+$alt\" &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\t(\n+\t\techo welcome\n+\t\techo\n+\t\techo $alt\n+\t\tgit var GIT_COMMITTER_IDENT |\n+\t\tsed -e \"s/>.*/>/\" -e \"s/^/Signed-off-by: /\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'signoff gap 2' '\n+\n+\techo 4 >positive &&\n+\tgit add positive &&\n+\talt=\"fixed: 34\" &&\n+\tgit commit -s -m \"welcome\n+\n+We have now\n+$alt\" &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\t(\n+\t\techo welcome\n+\t\techo\n+\t\techo We have now\n+\t\techo $alt\n+\t\techo\n+\t\tgit var GIT_COMMITTER_IDENT |\n+\t\tsed -e \"s/>.*/>/\" -e \"s/^/Signed-off-by: /\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'multiple -m' '\n \n \t>negative &&\n-- \n1.6.5.1\n"},{"id":"126168","messageId":"7vd447o0jp.fsf@alter.siamese.dyndns.org","threadId":"21392","inReplyTo":"20091028171344.GA22290@quaoar.codeaurora.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-28T18:06:50Z","receivedAt":"2009-10-28T18:06:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Brown writes:\n\n> From: David Brown \n>\n> 'git commit -s' will insert a blank line before the Signed-off-by\n> line at the end of the message, unless this last line is a\n> Signed-off-by line itself. Common use has other trailing lines\n> at the ends of commit text, in the style of RFC2822 headers.\n>\n> Be more generous in considering lines to be part of this footer.\n> If the last paragraph of the commit message reasonably resembles\n> RFC-2822 formatted lines, don't insert that blank line.\n\nI do not think it is particularly readable to add Cc: at the end, and in a\nsense this patch encourages that practice (without the patch, the end\nresult looks ugly and that has an effect to discourage people from adding\nCc: there).\n\nBut this is not a strong objection. Applied.\n\nThanks.\n"},{"id":"126172","messageId":"20091028181720.GA28165@huya.quicinc.com","threadId":"21392","inReplyTo":"7vd447o0jp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"David Brown","fromEmail":"davidb@quicinc.com","sentAt":"2009-10-28T18:17:20Z","receivedAt":"2009-10-28T18:17:20Z","isPatch":true,"sender":{"key":"git@davidb.org","avatar":"https://gravatar.com/avatar/94c86a2938470a74c2eac5e2b69afc0871f79a660295c02219597aba8cb101c1?d=mp&s=160"},"body":"On Wed, Oct 28, 2009 at 11:06:50AM -0700, Junio C Hamano wrote:\n\n> I do not think it is particularly readable to add Cc: at the end, and in a\n> sense this patch encourages that practice (without the patch, the end\n> result looks ugly and that has an effect to discourage people from adding\n> Cc: there).\n\nI wasn't actually even thinking of Cc: at the end. I was\nthinking more of things like Acked-by:, or Bugs-fixed:, or\nPatch-applied-even-though-I-dont-like-it-by:, or\nlike that.\n\nDavid\n"},{"id":"126658","messageId":"20091103165951.GA2241@neumann","threadId":"21392","inReplyTo":"7vd447o0jp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-11-03T16:59:51Z","receivedAt":"2009-11-03T16:59:51Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n> > From: David Brown \n> >\n> > 'git commit -s' will insert a blank line before the Signed-off-by\n> > line at the end of the message, unless this last line is a\n> > Signed-off-by line itself. Common use has other trailing lines\n> > at the ends of commit text, in the style of RFC2822 headers.\n> >\n> > Be more generous in considering lines to be part of this footer.\n> > If the last paragraph of the commit message reasonably resembles\n> > RFC-2822 formatted lines, don't insert that blank line.\n\nI think this patch was a bit too generous. If I make a one-line\ncommit message with git commit -s -m which has a colon in it, e.g.\n'subsystem: what I did', then this patch removes the empty line\nbetween the subject and the SOB line.\n\n\nBest,\nGábor\n"},{"id":"126708","messageId":"1257304146-15543-1-git-send-email-szeder@ira.uka.de","threadId":"21392","inReplyTo":"20091103165951.GA2241@neumann","subject":"[PATCH] commit: fix too generous RFC-2822 footer handling","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-11-04T03:09:06Z","receivedAt":"2009-11-04T03:09:06Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Since commit c1e01b0c (commit: More generous accepting of RFC-2822\nfooter lines, 2009-10-28) RFC-2822-looking lines at the end of the\nmessage are considered part of the footer and 'git commit -s -m'\ndoesn't add a newline between that footer and the new S-O-B line.\nThis new behaviour causes problems with subject-only commit messages\nwhich happens to look like an RFC-2822 header (e.g. 'git commit -s -m\n\"subsystem: coolest feature ever\"'). In such cases there won't be any\nnewline between the subject and the S-O-B line, and the S-O-B line\nwill show up at places where it should not (e.g. in the output of 'git\nshortlog').\n\nWith this patch the newline will be always added if a commit message\nhas only a single line, even if it looks like an RFC-2822 header.\n\nSigned-off-by: SZEDER Gábor \n---\n\n Maybe something like this? Be careful when reviewing, it's 4AM\n here...\n\n\n builtin-commit.c | 8 ++++++++\n t/t7501-commit.sh | 4 ++--\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex beddf01..4971156 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -429,6 +429,14 @@ static int ends_rfc2822_footer(struct strbuf *sb)\n \t\thit = (buf[i] == '\\n');\n \t}\n \n+\tfor (j = i-1; j > 0; j--)\n+\t\tif (buf[j] == '\\n') {\n+\t\t\thit = 1;\n+\t\t\tbreak;\n+\t\t}\n+\tif (!hit)\t/* one-line message */\n+\t\treturn 0;\n+\n \twhile (i < len - 1 && buf[i] == '\\n')\n \t\ti++;\n \ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex d2de576..aaeedda 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -215,10 +215,10 @@ test_expect_success 'sign off (1)' '\n \n \techo 1 >positive &&\n \tgit add positive &&\n-\tgit commit -s -m \"thank you\" &&\n+\tgit commit -s -m \"subsystem: coolest feature ever\" &&\n \tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n \t(\n-\t\techo thank you\n+\t\techo subsystem: coolest feature ever\n \t\techo\n \t\tgit var GIT_COMMITTER_IDENT |\n \t\tsed -e \"s/>.*/>/\" -e \"s/^/Signed-off-by: /\"\n-- \n1.6.5.2.201.g0f47\n"},{"id":"126720","messageId":"7vljimlsza.fsf@alter.siamese.dyndns.org","threadId":"21392","inReplyTo":"1257304146-15543-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] commit: fix too generous RFC-2822 footer handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-04T06:11:21Z","receivedAt":"2009-11-04T06:11:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> builtin-commit.c | 8 ++++++++\n> t/t7501-commit.sh | 4 ++--\n> 2 files changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index beddf01..4971156 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -429,6 +429,14 @@ static int ends_rfc2822_footer(struct strbuf *sb)\n> \t\thit = (buf[i] == '\\n');\n> \t}\n> \n> +\tfor (j = i-1; j > 0; j--)\n> +\t\tif (buf[j] == '\\n') {\n> +\t\t\thit = 1;\n> +\t\t\tbreak;\n> +\t\t}\n> +\tif (!hit)\t/* one-line message */\n> +\t\treturn 0;\n> +\n\nThat looks overly convoluted. Why isn't the attached patch enough?\n\n - We inspected the last line of the message buffer, and 'i' is at the\n beginning of that last line;\n\n - At the line that begins at 'i', we found something that does not match\n the sob we are going to add;\n\n - We want a newline if it is a single liner (i.e. i == 0), or if that\n last one is not sob/acked-by and friends.\n\nIf you are anal and want to allow an author with a funny name \"is allowed\nas the first word\", we _could_ encounter a single-liner commit like this:\n\n From: is allowed as the first word \n\tSubject: Signed-off-by: is allowed as the first word \n\n Signed-off-by: is allowed as the first word \n\nand you may want to add \"!i ||\" in front of prefixcmp(), but I do not\nthink that is worth it.\n\n builtin-commit.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex c395cbf..cfa6b06 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -530,7 +530,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n \t\t\t; /* do nothing */\n \t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (!ends_rfc2822_footer(&sb))\n+\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n \t\t\t\tstrbuf_addch(&sb, '\\n');\n \t\t\tstrbuf_addbuf(&sb, &sob);\n \t\t}\n"},{"id":"126771","messageId":"20091104151114.GD6118@neumann","threadId":"21392","inReplyTo":"7vljimlsza.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: fix too generous RFC-2822 footer handling","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-11-04T15:11:14Z","receivedAt":"2009-11-04T15:11:14Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Tue, Nov 03, 2009 at 10:11:21PM -0800, Junio C Hamano wrote:\n> That looks overly convoluted.\n\nI figured that the function ends_rfc2822_footer() should tell us\nwhether the message, well, ends with an rfc2822 _footer_. But since\nit may say so even if there is only a single line in the commit\nmessage, I thought this function should be fixed in the first place.\n\nBut yeah, that solution was unnecessarily complicated, after a good\nnight's sleep I would do it this way:\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex beddf01..c7dcbd0 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -428,6 +428,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)\n break;\n hit = (buf[i] == '\\n');\n }\n+ if (i == 0) /* one-line message */\n+ return 0;\n \n while (i < len - 1 && buf[i] == '\\n')\n i++;\n\n> Why isn't the attached patch enough?\n> \n> - We inspected the last line of the message buffer, and 'i' is at the\n> beginning of that last line;\n> \n> - At the line that begins at 'i', we found something that does not match\n> the sob we are going to add;\n> \n> - We want a newline if it is a single liner (i.e. i == 0), or if that\n> last one is not sob/acked-by and friends.\n\nYou are right in that there is no need to look for an rfc-2822\nformatted footer when the commit message has only a single line. But\nends_rfc2822_footer() still not completely behaves as its name would\nsuggest (i.e. it might match even if there is no footer). Perhaps it\ncould just be renamed to ends_rfc282().\n\n\nBest,\nGábor\n"}]}