Re: git-mailinfo doesn't stop parsing at the end of the header

From: Philip Hofstetter <phofstetter@sensational.ch>
Date: Nov 18, 2009, 17:11 UTC
Message-ID: <aa2993680911180911o7e3af804m4ebdc20096baa609@mail.gmail.com>
In-Reply-To: <20091118155154.GA15184@coredump.intra.peff.net>

Hello,

On Wed, Nov 18, 2009 at 4:51 PM, Jeff King <peff@peff.net> wrote:

> On Wed, Nov 18, 2009 at 03:20:48PM +0100, Philip Hofstetter wrote:

>  1. Improve the header-finding heuristic to actually look for something
>     more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
>     head which other headers we handle in this position. Probably
>     Date, too).

or at least don't prefer obviously invalid data over valid data that has already been seen.

>  2. Give mailinfo a "--strict" mode to indicate that it is directly
>     parsing the output of format-patch, and not some random email. Use
>     --strict when invoking "git am" via "git rebase".

That would solve the problem too, though it feels like adding yet another switch to guard against one specific issue. The purpose behind options like this tends to get forgotten over time.

Show 5 quoted lines

> As I explained above, there is a reason, but I don't think it's rude to
> have either of those lines. You were, after all, writing a commit
> message, not an email (and even if you were, it is a failure of the
> storage format if it can't represent your data correctly). So I think
> git is to blame here.

IMHO, another workable solution would be to reject a commit that later can't be handled. That way the current attempts at getting an email address can remain intact and the (much more) unlikely case that somebody begins the commit message with from: will be caught before damage is done.

So, just check that from-line for a valid email address at commit time. If it is, ok. If not, treat it as an error and inform the user that an invalid email address was given in the commit message.

Also, the error message by rebase (which is actually the message printed by am) could have been a bit more helpful. If am fails during a rebase, rebase could explicitly tell which commit am failed at. The output I got made me suspect the problem to be in the first commit (as that was the last one printed) when in fact it was in the second one (which was not printed).

But that's just nit-picking.

Philip