Jeff King [off-list ref] writes:
I'm tempting by the patch below, which would remove only the
syntactically significant meta-characters ("\n", "<", and ">"), as well
as trimming any stray whitespace at the edges. The problem is that we
don't really have a clue how many people were relying on this trimming
to clean up their names or emails, so there may be regressions for other
people.
What do you exactly mean by "syntactically significant"? In other
words, "whose syntax"?
The code with the patch will leave "." out of the crud, so with
spearce:*:1000:1000:Shawn O. Pearce:/home/spearce:/bin/sh
we would get:
From: Shawn O. Pearce [off-list ref]
without dropping the "." in the name. Your MTA would likely to
reject it.
I think that quoting "syntactically significant meta-characters" in
the context of e-mail headers is a job for the MSA, and the human
readable names in GIT_AUTHOR_NAME should allow any reasonable
character. And I agree that it is a sane definition of "reasonable"
to exclude "\n", "<", and ">" (and nothing else), as they are the
only "syntactically significant" in the context of commit object
header.
The patch goes in the right direction in that sense, but you need to
make sure that git-send-email and git-imap-send (the only two MSA we
ship) do the right thing when fed names with ".", dq, etc. first.
quoted hunk
diff --git a/ident.c b/ident.c
index 443c075..4552f8d 100644
--- a/ident.c
+++ b/ident.c
@@ -127,15 +127,8 @@ const char *ident_default_date(void)
static int crud(unsigned char c)
{
return c <= 32 ||
- c == '.' ||
- c == ',' ||
- c == ':' ||
- c == ';' ||
c == '<' ||
- c == '>' ||
- c == '"' ||
- c == '\\' ||
- c == '\'';
+ c == '>';
}
/*
--To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 29, 2012 at 12:04:18PM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
I'm tempting by the patch below, which would remove only the
syntactically significant meta-characters ("\n", "<", and ">"), as well
as trimming any stray whitespace at the edges. The problem is that we
don't really have a clue how many people were relying on this trimming
to clean up their names or emails, so there may be regressions for other
people.
What do you exactly mean by "syntactically significant"? In other
words, "whose syntax"?
I meant syntactically significant to git's ident lines. I.e., "<", ">",
and "\n". If the identities are going to be put somewhere else with
different syntax (e.g., an rfc822 header), then they would obviously
need to be quoted (and that is no different than the current state), but
that should happen elsewhere.
The code with the patch will leave "." out of the crud, so with
spearce:*:1000:1000:Shawn O. Pearce:/home/spearce:/bin/sh
we would get:
From: Shawn O. Pearce [off-list ref]
without dropping the "." in the name. Your MTA would likely to
reject it.
But that is already the case. The crud() function is checked only for
the beginning and end of each item. Which is why you get senseless
outcomes like "A.B.C." turning into "A.B.C". AFAICT, the motivation for
most items in the crud function is purely about "this is junk that we
might find in a gecos field and should be stripped to make the name
prettier". I.e., they are heuristics, and we now have two reports of
those heuristics being wrong.
My concern is that those heuristics are sometimes _right_, and are
helping people. But we don't know how often, and I suspect there is no
way to know without changing it and waiting for people to scream, which
does not excite me.
The only example I could come up with by thinking is that we probably
_do_ want to strip a trailing dot from an email address (e.g., some
people will express a hostname as "example.com." to indicate that it is
fully qualified, but it is typically omitted from an email address).
Handling that would involve splitting the heuristics for names and
emails.
I think that quoting "syntactically significant meta-characters" in
the context of e-mail headers is a job for the MSA, and the human
readable names in GIT_AUTHOR_NAME should allow any reasonable
character. And I agree that it is a sane definition of "reasonable"
to exclude "\n", "<", and ">" (and nothing else), as they are the
only "syntactically significant" in the context of commit object
header.
The patch goes in the right direction in that sense, but you need to
make sure that git-send-email and git-imap-send (the only two MSA we
ship) do the right thing when fed names with ".", dq, etc. first.
Actually, it is format-patch where the quoting should go, as it is the
thing that puts the ident in the rfc822 header. And indeed, it already
does so (as it must, because we _do_ allow "." as in "Shawn O. Pearce",
and have always done so).
-Peff