threads / patch / 14566

patchmailinfo: better parse email adresses containg parentheses

Subject: [PATCH] mailinfo: better parse email adresses containg parentheses

## tl;dr

4 messages between Jul 21, 2008 and Jul 22, 2008. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Philippe Bruhat (BooK)· Jul 21, 2008, 13:34 UTC · lore
    When using git-rebase, author fields containing a ')' at the last
    position had the close-parens character incorrectly removed
    because the From: parser incorrectly matched it as
        user@host (User Name)
    (removing parentheses), instead of
        User Name (me) <user@host>
Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 builtin-mailinfo.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin-mailinfo.c +2 −2
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index b99a5b6..5581c9f 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -115,10 +115,10 @@ static void handle_from(const struct strbuf *from)
 	 * the () pair at the end.
 	 */
 	strbuf_trim(&f);
-	if (f.buf[0] == '(')
+	if (f.buf[0] == '(' && f.len && f.buf[f.len - 1] == ')') {
 		strbuf_remove(&name, 0, 1);
-	if (f.len && f.buf[f.len - 1] == ')')
 		strbuf_setlen(&f, f.len - 1);
+	}
 
 	get_sane_name(&name, &f, &email);
 	strbuf_release(&f);

-- 
1.5.4.3
Lukas Sandström· Jul 21, 2008, 14:36 UTC · re: Philippe Bruhat (BooK) · lore

Re: [PATCH] mailinfo: better parse email adresses containg parentheses

Philippe Bruhat (BooK) wrote:
Show 11 quoted lines
>     When using git-rebase, author fields containing a ')' at the last
>     position had the close-parens character incorrectly removed
>     because the From: parser incorrectly matched it as
> 
>         user@host (User Name)
> 
>     (removing parentheses), instead of
> 
>         User Name (me) <user@host>
> 
> Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
ACK.
Show 15 quoted lines
> ---
>  builtin-mailinfo.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
> index b99a5b6..5581c9f 100644
> --- a/builtin-mailinfo.c
> +++ b/builtin-mailinfo.c
> @@ -115,10 +115,10 @@ static void handle_from(const struct strbuf *from)
>  	 * the () pair at the end.
>  	 */
>  	strbuf_trim(&f);
> -	if (f.buf[0] == '(')
> +	if (f.buf[0] == '(' && f.len && f.buf[f.len - 1] == ')') {
>  		strbuf_remove(&name, 0, 1);
Note: The line above should obviously be "strbuf_remove(&f, 0, 1);",
another error from the strbuf conversion I made. Sigh.
Show 6 quoted lines
> -	if (f.len && f.buf[f.len - 1] == ')')
>  		strbuf_setlen(&f, f.len - 1);
> +	}
>  
>  	get_sane_name(&name, &f, &email);
>  	strbuf_release(&f);
/Lukas
Junio C Hamano· Jul 22, 2008, 03:16 UTC · re: Philippe Bruhat (BooK) · lore

Re: [PATCH] mailinfo: better parse email adresses containg parentheses

"Philippe Bruhat (BooK)" <book@cpan.org> writes:
Show 11 quoted lines
>     When using git-rebase, author fields containing a ')' at the last
>     position had the close-parens character incorrectly removed
>     because the From: parser incorrectly matched it as
>
>         user@host (User Name)
>
>     (removing parentheses), instead of
>
>         User Name (me) <user@host>
>
> Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
Hmm, tests?
By the way, that second form parses like this:
	mailbox =
        name-addr =
        display-name angle-addr = "User Name (me) <user@host>"
        display-name =
        phrase = "User Name"
        
        angle-addr = CFWS "<" addr-spec ">" = "(me) <user@host>"

So strictly speaking, shouldn't we be stripping the whole (me) as garbage? It is not even part of the display-name but is a whitespace equivalent comment.
Philippe Bruhat (BooK)· Jul 22, 2008, 10:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] mailinfo: better parse email adresses containg parentheses

On Mon, Jul 21, 2008 at 08:16:43PM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> >
> > Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
> 
> Hmm, tests?
> 
> By the way, that second form parses like this:
> 
> 	mailbox =
>         name-addr =
>         display-name angle-addr = "User Name (me) <user@host>"
> 
>         display-name =
>         phrase = "User Name"
>         
>         angle-addr = CFWS "<" addr-spec ">" = "(me) <user@host>"
> 
> So strictly speaking, shouldn't we be stripping the whole (me) as garbage?
> It is not even part of the display-name but is a whitespace equivalent
> comment.
Well, I use this:
    "Philippe Bruhat (BooK)" <book@cpan.org>
as my From: line, and I would like to be able to use it so in git.

As git knows the difference between user name and user email, it should be able to keep the information separate all the way.

Which makes me think that since it seems that rebase goes through a patch-file step, the problem with my username may actually lie with creating the patch file (no quotes around a user name containing parens) rather than with parsing the patch From: line.

-- 
 Philippe Bruhat (BooK)

 Everyone's life seems easier from the outside.
                                    (Moral from Groo The Wanderer #45 (Epic))

← back to recent threads