[PATCH v5 07/10] send-email: reduce dependancies impact on parse_address_line
- From
Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
- Date
- Jun 20, 2015, 23:17 UTC
- Message-ID
- <1434842273-30945-7-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr>
- In-Reply-To
- <1434842273-30945-1-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr>
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.
When the user input is correct, this implementation behaves exactly like Mail::Address except when there are quotes inside the name:
"Jane Do"e <jdoe@example.com>
In this case the result of parse_address_line is:
With M::A : "Jane Do" e <jdoe@example.com> Without : "Jane Do e" <jdoe@example.com>
When the user input is not correct, the behavior is also mostly the same.
Unlike Mail::Address, this doesn't parse groups and recursive commentaries.
Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
---
I've added the function in Git.pm as suggested. I've also added a test named t9000-addresses.sh (I've read the README to name tests but I'm not sure about the name of this test). I made a separated test (t9000-addresses.sh) because I think it's better not to pollute t9001-send-email with this.
About the test itself, file t/t9000-addresses.sh is just a copy/paste of t/t0202-gettext-perl.sh. For the perl part, the TODO tests are verbose: they print out commands whereas test_expect_success doesn't. We can redirect todo_output to a variable but I've not found better... (Maybe someone has the solution here ?). Also there's no summary at the end of the test (as with other perl tests).
git-send-email.perl | 2 +- perl/Git.pm | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ t/t9000-addresses.sh | 25 ++++++++++++++++++ t/t9000/test.pl | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-) create mode 100755 t/t9000-addresses.sh create mode 100755 t/t9000/test.pl
diff --git a/git-send-email.perl b/git-send-email.perl index a0cd7ff..bced78e 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -478,7 +478,7 @@ sub parse_address_line { if ($have_mail_address) { return map { $_->format } Mail::Address->parse($_[0]); } else { - return split_addrs($_[0]); + return Git::parse_mailboxes($_[0]); } } diff --git a/perl/Git.pm b/perl/Git.pm index 9026a7b..97633e9 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -1584,6 +1584,73 @@ sub DESTROY { $self->_close_cat_blob(); } +=item parse_mailboxes + +Returns an array of mailboxes extracted from a string. + +=cut + +sub parse_mailboxes { + my $re_comment = qr/\((?:[^)]*)\)/; + my $re_quote = qr/"(?:[^\"\\]|\\.)*"/; + my $re_word = qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/; + + # divide the string in tokens of the above form + my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/; + my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_; + + # add a delimiter to simplify treatment for the last mailbox + push @tokens, ","; + + my (@addr_list, @phrase, @address, @comment, @buffer) = (); + foreach my $token (@tokens) { + if ($token =~ /^[,;]$/) { + # if buffer still contains undeterminated strings + # append it at the end of @address or @phrase + if (@address) { + push @address, @buffer; + } else { + push @phrase, @buffer; + } + + my $str_phrase = join ' ', @phrase; + my $str_address = join '', @address; + my $str_comment = join ' ', @comment; + + # quote are necessary if phrase contains + # special characters + if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) { + $str_phrase =~ s/(^|[^\\])"/$1/g; + $str_phrase = qq["$str_phrase"]; + } + + # add "<>" around the address if necessary + 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 { + push @buffer, $token; + } + } + + return @addr_list; +} # Pipe implementation for ActiveState Perl. diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 0000000..280f2c5 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,25 @@ +#!/bin/sh +# +# Copyright (c) 2015 +# + +test_description='compare address parsing with and without Mail::Address' +. ./test-lib.sh + +if ! test_have_prereq PERL; then + skip_all='skipping perl interface tests, perl not available' + test_done +fi + +perl -MTest::More -e 0 2>/dev/null || { + skip_all="Perl Test::More unavailable, skipping test" + test_done +} + +test_external_has_tap=1 + +test_external_without_stderr \ + 'Perl address parsing function' \ + perl "$TEST_DIRECTORY"/t9000/test.pl + +test_done diff --git a/t/t9000/test.pl b/t/t9000/test.pl new file mode 100755 index 0000000..f8b7b34 --- /dev/null +++ b/t/t9000/test.pl @@ -0,0 +1,71 @@ +#!/usr/bin/perl +use lib (split(/:/, $ENV{GITPERLLIB})); + +use 5.008; +use warnings; +use strict; + +use Test::More; + +BEGIN { + Test::More->builder->no_ending(1); +} + +BEGIN { use_ok('Git') } +BEGIN { use_ok('Mail::Address') } + +my @success_list = (q[Jane], + q[jdoe@example.com], + q[<jdoe@example.com>], + q[Jane <jdoe@example.com>], + q[Jane Doe <jdoe@example.com>], + q["Jane" <jdoe@example.com>], + q["Doe, Jane" <jdoe@example.com>], + q["Jane@:;\>.,()<Doe" <jdoe@example.com>], + q[Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>], + q["<jdoe@example.com>"], + q["Jane jdoe@example.com"], + q[Jane Doe <jdoe @ example.com >], + q[Jane Doe < jdoe@example.com >], + q[Jane @ Doe @ Jane @ Doe], + q["Jane, 'Doe'" <jdoe@example.com>], + q['Doe, "Jane' <jdoe@example.com>], + q["Jane" "Do"e <jdoe@example.com>], + q["Jane' Doe" <jdoe@example.com>], + q["Jane Doe <jdoe@example.com>" <jdoe@example.com>], + q["Jane\" Doe" <jdoe@example.com>], + q[Doe, jane <jdoe@example.com>], + q["Jane Doe <jdoe@example.com>], + q['Jane 'Doe' <jdoe@example.com>]); + +my @known_failure_list = (q[Jane\ Doe <jdoe@example.com>], + q["Doe, Ja"ne <jdoe@example.com>], + q["Doe, Katarina" Jane <jdoe@example.com>], + q[Jane@:;\.,()<>Doe <jdoe@example.com>], + q[Jane jdoe@example.com], + q[<jdoe@example.com> Jane Doe], + q[Jane <jdoe@example.com> Doe], + q["Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>], + q[Jane Doe], + q[Jane "Doe <jdoe@example.com>"], + q[\"Jane Doe <jdoe@example.com>], + q[Jane\"\" Doe <jdoe@example.com>], + q['Jane "Katarina\" \' Doe' <jdoe@example.com>]); + +foreach my $str (@success_list) { + my @expected = map { $_->format } Mail::Address->parse("$str"); + my @actual = Git::parse_mailboxes("$str"); + is_deeply(\@expected, \@actual, qq[same output : $str]); +} + +TODO: { + local $TODO = "known breakage"; + foreach my $str (@known_failure_list) { + my @expected = map { $_->format } Mail::Address->parse("$str"); + my @actual = Git::parse_mailboxes("$str"); + is_deeply(\@expected, \@actual, qq[same output : $str]); + } +} + +my $is_passing = Test::More->builder->is_passing; +exit($is_passing ? 0 : 1);
-- 1.9.1