Re: [PATCH v2] git am/mailinfo: Don't look at in-body headers when rebasing

[Jeff King <peff@peff.net>]

On Thu, Nov 19, 2009 at 09:51:36AM +0100, Lukas Sandström wrote:

> When we are rebasing we know that the header lines in the
> patch are good and that we don't need to pick up any headers
> from the body of the patch.
> 
> This makes it possible to rebase commits whose commit message
> start with "From" or "Date".
> 
> Test vectors by Jeff King.

Thanks, it did end up being a pretty small change. Though I think we may
be better off with _both_ patches. Your patch protects the message
absolutely during rebasing, and my patch improves the heuristic when
applying non-rebase patches.

> @@ -771,6 +772,8 @@ static int handle_commit_msg(struct strbuf *line)
>  		return 0;
> 
>  	if (still_looking) {
> +		if (!use_inbody_headers)
> +			still_looking = 0;
>  		strbuf_ltrim(line);
>  		if (!line->len)
>  			return 0;

Hmm. But we still end up in this conditional for the very first line.
Which I guess happens to work because the first line we feed is
presumably the empty blank line (but I didn't check). Still, wouldn't it
be more clear as:

  if (use_inbody_headers && still_looking) {
     ...

in which case still_looking simply becomes irrelevant when the feature
is disabled?

> +From nobody Mon Sep 17 00:00:00 2001
> +From: A U Thor <a.u.thor@example.com>
> +Subject: check bogus body header (from)
> +Date: Fri, 9 Jun 2006 00:44:16 -0700
> +
> +From: bogosity
> +  - a list
> +  - of stuff
> +---

Since your feature is meant to prevent us looking at inbody headers no
matter if they are valid-looking or not, wouldn't a better test be to
actually have:

  From: Other Author <other@example.com>

Otherwise, you don't know if it is your feature blocking it, or my patch
(if it gets applied on top).

> +From nobody Mon Sep 17 00:00:00 2001
> +From: A U Thor <a.u.thor@example.com>
> +Subject: check bogus body header (date)
> +Date: Fri, 9 Jun 2006 00:44:16 -0700
> +
> +Date: bogus
> +
> +and some content
> +

And ditto for the Date here.

-Peff