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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 17, 2015, 21:27 UTC
Message-ID: <xmqqioam58kz.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1434550720-24130-7-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr>

Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> 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.?

Show 24 quoted lines

> +	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 "git@vger.kernel.org" and "<git@vger.kernel.org>" around here for an address without comment or phrase; this chooses to turn them both into "<git@vger.kernel.org>" 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).

Show 7 quoted lines

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

Show 13 quoted lines

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