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

From: Jeff King <peff@peff.net>
Date: Nov 19, 2009, 15:36 UTC
Message-ID: <20091119153622.GC6877@coredump.intra.peff.net>
In-Reply-To: <4B050718.8070506@gmail.com>

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

Show 8 quoted lines

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

Show 9 quoted lines

> @@ -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?

Show 9 quoted lines

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

Show 9 quoted lines

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