Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.

David Brown [off-list ref] writes:

From: David Brown <redacted>

'git commit -s' will insert a blank line before the Signed-off-by
line at the end of the message, unless this last line is a
Signed-off-by line itself.  Common use has other trailing lines
at the ends of commit text, in the style of RFC2822 headers.

Be more generous in considering lines to be part of this footer.
If the last paragraph of the commit message reasonably resembles
RFC-2822 formatted lines, don't insert that blank line.

I do not think it is particularly readable to add Cc: at the end, and in a
sense this patch encourages that practice (without the patch, the end
result looks ugly and that has an effect to discourage people from adding
Cc: there).

But this is not a strong objection.  Applied.

Thanks.

On Wed, Oct 28, 2009 at 11:06:50AM -0700, Junio C Hamano wrote:

I do not think it is particularly readable to add Cc: at the end, and in a
sense this patch encourages that practice (without the patch, the end
result looks ugly and that has an effect to discourage people from adding
Cc: there).

I wasn't actually even thinking of Cc: at the end.  I was
thinking more of things like Acked-by:, or Bugs-fixed:, or
Patch-applied-even-though-I-dont-like-it-by:, or
like that.

David

Hi,

quoted

From: David Brown <redacted>

'git commit -s' will insert a blank line before the Signed-off-by
line at the end of the message, unless this last line is a
Signed-off-by line itself.  Common use has other trailing lines
at the ends of commit text, in the style of RFC2822 headers.

Be more generous in considering lines to be part of this footer.
If the last paragraph of the commit message reasonably resembles
RFC-2822 formatted lines, don't insert that blank line.

I think this patch was a bit too generous.  If I make a one-line
commit message with git commit -s -m which has a colon in it, e.g.
'subsystem: what I did', then this patch removes the empty line
between the subject and the SOB line.


Best,
Gábor

Since commit c1e01b0c (commit: More generous accepting of RFC-2822
footer lines, 2009-10-28) RFC-2822-looking lines at the end of the
message are considered part of the footer and 'git commit -s -m'
doesn't add a newline between that footer and the new S-O-B line.
This new behaviour causes problems with subject-only commit messages
which happens to look like an RFC-2822 header (e.g. 'git commit -s -m
"subsystem: coolest feature ever"').  In such cases there won't be any
newline between the subject and the S-O-B line, and the S-O-B line
will show up at places where it should not (e.g. in the output of 'git
shortlog').

With this patch the newline will be always added if a commit message
has only a single line, even if it looks like an RFC-2822 header.

Signed-off-by: SZEDER Gábor <redacted>
---

 Maybe something like this?  Be careful when reviewing, it's 4AM
 here...


 builtin-commit.c  |    8 ++++++++
 t/t7501-commit.sh |    4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index beddf01..4971156 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -429,6 +429,14 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		hit = (buf[i] == '\n');
 	}
 
+	for (j = i-1; j > 0; j--)
+		if (buf[j] == '\n') {
+			hit = 1;
+			break;
+		}
+	if (!hit)	/* one-line message */
+		return 0;
+
 	while (i < len - 1 && buf[i] == '\n')
 		i++;
 

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d2de576..aaeedda 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh

@@ -215,10 +215,10 @@ test_expect_success 'sign off (1)' '
 
 	echo 1 >positive &&
 	git add positive &&
-	git commit -s -m "thank you" &&
+	git commit -s -m "subsystem: coolest feature ever" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
 	(
-		echo thank you
+		echo subsystem: coolest feature ever
 		echo
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"

-- 
1.6.5.2.201.g0f47