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

From: Junio C Hamano <[email-masked]>
Date: Wed, 17 Jun 2015 14:27:40 -0700

Remi Lespinet <[email-masked]> writes:

> parse_address_line had not the same behavior whether the user had
> Mail::Address or not. Teach parse_address_line to behave like
> Mail::Address.

Sounds like a fun project ;-)

> +	my $commentrgx=qr/\((?:[^)]*)\)/;
> +	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
> +	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
> +	my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;

Suffix "rgx" that means "regular expression" is a bit unusual, and
also hard to read when squashed to another word.  Elsewhere in the
same script, we seem to use $re_whatever to store precompiled
regular expressions, so perhaps $re_comment, $re_quote, etc.?

> +	my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
> +	push @tokens, ",";
> +
> +	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> +	foreach my $token (@tokens) {
> +	    if ($token =~ /^[,;]$/) {
> +		if (@address) {
> +		    push @address, @buffer;
> +		} else {
> +		    push @phrase, @buffer;
> +		}
> +
> +		my $str_phrase = join ' ', @phrase;
> +		my $str_address = join '', @address;
> +		my $str_comment = join ' ', @comment;
> +
> +		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>];
> +		}

We see both "[email-masked]" and "<[email-masked]>" around
here for an address without comment or phrase; this chooses to turn
them both into "<[email-masked]>" form?  Not a complaint but am
thinking aloud to see if I am reading it correctly.

> +
> +		my $str_mailbox = "$str_phrase $str_address $str_comment";
> +		$str_mailbox =~ s/^\s*|\s*$//g;

So an empty @comment will not leave spaces after $str_address, which
makes sense (likewise for @phrase).

> +		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);

That is a clever use of splice (My Perl's rusty; you learn new
things every day) ;-)

> +	    } elsif ($token eq ">") {
> +		push @address, (splice @buffer);
> +	    } elsif ($token eq "@") {
> +		push @address, (splice @buffer), "@";
> +	    } elsif ($token eq ".") {
> +		push @address, (splice @buffer), ".";
> +	    } else {
> +		push @buffer, $token;
> +	    }
> +	}
> +
> +	return @addr_list;
>  }