[PATCH v7 00/10] send-email address management

[PATCH v7 00/10] send-email address management

[Matthieu Moy]

This is an almost unmodified resend of Remi's patch here:

http://thread.gmane.org/gmane.comp.version-control.git/271844/focus=272499

The last patches had trouble reaching the list, hopefully this will be
easier to apply. Two minor changes:

* Removed the Helped-by: Remi trailer in a message sent by the same
  Remi.

* Allow -> allow in the subject line of a patch.

No code change.

Remi Lespinet (10):
  t9001-send-email: move script creation in a setup test
  send-email: allow aliases in patch header and command script outputs
  t9001-send-email: refactor header variable fields replacement
  send-email: refactor address list process
  send-email: allow use of aliases in the From field of --compose mode
  send-email: minor code refactoring
  send-email: reduce dependencies impact on parse_address_line
  send-email: consider quote as delimiter instead of character
  send-email: allow multiple emails using --cc, --to and --bcc
  send-email: suppress meaningless whitespaces in from field

 Documentation/git-send-email.txt |  12 +--
 git-send-email.perl              |  50 ++++++-------
 perl/Git.pm                      |  67 +++++++++++++++++
 t/t9000-addresses.sh             |  30 ++++++++
 t/t9000/test.pl                  |  67 +++++++++++++++++
 t/t9001-send-email.sh            | 154 ++++++++++++++++++++++++++++++++++++---
 6 files changed, 336 insertions(+), 44 deletions(-)
 create mode 100755 t/t9000-addresses.sh
 create mode 100755 t/t9000/test.pl

-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 01/10] t9001-send-email: move script creation in a setup test

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Move the creation of the scripts used in to-cmd and cc-cmd tests
in a setup test to make them available for later tests.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t9001-send-email.sh | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index db2f45e..8caf7b0 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -312,13 +312,19 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
 	)
 '
 
+test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+	write_script tocmd-sed <<-\EOF &&
+	sed -n -e "s/^tocmd--//p" "$1"
+	EOF
+	write_script cccmd-sed <<-\EOF
+	sed -n -e "s/^cccmd--//p" "$1"
+	EOF
+'
+
 test_expect_success $PREREQ 'tocmd works' '
 	clean_fake_sendmail &&
 	cp $patches tocmd.patch &&
 	echo tocmd--tocmd@example.com >>tocmd.patch &&
-	write_script tocmd-sed <<-\EOF &&
-	sed -n -e "s/^tocmd--//p" "$1"
-	EOF
 	git send-email \
 		--from="Example <nobody@example.com>" \
 		--to-cmd=./tocmd-sed \
@@ -332,9 +338,6 @@ test_expect_success $PREREQ 'cccmd works' '
 	clean_fake_sendmail &&
 	cp $patches cccmd.patch &&
 	echo "cccmd--  cccmd@example.com" >>cccmd.patch &&
