From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Josh Stone <jistone@redhat.com>, git@vger.kernel.org
Subject: Re: [PATCH] blame: Improve parsing for emails with spaces
Date: Fri, 29 Apr 2011 10:59:43 -0700 [thread overview]
Message-ID: <7vvcxxvsz4.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20110429131103.GB4540@sigill.intra.peff.net> (Jeff King's message of "Fri, 29 Apr 2011 09:11:03 -0400")
Jeff King <peff@peff.net> writes:
> On Thu, Apr 21, 2011 at 03:07:36PM -0700, Josh Stone wrote:
>
>> One of my git repositories has some old commits where the authors
>> obfuscated their email address as <author at example dot com>. To
>> handle this, blame needs to look for the leading '<' when scanning
>> to split the "name <email>", rather then only a space delimiter.
Given that we enclose the e-mail inside "<>" pair and excise "<" from
author names in fmt_ident(), I think it makes sense to look for " <" like
this patch does.
> Hmm. That address seems bogus, and I wonder if we should be rejecting it
> at commit time. Still, it is something we may run across in existing
> repositories, so handling it more gracefully makes sense.
Perhaps but within reason.
What new types of breakages are we proposing to tolerate, what breakages
are we declaring not worth fixing, and what is the price of not loosening
this? Without this patch, such a broken commit will result in the author
email shown somewhat broken, but the original is already broken to begin
with, and also the entry for the blamed line will come with its commit
object name anyway, so I do not think it is such a big deal.
It would be an entirely different issue if the command barfed and refused
to blame the file.
> Looking over the other places we parse author info, I don't think the
> same problem exists elsewhere. Most other places parse from left to
> right, and just go to the closing ">".
Because fmt_ident() removes "<" or ">" from given email address, I think
it is sufficient.
It would be nice to have an abstraction around the parsing in a way
similar to fmt_ident() is an abstraction around the formatting, but that
would be a separate topic.
next prev parent reply other threads:[~2011-04-29 18:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-21 22:07 [PATCH] blame: Improve parsing for emails with spaces Josh Stone
2011-04-29 13:11 ` Jeff King
2011-04-29 17:59 ` Junio C Hamano [this message]
2011-04-29 18:09 ` Junio C Hamano
2011-04-29 18:17 ` Josh Stone
2011-04-29 19:13 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vvcxxvsz4.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jistone@redhat.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).