Subject: Re: [PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line

From: Matthieu Moy <[email-masked]>
Date: Wed, 17 Jun 2015 17:45:47 +0200

Remi Lespinet <[email-masked]> writes:

> ---
>  git-send-email.perl | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index a0cd7ff..a1f6c18 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -477,9 +477,59 @@ foreach my $entry (@bcclist) {
>  sub parse_address_line {
>  	if ($have_mail_address) {
>  		return map { $_->format } Mail::Address->parse($_[0]);
> -	} else {
> -		return split_addrs($_[0]);
>  	}
> +
> +	my $commentrgx=qr/\((?:[^)]*)\)/;
> +	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
> +	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;

Spaces around = please.

The code below is a bit hard to read (I'm neither fluent in Perl nor in
the RFC ...). A few more comments would help. A few examples below (it's
up to you to integrate them or not).

> +	my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;
> +
> +	my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
> +	push @tokens, ",";

        # parse a full address like
        # "Phrase" (comment) <[email-masked]>

(to clarify the wording)

> +	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> +	foreach my $token (@tokens) {
> +	    if ($token =~ /^[,;]$/) {

Here and below: you're indenting with a 4-column offset, it should be 8.

> +		if (@address) {
> +		    push @address, @buffer;
> +		} else {
> +		    push @phrase, @buffer;
> +		}
> +
> +		my $str_phrase = join ' ', @phrase;
> +		my $str_address = join '', @address;
> +		my $str_comment = join ' ', @comment;

                # Escape special-characters with backslash
> +		if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
> +		    $str_phrase =~ s/(^|[^\\])"/$1/g;
> +		    $str_phrase = qq["$str_phrase"];
> +		}
> +
> +		if ($str_address ne "" && $str_phrase ne "") {
> +		    $str_address = qq[<$str_address>];
> +		}
> +
> +		my $str_mailbox = "$str_phrase $str_address $str_comment";
> +		$str_mailbox =~ s/^\s*|\s*$//g;
> +		push @addr_list, $str_mailbox if ($str_mailbox);
> +
> +		@phrase = @address = @comment = @buffer = ();
> +	    } elsif ($token =~ /^\(/) {
> +		push @comment, $token;
> +	    } elsif ($token eq "<") {
> +		push @phrase, (splice @address), (splice @buffer);
> +	    } elsif ($token eq ">") {
> +		push @address, (splice @buffer);
> +	    } elsif ($token eq "@") {
> +		push @address, (splice @buffer), "@";
> +	    } elsif ($token eq ".") {
> +		push @address, (splice @buffer), ".";
> +	    } else {

                # We don't know what the token belongs to yet. We'll
                # decide where to append @buffer later.
> +		push @buffer, $token;
> +	    }
> +	}
> +
> +	return @addr_list;

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/