-	write_script cccmd-sed <<-\EOF &&
-	sed -n -e "s/^cccmd--//p" "$1"
-	EOF
 	git send-email \
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 02/10] send-email: allow aliases in patch header and command script outputs

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Interpret aliases in:

  -  Header fields of patches generated by git format-patch
     (using --to, --cc, --add-header for example) or
     manually modified. Example of fields in header:

      To: alias1
      Cc: alias2
      Cc: alias3

  -  Outputs of command scripts specified by --cc-cmd and
     --to-cmd. Example of script:

      #!/bin/sh
      echo alias1
      echo alias2

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl   |  2 ++
 t/t9001-send-email.sh | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index ae9f869..3cbdb1a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1566,7 +1566,9 @@ foreach my $t (@files) {
 		($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
 	$needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc);
 
+	@to = expand_aliases(@to);
 	@to = validate_address_list(sanitize_address_list(@to));
+	@cc = expand_aliases(@cc);
 	@cc = validate_address_list(sanitize_address_list(@cc));
 
 	@to = (@initial_to, @to);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 8caf7b0..1914439 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1624,6 +1624,66 @@ test_sendmail_aliases 'sendmail aliases tolerate bogus line folding' \
 test_sendmail_aliases 'sendmail aliases empty' alice bcgrp <<-\EOF
 	EOF
 
+test_expect_success $PREREQ 'alias support in To header' '
+	clean_fake_sendmail &&
+	echo "alias sbd  someone@example.org" >.mailrc &&
+	test_config sendemail.aliasesfile ".mailrc" &&
+	test_config sendemail.aliasfiletype mailrc &&
+	git format-patch --stdout -1 --to=sbd >aliased.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		aliased.patch \
+		2>errors >out &&
+	grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'alias support in Cc header' '
+	clean_fake_sendmail &&
+	echo "alias sbd  someone@example.org" >.mailrc &&
+	test_config sendemail.aliasesfile ".mailrc" &&
+	test_config sendemail.aliasfiletype mailrc &&
+	git format-patch --stdout -1 --cc=sbd >aliased.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		aliased.patch \
+		2>errors >out &&
+	grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'tocmd works with aliases' '
+	clean_fake_sendmail &&
+	echo "alias sbd  someone@example.org" >.mailrc &&
+	test_config sendemail.aliasesfile ".mailrc" &&
+	test_config sendemail.aliasfiletype mailrc &&
+	git format-patch --stdout -1 >tocmd.patch &&
+	echo tocmd--sbd >>tocmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to-cmd=./tocmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		tocmd.patch \
+		2>errors >out &&
+	grep "^!someone@example\.org!$" commandline1
+'
+
+test_expect_success $PREREQ 'cccmd works with aliases' '
+	clean_fake_sendmail &&
+	echo "alias sbd  someone@example.org" >.mailrc &&
+	test_config sendemail.aliasesfile ".mailrc" &&
+	test_config sendemail.aliasfiletype mailrc &&
+	git format-patch --stdout -1 >cccmd.patch &&
+	echo cccmd--sbd >>cccmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--cc-cmd=./cccmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		cccmd.patch \
+		2>errors >out &&
+	grep "^!someone@example\.org!$" commandline1
+'
+
 do_xmailer_test () {
 	expected=$1 params=$2 &&
 	git format-patch -1 &&
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 03/10] t9001-send-email: refactor header variable fields replacement

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Create a function which replaces Date, Message-Id and
X-Mailer lines generated by git-send-email by a specific string:

Date:.*$       -> Date: DATE-STRING
Message-Id:.*$ -> Message-Id: MESSAGE-ID-STRING
X-Mailer:.*$   -> X-Mailer: X-MAILER-STRING
Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 t/t9001-send-email.sh | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1914439..fce081c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -522,6 +522,12 @@ Result: OK
 EOF
 "
 
+replace_variable_fields () {
+	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/"
+}
+
 test_suppression () {
 	git send-email \
 		--dry-run \
@@ -529,10 +535,7 @@ test_suppression () {
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
-		$patches |
-	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
-		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
-		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		$patches | replace_variable_fields \
 		>actual-suppress-$1${2+"-$2"} &&
 	test_cmp expected-suppress-$1${2+"-$2"} actual-suppress-$1${2+"-$2"}
 }
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 04/10] send-email: refactor address list process

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Simplify code by creating a function which transform a list of strings
containing email addresses (separated by commas, comporting aliases)
into a clean list of valid email addresses.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3cbdb1a..994697e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -839,12 +839,9 @@ sub expand_one_alias {
 	return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
 }
 
-@initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
-@initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
-@bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@initial_to = process_address_list(@initial_to);
+@initial_cc = process_address_list(@initial_cc);
+@bcclist = process_address_list(@bcclist);
 
 if ($thread && !defined $initial_reply_to && $prompting) {
 	$initial_reply_to = ask(
@@ -1057,6 +1054,13 @@ sub sanitize_address_list {
 	return (map { sanitize_address($_) } @_);
 }
 
+sub process_address_list {
+	my @addr_list = expand_aliases(@_);
+	@addr_list = sanitize_address_list(@addr_list);
+	@addr_list = validate_address_list(@addr_list);
+	return @addr_list;
+}
+
 # Returns the local Fully Qualified Domain Name (FQDN) if available.
 #
 # Tightly configured MTAa require that a caller sends a real DNS
@@ -1566,10 +1570,8 @@ foreach my $t (@files) {
 		($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
 	$needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc);
 
-	@to = expand_aliases(@to);
-	@to = validate_address_list(sanitize_address_list(@to));
-	@cc = expand_aliases(@cc);
-	@cc = validate_address_list(sanitize_address_list(@cc));
+	@to = process_address_list(@to);
+	@cc = process_address_list(@cc);
 
 	@to = (@initial_to, @to);
 	@cc = (@initial_cc, @cc);
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 05/10] send-email: allow use of aliases in the From field of --compose mode

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Aliases were expanded before considering the From field of the
--compose option. This is inconsistent with other fields
(To, Cc, ...) which already support aliases.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 994697e..da1d4a4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -561,8 +561,6 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
 	}
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
 # is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
 # $f is a revision list specification to be passed to format-patch.
 sub is_format_patch_arg {
@@ -807,6 +805,8 @@ if (!$force) {
 	}
 }
 
+($sender) = expand_aliases($sender) if defined $sender;
+
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
 }
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 06/10] send-email: minor code refactoring

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Group expressions in a single if statement. This avoid checking
multiple time if the variable $sender is defined.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index da1d4a4..49fc275 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -805,9 +805,9 @@ if (!$force) {
 	}
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+	($sender) = expand_aliases($sender);
+} else {
 	$sender = $repoauthor || $repocommitter || '';
 }
 
