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

From: Jeff King <peff@peff.net>
Date: Nov 18, 2009, 17:24 UTC
Message-ID: <20091118172424.GA24416@coredump.intra.peff.net>
In-Reply-To: <aa2993680911180911o7e3af804m4ebdc20096baa609@mail.gmail.com>

On Wed, Nov 18, 2009 at 06:11:36PM +0100, Philip Hofstetter wrote:

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

I'm not sure I like that solution for a few reasons:

  1. It creates a bad user experience. You are not unreasonable for
     wanting to put some specific text in your commit message. Having
     git come back and say "oops, I might get confused by this later"
     just seems like an annoyance to the user.

  2. Mailinfo has to deal with data created by older versions of git. So
     in your case, the rebase was a bomb waiting to go off. If we can
     fix it so that an existing bomb doesn't go off, rather than not
     creating the bomb in the first place, then we are better off.

  3. Commit has to know about rules for mailinfo, even versions of
     mailinfo that will exist in the future. Probably the rules aren't
     going to change much, but it is a weakness.

  4. Commit messages can come from other places than "git commit". What
     should we do with a commit message like this that is imported from
     SVN? Reject the import? Munge the message?

Of course all of that presupposes that we can correctly handle the existing data after the fact. Even with my patch, you still can't write "From: foo@example.com" as the first line of your commit body. But that is, IMHO, getting even more unlikely than your "From:" (which already seems fairly unlikely).

I also think "git commit" would not be the right time for such a feature. The problem is not that you have this text in your commit message. The problem is that the "format-patch | am" transport is lossy. You would do better to have format-patch say "Ah, this is going to create a bogus email address" and somehow quote it appropriately.

-Peff