Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
#!/usr/bin/perl Should we have hard-coded PATH to perl here ? /usr/bin/perl --version This is perl, v5.10.0 built for darwin-thread-multi-2level (with 2 registered patches, see perl -V for more detail) + +my $is_passing = Test::More-builder-is_passing; +exit($is_passing ? 0 : 1); This seems to give problems: debug=t verbose=t ./t9000-addresses.sh Initialized empty Git repository in /Users/me/projects/git/git.pu/t/trash directory.t9000-addresses/.git/ # run 0: Perl address parsing function (perl /Users/me/projects/git/git.pu/t/t9000/test.pl) ok 1 - use Git; ok 2 - same output : Jane ok 3 - same output : j...@example.com ok 4 - same output : j...@example.com ok 5 - same output : Jane j...@example.com ok 6 - same output : Jane Doe j...@example.com ok 7 - same output : Jane j...@example.com ok 8 - same output : Doe, Jane j...@example.com ok 9 - same output : Jane@:;\.,()Doe j...@example.com ok 10 - same output : Jane!\#$%'*+-/=?^_{|}~Doe' j...@example.com ok 11 - same output : j...@example.com ok 12 - same output : Jane j...@example.com ok 13 - same output : Jane Doe jdoe@ example.com ok 14 - same output : Jane Doe j...@example.com ok 15 - same output : Jane @ Doe @ Jane @ Doe ok 16 - same output : Jane, 'Doe' j...@example.com ok 17 - same output : 'Doe, Jane' j...@example.com ok 18 - same output : Jane Doe j...@example.com ok 19 - same output : Jane' Doe j...@example.com ok 20 - same output : Jane Doe j...@example.com j...@example.com ok 21 - same output : Jane\ Doe j...@example.com ok 22 - same output : Doe, jane j...@example.com ok 23 - same output : Jane Doe j...@example.com ok 24 - same output : 'Jane 'Doe' j...@example.com not ok 25 - same output : Jane\ Doe j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Jane\ Doe j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane \ Doe j...@example.com' # $expected-[0] = 'Jane\ Doe j...@example.com' not ok 26 - same output : Doe, Jane j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Doe, Jane j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Doe, Ja ne j...@example.com' # $expected-[0] = 'Doe, Ja ne j...@example.com' not ok 27 - same output : Doe, Katarina Jane j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Doe, Katarina Jane j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Doe, Katarina Jane j...@example.com' # $expected-[0] = 'Doe, Katarina Jane j...@example.com' not ok 28 - same output : Jane@:;\.,()Doe j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Jane@:;\.,()Doe j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[1] = '\.' # $expected-[1] = '\.' not ok 29 - same output : Jane j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Jane j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane' # $expected-[0] = 'janej...@example.com' not ok 30 - same output : j...@example.com Jane Doe # TODO known breakage # Failed (TODO) test 'same output : j...@example.com Jane Doe' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane Doe j...@example.com' # $expected-[0] = 'jdoe@example.comJaneDoe' not ok 31 - same output : Jane j...@example.com Doe # TODO known breakage # Failed (TODO) test 'same output : Jane j...@example.com Doe' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane Doe j...@example.com' # $expected-[0] = 'Jane jdoe@example.comDoe' not ok 32 - same output : Jane Kata rina ,Doe j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Jane Kata rina ,Doe j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane Kat a ri na ,Doe j...@example.com' # $expected-[0] = 'Jane Kat a ri na ,Doe j...@example.com' not ok 33 - same output : Jane Doe # TODO known breakage # Failed (TODO) test 'same output : Jane Doe' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane' # $expected-[0] = 'Jane Doe' not ok 34 - same output : Jane Doe j...@example.com # TODO known breakage # Failed (TODO) test 'same output : Jane Doe j...@example.com' # at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62. # Structures begin differing at: # $got-[0] = 'Jane' # $expected-[0] = 'Jane Doe
Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
Torsten Bögershausen tbo...@web.de writes: #!/usr/bin/perl Should we have hard-coded PATH to perl here ? This is what is done in other tests: $ head t*/test.pl -n 1 == t0202/test.pl == #!/usr/bin/perl == t9000/test.pl == #!/usr/bin/perl == t9700/test.pl == #!/usr/bin/perl We actually don't use it when running the testsuite properly, since we call perl $TEST_DIRECTORY/t9000/test.pl and perl is defined as perl () { command $PERL_PATH $@ } So, it's OK. /usr/bin/perl --version This is perl, v5.10.0 built for darwin-thread-multi-2level (with 2 registered patches, see perl -V for more detail) + +my $is_passing = Test::More-builder-is_passing; +exit($is_passing ? 0 : 1); This seems to give problems: debug=t verbose=t ./t9000-addresses.sh Indeed, is_passing seems too recent for your version of perl. A similar problem was solved in t9700 by 635155f (t9700: Use Test::More-builder, not $Test::Builder::Test, 2010-06-26). I'll use the same solution: my $is_passing = eval { Test::More-is_passing }; exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/; -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Sorry for not having resent the patches myself, I currently have no Internet access, neither at work nor at home... Here's a try on my phone: I though the copyright line was necessary, but I did not know what to write after, and I forgot to ask, so I'm really happy with simply removing it. :) OK, so Junio, you can just remove it. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Sorry for not having resent the patches myself, I currently have no Internet access, neither at work nor at home... Here's a try on my phone: I though the copyright line was necessary, but I did not know what to write after, and I forgot to ask, so I'm really happy with simply removing it. :) Thanks! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
From: Remi Lespinet remi.lespi...@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 Doe j...@example.com In this case the result of parse_address_line is: With M::A : Jane Do e j...@example.com Without : Jane Do e j...@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.lespi...@ensimag.grenoble-inp.fr Signed-off-by: Matthieu Moy matthieu@imag.fr --- git-send-email.perl | 2 +- perl/Git.pm | 67 t/t9000-addresses.sh | 30 +++ t/t9000/test.pl | 67 4 files changed, 165 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 49fc275..4268ed9 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..19ef081 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -864,6 +864,73 @@ sub ident_person { return $ident[0] $ident[1]; } +=item parse_mailboxes + +Return 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; +} =item hash_object ( TYPE, FILENAME ) diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 000..7223d03 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,30 @@ +#!/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,
Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
Matthieu Moy matthieu@imag.fr writes: diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 000..7223d03 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# +# Copyright (c) 2015 That does not look like a valid copyright notice. In the modern age, I'd personally perfer not to add one (I would not have a strong objection to others asserting their copyright), but if you want to add one, you would need the name of the copyright holder after the year (I presume that it would be your school name?). IIRC, (c) in place of circle-C does no carry legal weight, but having the word Copyright spelled out there is sufficient. Thanks for tying the loose ends (not just this topic, but the other ones, too). Very much appreciated. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 000..7223d03 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# +# Copyright (c) 2015 That does not look like a valid copyright notice. In the modern age, I'd personally perfer not to add one I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Junio: do you want me to resend? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html