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

[Philip Hofstetter <phofstetter@sensational.ch>]

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.

> 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