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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > 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@xxxxxxxxxxx" 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]