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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Nov 4, 2009, 15:11 UTC
Message-ID: <20091104151114.GD6118@neumann>
In-Reply-To: <7vljimlsza.fsf@alter.siamese.dyndns.org>

Hi,

On Tue, Nov 03, 2009 at 10:11:21PM -0800, Junio C Hamano wrote:

> That looks overly convoluted.

I figured that the function ends_rfc2822_footer() should tell us whether the message, well, ends with an rfc2822 _footer_. But since it may say so even if there is only a single line in the commit message, I thought this function should be fixed in the first place.

But yeah, that solution was unnecessarily complicated, after a good night's sleep I would do it this way:

diff --git a/builtin-commit.c b/builtin-commit.c
index beddf01..c7dcbd0 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -428,6 +428,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
                        break;
                hit = (buf[i] == '\n');
        }
+       if (i == 0)     /* one-line message */
+               return 0;
 
        while (i < len - 1 && buf[i] == '\n')
                i++;

> 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.

You are right in that there is no need to look for an rfc-2822
formatted footer when the commit message has only a single line.  But
ends_rfc2822_footer() still not completely behaves as its name would
suggest (i.e. it might match even if there is no footer).  Perhaps it
could just be renamed to ends_rfc282().


Best,
Gábor