Re: [PATCH] commit: fix too generous RFC-2822 footer handling

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder@ira.uka.de> writes:

>  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;
> +

That looks overly convoluted.  Why isn't the attached patch enough?

 - We inspected the last line of the message buffer, and 'i' is at the
   beginning of that last line;

 - At the line that begins at 'i', we found something that does not match
   the sob we are going to add;

 - We want a newline if it is a single liner (i.e. i == 0), or if that
   last one is not sob/acked-by and friends.

If you are anal and want to allow an author with a funny name "is allowed
as the first word", we _could_ encounter a single-liner commit like this:

        From: is allowed as the first word <author@example.xz>
	Subject: Signed-off-by: is allowed as the first word <author@example.xz>

        Signed-off-by: is allowed as the first word <author@example.xz>

and you may want to add "!i ||" in front of prefixcmp(), but I do not
think that is worth it.

 builtin-commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..cfa6b06 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -530,7 +530,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
 			; /* do nothing */
 		if (prefixcmp(sb.buf + i, sob.buf)) {
-			if (!ends_rfc2822_footer(&sb))
+			if (!i || !ends_rfc2822_footer(&sb))
 				strbuf_addch(&sb, '\n');
 			strbuf_addbuf(&sb, &sob);
 		}