-- 
2.5.0.rc0.10.g7792c2a

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

[Matthieu Moy]

From: Remi Lespinet <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>
Signed-off-by: Matthieu Moy <Matthieu.Moy@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 0000000..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, skipping test"
+	test_done
+}
+
+perl -MMail::Address -e 0 2>/dev/null || {
+	skip_all="Perl Mail::Address 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..8e2b760
--- /dev/null
+++ b/t/t9000/test.pl
@@ -0,0 +1,67 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use 5.008;
+use warnings;
+use strict;
+
+use Test::More qw(no_plan);
+use Mail::Address;
+
+BEGIN { use_ok('Git') }
+
+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);
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 08/10] send-email: consider quote as delimiter instead of character

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Do not consider quote inside a recipient name as character when
they are not escaped. This interprets:

  "Jane" "Doe" <jdoe@example.com>

as:

  "Jane Doe" <jdoe@example.com>

instead of:

  "Jane\" \"Doe" <jdoe@example.com>

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 4268ed9..df9d3f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1034,15 +1034,17 @@ sub sanitize_address {
 		return $recipient;
 	}
 
+	# remove non-escaped quotes
+	$recipient_name =~ s/(^|[^\\])"/$1/g;
+
 	# rfc2047 is needed if a non-ascii char is included
 	if ($recipient_name =~ /[^[:ascii:]]/) {
-		$recipient_name =~ s/^"(.*)"$/$1/;
 		$recipient_name = quote_rfc2047($recipient_name);
 	}
 
 	# double quotes are needed if specials or CTLs are included
 	elsif ($recipient_name =~ /[][()<>@,;:\\".\000-\037\177]/) {
-		$recipient_name =~ s/(["\\\r])/\\$1/g;
+		$recipient_name =~ s/([\\\r])/\\$1/g;
 		$recipient_name = qq["$recipient_name"];
 	}
 
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 09/10] send-email: allow multiple emails using --cc, --to and --bcc

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Accept a list of emails separated by commas in flags --cc, --to and
--bcc.  Multiple addresses can already be given by using these options
multiple times, but it is more convenient to allow cutting-and-pasting
a list of addresses from the header of an existing e-mail message,
which already lists them as comma-separated list, as a value to a
single parameter.

The following format can now be used:

    $ git send-email --to='Jane <jdoe@example.com>, mike@example.com'

Remove the limitation imposed by 79ee555b (Check and document the
options to prevent mistakes, 2006-06-21) which rejected every argument
with comma in --cc, --to and --bcc.

Signed-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>
Signed-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-send-email.txt | 12 +++++------
 git-send-email.perl              | 17 ++--------------
 t/t9001-send-email.sh            | 44 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 7ae467b..f14705e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,17 @@ Composing
 	of 'sendemail.annotate'. See the CONFIGURATION section for
 	'sendemail.multiEdit'.
 
---bcc=<address>::
+--bcc=<address>,...::
 	Specify a "Bcc:" value for each email. Default is the value of
 	'sendemail.bcc'.
 +
-The --bcc option must be repeated for each user you want on the bcc list.
+This option may be specified multiple times.
 
---cc=<address>::
+--cc=<address>,...::
 	Specify a starting "Cc:" value for each email.
 	Default is the value of 'sendemail.cc'.
 +
-The --cc option must be repeated for each user you want on the cc list.
+This option may be specified multiple times.
 
 --compose::
 	Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -110,13 +110,13 @@ is not set, this will be prompted for.
 	Only necessary if --compose is also set.  If --compose
 	is not set, this will be prompted for.
 
---to=<address>::
+--to=<address>,...::
 	Specify the primary recipient of the emails generated. Generally, this
 	will be the upstream maintainer of the project involved. Default is the
 	value of the 'sendemail.to' configuration value; if that is unspecified,
 	and --to-cmd is not specified, this will be prompted for.
 +
-The --to option must be repeated for each user you want on the to list.
+This option may be specified multiple times.
 
 --8bit-encoding=<encoding>::
 	When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index df9d3f6..4a681f5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -460,20 +460,6 @@ my ($repoauthor, $repocommitter);
 ($repoauthor) = Git::ident_person(@repo, 'author');
 ($repocommitter) = Git::ident_person(@repo, 'committer');
 
-# Verify the user input
-
-foreach my $entry (@initial_to) {
-	die "Comma in --to entry: $entry'\n" unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
-	die "Comma in --cc entry: $entry'\n" unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
-	die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/;
-}
-
 sub parse_address_line {
 	if ($have_mail_address) {
 		return map { $_->format } Mail::Address->parse($_[0]);
@@ -1057,7 +1043,8 @@ sub sanitize_address_list {
 }
 
 sub process_address_list {
-	my @addr_list = expand_aliases(@_);
+	my @addr_list = map { parse_address_line($_) } @_;
+	@addr_list = expand_aliases(@addr_list);
 	@addr_list = sanitize_address_list(@addr_list);
 	@addr_list = validate_address_list(@addr_list);
 	return @addr_list;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index fce081c..733431b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1720,4 +1720,48 @@ test_expect_success $PREREQ '--[no-]xmailer with sendemail.xmailer=false' '
 	do_xmailer_test 1 "--xmailer"
 '
 
+test_expect_success $PREREQ 'setup expected-list' '
+	git send-email \
+	--dry-run \
+	--from="Example <from@example.com>" \
+	--to="To 1 <to1@example.com>" \
+	--to="to2@example.com" \
+	--to="to3@example.com" \
+	--cc="Cc 1 <cc1@example.com>" \
+	--cc="Cc2 <cc2@example.com>" \
+	--bcc="bcc1@example.com" \
+	--bcc="bcc2@example.com" \
+	0001-add-master.patch | replace_variable_fields \
+	>expected-list
+'
+
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '
+	git send-email \
+	--dry-run \
+	--from="Example <from@example.com>" \
+	--to="To 1 <to1@example.com>, to2@example.com" \
+	--to="to3@example.com" \
+	--cc="Cc 1 <cc1@example.com>, Cc2 <cc2@example.com>" \
+	--bcc="bcc1@example.com, bcc2@example.com" \
+	0001-add-master.patch | replace_variable_fields \
+	>actual-list &&
+	test_cmp expected-list actual-list
+'
+
+test_expect_success $PREREQ 'aliases work with email list' '
+	echo "alias to2 to2@example.com" >.mutt &&
+	echo "alias cc1 Cc 1 <cc1@example.com>" >>.mutt &&
+	test_config sendemail.aliasesfile ".mutt" &&
+	test_config sendemail.aliasfiletype mutt &&
+	git send-email \
+	--dry-run \
+	--from="Example <from@example.com>" \
+	--to="To 1 <to1@example.com>, to2, to3@example.com" \
+	--cc="cc1, Cc2 <cc2@example.com>" \
+	--bcc="bcc1@example.com, bcc2@example.com" \
+	0001-add-master.patch | replace_variable_fields \
+	>actual-list &&
+	test_cmp expected-list actual-list
+'
+
 test_done
-- 
2.5.0.rc0.10.g7792c2a

[PATCH v7 10/10] send-email: suppress meaningless whitespaces in from field

[Matthieu Moy]

From: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>

Remove leading and trailing whitespaces in from field before
interepreting it to improve consistency with other options.  The
split_addrs function already take care of trailing and leading
whitespaces for to, cc and bcc fields.
The from option now:

 - has the same behavior when passing arguments like
   "  jdoe@example.com ", "\t jdoe@example.com " or
   "jdoe@example.com".

 - interprets aliases in string containing leading and trailing
   whitespaces such as " alias" or "alias\t" like other options.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-send-email.perl   |  1 +
 t/t9001-send-email.sh | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 4a681f5..b660cc2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -792,6 +792,7 @@ if (!$force) {
 }
 
 if (defined $sender) {
+	$sender =~ s/^\s+|\s+$//g;
 	($sender) = expand_aliases($sender);
 } else {
 	$sender = $repoauthor || $repocommitter || '';
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 733431b..5b4a5ce 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1764,4 +1764,28 @@ test_expect_success $PREREQ 'aliases work with email list' '
 	test_cmp expected-list actual-list
 '
 
+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
+	echo "alias to2 to2@example.com" >.mutt &&
+	echo "alias cc1 Cc 1 <cc1@example.com>" >>.mutt &&
+	test_config sendemail.aliasesfile ".mutt" &&
+	test_config sendemail.aliasfiletype mutt &&
+	TO1=$(echo "QTo 1 <to1@example.com>" | q_to_tab) &&
+	TO2=$(echo "QZto2" | qz_to_tab_space) &&
+	CC1=$(echo "cc1" | append_cr) &&
+	BCC1=$(echo "Q bcc1@example.com Q" | q_to_nul) &&
+	git send-email \
+	--dry-run \
+	--from="	Example <from@example.com>" \
+	--to="$TO1" \
+	--to="$TO2" \
+	--to="  to3@example.com   " \
+	--cc="$CC1" \
+	--cc="Cc2 <cc2@example.com>" \
+	--bcc="$BCC1" \
+	--bcc="bcc2@example.com" \
+	0001-add-master.patch | replace_variable_fields \
+	>actual-list &&
+	test_cmp expected-list actual-list
+'
+
 test_done
-- 
2.5.0.rc0.10.g7792c2a

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

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
> new file mode 100755
> index 0000000..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.

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

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
>> new file mode 100755
>> index 0000000..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/

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

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>>
>>> diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
>>> new file mode 100755
>>> index 0000000..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?

It would be simpler to amend in-place either way.  Just let me know
if we want to drop it or add a name (and if the latter, what name).

Thanks.

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

[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!

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

[Matthieu Moy]

Remi Lespinet <remi.lespinet@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/

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

[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 : jdoe@example.com
ok 4 - same output : <jdoe@example.com>
ok 5 - same output : Jane <jdoe@example.com>
ok 6 - same output : Jane Doe <jdoe@example.com>
ok 7 - same output : "Jane" <jdoe@example.com>
ok 8 - same output : "Doe, Jane" <jdoe@example.com>
ok 9 - same output : "Jane@:;\>.,()<Doe" <jdoe@example.com>
ok 10 - same output : Jane!\#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
ok 11 - same output : "<jdoe@example.com>"
ok 12 - same output : "Jane jdoe@example.com"
ok 13 - same output : Jane Doe <jdoe    @   example.com  >
ok 14 - same output : Jane       Doe <  jdoe@example.com  >
ok 15 - same output : Jane @ Doe @ Jane @ Doe
ok 16 - same output : "Jane, 'Doe'" <jdoe@example.com>
ok 17 - same output : 'Doe, "Jane' <jdoe@example.com>
ok 18 - same output : "Jane" "Do"e <jdoe@example.com>
ok 19 - same output : "Jane' Doe" <jdoe@example.com>
ok 20 - same output : "Jane Doe <jdoe@example.com>" <jdoe@example.com>
ok 21 - same output : "Jane\" Doe" <jdoe@example.com>
ok 22 - same output : Doe, jane <jdoe@example.com>
ok 23 - same output : "Jane Doe <jdoe@example.com>
ok 24 - same output : 'Jane 'Doe' <jdoe@example.com>
not ok 25 - same output : Jane\ Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane\ Doe <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Jane \ Doe" <jdoe@example.com>'
#     $expected->[0] = '"Jane\ Doe" <jdoe@example.com>'
not ok 26 - same output : "Doe, Ja"ne <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : "Doe, Ja"ne <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Doe, Ja" ne <jdoe@example.com>'
#     $expected->[0] = '"Doe, Ja ne" <jdoe@example.com>'
not ok 27 - same output : "Doe, Katarina" Jane <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : "Doe, Katarina" Jane <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Doe, Katarina" Jane <jdoe@example.com>'
#     $expected->[0] = '"Doe, Katarina Jane" <jdoe@example.com>'
not ok 28 - same output : Jane@:;\.,()<>Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane@:;\.,()<>Doe <jdoe@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 jdoe@example.com # TODO known breakage
#   Failed (TODO) test 'same output : Jane jdoe@example.com'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane'
#     $expected->[0] = 'Janejdoe@example.com'
not ok 30 - same output : <jdoe@example.com> Jane Doe # TODO known breakage
#   Failed (TODO) test 'same output : <jdoe@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 <jdoe@example.com>'
#     $expected->[0] = 'jdoe@example.comJaneDoe'
not ok 31 - same output : Jane <jdoe@example.com> Doe # TODO known breakage
#   Failed (TODO) test 'same output : Jane <jdoe@example.com> Doe'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane Doe <jdoe@example.com>'
#     $expected->[0] = 'Jane <jdoe@example.comDoe>'
not ok 32 - same output : "Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : "Jane "Kat"a" ri"na" ",Doe" <jdoe@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" <jdoe@example.com>'
#     $expected->[0] = '"Jane  Kat a ri na ,Doe" <jdoe@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 <jdoe@example.com>" # TODO known breakage
#   Failed (TODO) test 'same output : Jane "Doe <jdoe@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 <jdoe@example.com>"'
not ok 35 - same output : \"Jane Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : \"Jane Doe <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '\ " Jane Doe <jdoe@example.com>'
#     $expected->[0] = '"\"Jane Doe" <jdoe@example.com>'
not ok 36 - same output : Jane\"\" Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane\"\" Doe <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane \ " \ " Doe <jdoe@example.com>'
#     $expected->[0] = '"Jane\"\" Doe" <jdoe@example.com>'
not ok 37 - same output : 'Jane "Katarina\" \' Doe' <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : 'Jane "Katarina\" \' Doe' <jdoe@example.com>'
#   at /Users/me/projects/git/git.pu/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = ''Jane " Katarina \ " \ ' Doe' <jdoe@example.com>'
#     $expected->[0] = '"'Jane  Katarina\" \' Doe'" <jdoe@example.com>'
1..37
# test_external test Perl address parsing function failed: perl /Users/me/projects/git/git.pu/t/t9000/test.pl
# expecting no stderr from previous command
# test_external_without_stderr test no stderr: Perl address parsing function failed: perl /Users/me/projects/git/git.pu/t/t9000/test.pl: 
# Stderr is:
Can't locate object method "is_passing" via package "Test::Builder" at /Users/me/projects/git/git.pu/t/t9000/test.pl line 66.
# Looks like your test died just after 37.

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

[Matthieu Moy]

Torsten Bögershausen <tboegi@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/

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

[Matthieu Moy]

From: Remi Lespinet <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>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Since v7: portability fix on the final exit() in the perl script.

 git-send-email.perl  |  2 +-
 perl/Git.pm          | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t9000-addresses.sh | 27 +++++++++++++++++++++
 t/t9000/test.pl      | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 162 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 09ecad8..486cb36 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 0000000..a1ebef6
--- /dev/null
+++ b/t/t9000-addresses.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+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
+}
+
+perl -MMail::Address -e 0 2>/dev/null || {
+	skip_all="Perl Mail::Address 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..2d05d3e
--- /dev/null
+++ b/t/t9000/test.pl
@@ -0,0 +1,67 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use 5.008;
+use warnings;
+use strict;
+
+use Test::More qw(no_plan);
+use Mail::Address;
+
+BEGIN { use_ok('Git') }
+
+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 = eval { Test::More->is_passing };
+exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;
-- 
2.5.0.rc0.7.ge1edd74.dirty

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

[Torsten Bögershausen]

(Thanks for the quick reply.
Sorry for the noise about
#!/usr/bin/perl
of course we call the right perl)

The new patch seems to be integrated in pu (I tested d08caa8e022f08d)
Test seems to pass, but some noise is on the channel:


Initialized empty Git repository in
/Users/tb/NoBackup/projects/git/git.next/t/trash directory.t9000-addresses/.git/
# run 0: Perl address parsing function (perl
/Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl)
ok 1 - use Git;
ok 2 - same output : Jane
ok 3 - same output : jdoe@example.com
ok 4 - same output : <jdoe@example.com>
ok 5 - same output : Jane <jdoe@example.com>
ok 6 - same output : Jane Doe <jdoe@example.com>
ok 7 - same output : "Jane" <jdoe@example.com>
ok 8 - same output : "Doe, Jane" <jdoe@example.com>
ok 9 - same output : "Jane@:;\>.,()<Doe" <jdoe@example.com>
ok 10 - same output : Jane!\#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
ok 11 - same output : "<jdoe@example.com>"
ok 12 - same output : "Jane jdoe@example.com"
ok 13 - same output : Jane Doe <jdoe    @   example.com  >
ok 14 - same output : Jane       Doe <  jdoe@example.com  >
ok 15 - same output : Jane @ Doe @ Jane @ Doe
ok 16 - same output : "Jane, 'Doe'" <jdoe@example.com>
ok 17 - same output : 'Doe, "Jane' <jdoe@example.com>
ok 18 - same output : "Jane" "Do"e <jdoe@example.com>
ok 19 - same output : "Jane' Doe" <jdoe@example.com>
ok 20 - same output : "Jane Doe <jdoe@example.com>" <jdoe@example.com>
ok 21 - same output : "Jane\" Doe" <jdoe@example.com>
ok 22 - same output : Doe, jane <jdoe@example.com>
ok 23 - same output : "Jane Doe <jdoe@example.com>
ok 24 - same output : 'Jane 'Doe' <jdoe@example.com>
not ok 25 - same output : Jane\ Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane\ Doe <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Jane \ Doe" <jdoe@example.com>'
#     $expected->[0] = '"Jane\ Doe" <jdoe@example.com>'
not ok 26 - same output : "Doe, Ja"ne <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : "Doe, Ja"ne <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Doe, Ja" ne <jdoe@example.com>'
#     $expected->[0] = '"Doe, Ja ne" <jdoe@example.com>'
not ok 27 - same output : "Doe, Katarina" Jane <jdoe@example.com> # TODO known
breakage
#   Failed (TODO) test 'same output : "Doe, Katarina" Jane <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Doe, Katarina" Jane <jdoe@example.com>'
#     $expected->[0] = '"Doe, Katarina Jane" <jdoe@example.com>'
not ok 28 - same output : Jane@:;\.,()<>Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane@:;\.,()<>Doe <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[1] = '\.'
#     $expected->[1] = '"\."'
not ok 29 - same output : Jane jdoe@example.com # TODO known breakage
#   Failed (TODO) test 'same output : Jane jdoe@example.com'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane'
#     $expected->[0] = 'Janejdoe@example.com'
not ok 30 - same output : <jdoe@example.com> Jane Doe # TODO known breakage
#   Failed (TODO) test 'same output : <jdoe@example.com> Jane Doe'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane Doe <jdoe@example.com>'
#     $expected->[0] = 'jdoe@example.comJaneDoe'
not ok 31 - same output : Jane <jdoe@example.com> Doe # TODO known breakage
#   Failed (TODO) test 'same output : Jane <jdoe@example.com> Doe'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane Doe <jdoe@example.com>'
#     $expected->[0] = 'Jane <jdoe@example.comDoe>'
not ok 32 - same output : "Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com> # TODO
known breakage
#   Failed (TODO) test 'same output : "Jane "Kat"a" ri"na" ",Doe"
<jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '"Jane " Kat "a" ri "na" ",Doe" <jdoe@example.com>'
#     $expected->[0] = '"Jane  Kat a ri na ,Doe" <jdoe@example.com>'
not ok 33 - same output : Jane Doe # TODO known breakage
#   Failed (TODO) test 'same output : Jane Doe'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane'
#     $expected->[0] = 'Jane Doe'
not ok 34 - same output : Jane "Doe <jdoe@example.com>" # TODO known breakage
#   Failed (TODO) test 'same output : Jane "Doe <jdoe@example.com>"'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane'
#     $expected->[0] = '"Jane Doe <jdoe@example.com>"'
not ok 35 - same output : \"Jane Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : \"Jane Doe <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = '\ " Jane Doe <jdoe@example.com>'
#     $expected->[0] = '"\"Jane Doe" <jdoe@example.com>'
not ok 36 - same output : Jane\"\" Doe <jdoe@example.com> # TODO known breakage
#   Failed (TODO) test 'same output : Jane\"\" Doe <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = 'Jane \ " \ " Doe <jdoe@example.com>'
#     $expected->[0] = '"Jane\"\" Doe" <jdoe@example.com>'
not ok 37 - same output : 'Jane "Katarina\" \' Doe' <jdoe@example.com> # TODO
known breakage
#   Failed (TODO) test 'same output : 'Jane "Katarina\" \' Doe' <jdoe@example.com>'
#   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
#     Structures begin differing at:
#          $got->[0] = ''Jane " Katarina \ " \ ' Doe' <jdoe@example.com>'
#     $expected->[0] = '"'Jane  Katarina\" \' Doe'" <jdoe@example.com>'
1..37
# test_external test Perl address parsing function was ok
# expecting no stderr from previous command
# test_external_without_stderr test no stderr: Perl address parsing function was ok

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

[Matthieu Moy]

Torsten Bögershausen <tboegi@web.de> writes:

> (Thanks for the quick reply.
> Sorry for the noise about
> #!/usr/bin/perl
> of course we call the right perl)

No problem.

> The new patch seems to be integrated in pu (I tested d08caa8e022f08d)
> Test seems to pass,

Good.

> but some noise is on the channel:

Are you talking about the output of "not ok ... # TODO known breakage"
part, like this:

> ok 24 - same output : 'Jane 'Doe' <jdoe@example.com>
> not ok 25 - same output : Jane\ Doe <jdoe@example.com> # TODO known breakage
> #   Failed (TODO) test 'same output : Jane\ Doe <jdoe@example.com>'
> #   at /Users/tb/NoBackup/projects/git/git.next/t/t9000/test.pl line 62.
> #     Structures begin differing at:
> #          $got->[0] = '"Jane \ Doe" <jdoe@example.com>'
> #     $expected->[0] = '"Jane\ Doe" <jdoe@example.com>'

?

If so, it seems to be the way Perl's Test::More works with TODO blocks.
I am reluctant to try to change it for us.


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/