Re: [PATCH v7 07/10] send-email: reduce dependencies impact on parse_address_line

2015-07-07 Thread Torsten Bögershausen
#!/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

2015-07-07 Thread Matthieu Moy
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

2015-07-01 Thread Matthieu Moy
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

2015-07-01 Thread Remi Lespinet
 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

2015-06-30 Thread Matthieu Moy
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

2015-06-30 Thread Junio C Hamano
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

2015-06-30 Thread Matthieu Moy
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