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

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

[Remi Lespinet]

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>
---
 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 a3663da..eef12e6 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 \
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 6bedf74..8bf38ee 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1560,7 +1560,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 eef12e6..f7d4132 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1579,6 +1579,66 @@ test_expect_success $PREREQ 'sendemail.aliasfiletype=sendmail' '
 	grep "^!o@example\.com!$" commandline1
 '
 
+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 &&
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 f7d4132..714fcae 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"}
 }
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 8bf38ee..2d5c530 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -833,12 +833,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(
@@ -1051,6 +1048,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
@@ -1560,10 +1564,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);
-- 
1.9.1

[PATCH/RFC v4 05/10] send-email: Allow use of aliases in the From field of --compose mode

[Remi Lespinet]

Aliases were expanded before checking 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>
---
 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 2d5c530..f61449d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -555,8 +555,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 {
@@ -801,6 +799,8 @@ if (!$force) {
 	}
 }
 
+($sender) = expand_aliases($sender) if defined $sender;
+
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
 }
-- 
1.9.1

Re: [PATCH/RFC v4 05/10] send-email: Allow use of aliases in the From field of --compose mode

[Matthieu Moy]

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

> Aliases were expanded before checking the From field of the

"checking" is misleading here. I thought you meant "check that the From
field is well-formed", while you mean "set $sender based on the From:
field".

> --compose option. This is inconsistent with other fields
> (To, Cc, ...) which already support aliases.

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

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

[Remi Lespinet]

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>
---
 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 f61449d..a0cd7ff 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,9 +799,9 @@ if (!$force) {
 	}
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+	($sender) = expand_aliases($sender);
+} else {
 	$sender = $repoauthor || $repocommitter || '';
 }
 
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 git-send-email.perl | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index a0cd7ff..a1f6c18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -477,9 +477,59 @@ foreach my $entry (@bcclist) {
 sub parse_address_line {
 	if ($have_mail_address) {
 		return map { $_->format } Mail::Address->parse($_[0]);
-	} else {
-		return split_addrs($_[0]);
 	}
+
+	my $commentrgx=qr/\((?:[^)]*)\)/;
+	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
+	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
+	my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;
+
+	my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
+	push @tokens, ",";
+
+	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
+	foreach my $token (@tokens) {
+	    if ($token =~ /^[,;]$/) {
+		if (@address) {
+		    push @address, @buffer;
+		} else {
+		    push @phrase, @buffer;
+		}
+
+		my $str_phrase = join ' ', @phrase;
+		my $str_address = join '', @address;
+		my $str_comment = join ' ', @comment;
+
+		if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
+		    $str_phrase =~ s/(^|[^\\])"/$1/g;
+		    $str_phrase = qq["$str_phrase"];
+		}
+
+		if ($str_address ne "" && $str_phrase ne "") {
+		    $str_address = qq[<$str_address>];
+		}
+
+		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;
 }
 
 sub split_addrs {
-- 
1.9.1

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

[Matthieu Moy]

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

> ---
>  git-send-email.perl | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index a0cd7ff..a1f6c18 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -477,9 +477,59 @@ foreach my $entry (@bcclist) {
>  sub parse_address_line {
>  	if ($have_mail_address) {
>  		return map { $_->format } Mail::Address->parse($_[0]);
> -	} else {
> -		return split_addrs($_[0]);
>  	}
> +
> +	my $commentrgx=qr/\((?:[^)]*)\)/;
> +	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
> +	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;

Spaces around = please.

The code below is a bit hard to read (I'm neither fluent in Perl nor in
the RFC ...). A few more comments would help. A few examples below (it's
up to you to integrate them or not).

> +	my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;
> +
> +	my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
> +	push @tokens, ",";


        # parse a full address like
        # "Phrase" (comment) <address@example.com>

(to clarify the wording)

> +	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> +	foreach my $token (@tokens) {
> +	    if ($token =~ /^[,;]$/) {

Here and below: you're indenting with a 4-column offset, it should be 8.

> +		if (@address) {
> +		    push @address, @buffer;
> +		} else {
> +		    push @phrase, @buffer;
> +		}
> +
> +		my $str_phrase = join ' ', @phrase;
> +		my $str_address = join '', @address;
> +		my $str_comment = join ' ', @comment;

                # Escape special-characters with backslash
> +		if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
> +		    $str_phrase =~ s/(^|[^\\])"/$1/g;
> +		    $str_phrase = qq["$str_phrase"];
> +		}
> +
> +		if ($str_address ne "" && $str_phrase ne "") {
> +		    $str_address = qq[<$str_address>];
> +		}
> +
> +		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 {

                # We don't know what the token belongs to yet. We'll
                # decide where to append @buffer later.
> +		push @buffer, $token;
> +	    }
> +	}
> +
> +	return @addr_list;

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

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

[Remi Lespinet]

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

> > +	my $commentrgx=qr/\((?:[^)]*)\)/;
> > +	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
> > +	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
> 
> Spaces around = please.
> ...
> > +	foreach my $token (@tokens) {
> > +	    if ($token =~ /^[,;]$/) {
> 
> Here and below: you're indenting with a 4-column offset, it should be 8.

Should have spent more time on the form... Thanks

> The code below is a bit hard to read (I'm neither fluent in Perl nor in
> the RFC ...). A few more comments would help. A few examples below (it's
> up to you to integrate them or not).

Ok, I'll add comments for the hardest parts.

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

[Junio C Hamano]

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

> parse_address_line had not the same behavior whether the user had
> Mail::Address or not. Teach parse_address_line to behave like
> Mail::Address.

Sounds like a fun project ;-)

> +	my $commentrgx=qr/\((?:[^)]*)\)/;
> +	my $quotergx=qr/"(?:[^\"\\]|\\.)*"/;
> +	my $wordrgx=qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
> +	my $tokenrgx = qr/(?:$quotergx|$wordrgx|$commentrgx|\S)/;

Suffix "rgx" that means "regular expression" is a bit unusual, and
also hard to read when squashed to another word.  Elsewhere in the
same script, we seem to use $re_whatever to store precompiled
regular expressions, so perhaps $re_comment, $re_quote, etc.?

> +	my @tokens = map { $_ =~ /\s*($tokenrgx)\s*/g } @_;
> +	push @tokens, ",";
> +
> +	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
> +	foreach my $token (@tokens) {
> +	    if ($token =~ /^[,;]$/) {
> +		if (@address) {
> +		    push @address, @buffer;
> +		} else {
> +		    push @phrase, @buffer;
> +		}
> +
> +		my $str_phrase = join ' ', @phrase;
> +		my $str_address = join '', @address;
> +		my $str_comment = join ' ', @comment;
> +
> +		if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
> +		    $str_phrase =~ s/(^|[^\\])"/$1/g;
> +		    $str_phrase = qq["$str_phrase"];
> +		}
> +
> +		if ($str_address ne "" && $str_phrase ne "") {
> +		    $str_address = qq[<$str_address>];
> +		}

We see both "git@vger.kernel.org" and "<git@vger.kernel.org>" around
here for an address without comment or phrase; this chooses to turn
them both into "<git@vger.kernel.org>" form?  Not a complaint but am
thinking aloud to see if I am reading it correctly.

> +
> +		my $str_mailbox = "$str_phrase $str_address $str_comment";
> +		$str_mailbox =~ s/^\s*|\s*$//g;

So an empty @comment will not leave spaces after $str_address, which
makes sense (likewise for @phrase).

> +		push @addr_list, $str_mailbox if ($str_mailbox);
> +
> +		@phrase = @address = @comment = @buffer = ();
> +	    } elsif ($token =~ /^\(/) {
> +		push @comment, $token;
> +	    } elsif ($token eq "<") {
> +		push @phrase, (splice @address), (splice @buffer);

That is a clever use of splice (My Perl's rusty; you learn new
things every day) ;-)

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

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

[Remi Lespinet]

> Junio C Hamano <gitster@pobox.com> writes
> Suffix "rgx" that means "regular expression" is a bit unusual, and
> also hard to read when squashed to another word.  Elsewhere in the
> same script, we seem to use $re_whatever to store precompiled
> regular expressions, so perhaps $re_comment, $re_quote, etc.?

Yes it's indeed a better name. I had not seen it, thanks!


> > +                if ($str_address ne "" && $str_phrase ne "") {
> > +                    $str_address = qq[<$str_address>];
> > +                }
> 
> We see both "git@vger.kernel.org" and "<git@vger.kernel.org>" around
> here for an address without comment or phrase; this chooses to turn
> them both into "<git@vger.kernel.org>" form?  Not a complaint but am
> thinking aloud to see if I am reading it correctly.

If there's no phrase, this will choose the "git@vger.kernel.org" form,
in both cases, because it'll be recognize as an address, $str_address
will be "git@vger.kernel.org" and $str_phrase will be empty before the
if ($str_address ne "" ...)
Here are some tests:

Input: <jdoe@example.com>
Split: jdoe@example.com
M::A : jdoe@example.com
----------
Input: jdoe@example.com
Split: jdoe@example.com
M::A : jdoe@example.com
----------
Input: Jane <jdoe@example.com>
Split: Jane <jdoe@example.com>
M::A : Jane <jdoe@example.com>
----------
Input: Jane Doe <jdoe@example.com>
Split: Jane Doe <jdoe@example.com>
M::A : Jane Doe <jdoe@example.com>
----------
Input: "Jane" <jdoe@example.com>
Split: "Jane" <jdoe@example.com>
M::A : "Jane" <jdoe@example.com>
----------
Input: "Doe, Jane" <jdoe@example.com>
Split: "Doe, Jane" <jdoe@example.com>
M::A : "Doe, Jane" <jdoe@example.com>

I've some more tests, maybe I should put them all in this post ?

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

[Matthieu Moy]

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

> I've some more tests, maybe I should put them all in this post ?

Yes, please post as much as you have. Ideally, this should be
automatically tested, but if you don't have time to write the automated
tests, at least having a track of what you did on the list archives can
help someone else to do it.

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

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

[Remi Lespinet]

> Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:
> 
> > I've some more tests, maybe I should put them all in this post ?
> 
> Yes, please post as much as you have. Ideally, this should be
> automatically tested, but if you don't have time to write the automated
> tests, at least having a track of what you did on the list archives can
> help someone else to do it.

It may not be easily readable without colors, so there are the scripts
at the end. You can change the tested input by changing lines after
the "cat >.tmplist" line in testall.sh. (There are two scripts 
testall.sh and testone.perl).

Here are the tests results:

Input: 
Split: 
M::A : 
Same : Yes
----------
Input: Jane
Split: Jane
M::A : Jane
Same : Yes
----------
Input: jdoe@example.com
Split: jdoe@example.com
M::A : jdoe@example.com
Same : Yes
----------
Input: <jdoe@example.com>
Split: jdoe@example.com
M::A : jdoe@example.com
Same : Yes
----------
Input: Jane <jdoe@example.com>
Split: Jane <jdoe@example.com>
M::A : Jane <jdoe@example.com>
Same : Yes
----------
Input: Jane Doe <jdoe@example.com>
Split: Jane Doe <jdoe@example.com>
M::A : Jane Doe <jdoe@example.com>
Same : Yes
----------
Input: Jane\ Doe <jdoe@example.com>
Split: "Jane\ Doe" <jdoe@example.com>
M::A : "Jane \ Doe" <jdoe@example.com>
Same : No
----------
Input: "Jane" <jdoe@example.com>
Split: "Jane" <jdoe@example.com>
M::A : "Jane" <jdoe@example.com>
Same : Yes
----------
Input: "Doe, Jane" <jdoe@example.com>
Split: "Doe, Jane" <jdoe@example.com>
M::A : "Doe, Jane" <jdoe@example.com>
Same : Yes
----------
Input: "Doe, Ja"ne <jdoe@example.com>
Split: "Doe, Ja ne" <jdoe@example.com>
M::A : "Doe, Ja" ne <jdoe@example.com>
Same : No
----------
Input: "Doe, Katarina" Jane <jdoe@example.com>
Split: "Doe, Katarina Jane" <jdoe@example.com>
M::A : "Doe, Katarina" Jane <jdoe@example.com>
Same : No
----------
Input: "Jane@:;\>.,()<Doe" <jdoe@example.com>
Split: "Jane@:;\>.,()<Doe" <jdoe@example.com>
M::A : "Jane@:;\>.,()<Doe" <jdoe@example.com>
Same : Yes
----------
Input: Jane@:;\.,()<>Doe <jdoe@example.com>
Split: Jane@:
     : "\."
     : Doe <jdoe@example.com> ()
M::A : Jane@:
     : \.
     : Doe <jdoe@example.com> ()
Same : No
----------
Input: Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
Split: Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
M::A : Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
Same : Yes
----------
Input: "<jdoe@example.com>"
Split: "<jdoe@example.com>"
M::A : "<jdoe@example.com>"
Same : Yes
----------
Input: "Jane jdoe@example.com"
Split: "Jane jdoe@example.com"
M::A : "Jane jdoe@example.com"
Same : Yes
----------
Input: Jane Doe <jdoe    @   example.com  >
Split: Jane Doe <jdoe@example.com>
M::A : Jane Doe <jdoe@example.com>
Same : Yes
----------
Input: Jane       Doe <  jdoe@example.com  >
Split: Jane Doe <jdoe@example.com>
M::A : Jane Doe <jdoe@example.com>
Same : Yes
----------
Input: Jane @ Doe @ Jane @ Doe
Split: Jane@Doe@Jane@Doe
M::A : Jane@Doe@Jane@Doe
Same : Yes
----------
Input: Jane jdoe@example.com
Split: Janejdoe@example.com
M::A : Jane
     : jdoe@example.com
Same : No
----------
Input: <jdoe@example.com> Jane Doe
Split: jdoe@example.comJaneDoe
M::A : Jane Doe <jdoe@example.com>
Same : No
----------
Input: Jane <jdoe@example.com> Doe
Split: Jane <jdoe@example.comDoe>
M::A : Jane Doe <jdoe@example.com>
Same : No
----------
Input: "Jane, 'Doe'" <jdoe@example.com>
Split: "Jane, 'Doe'" <jdoe@example.com>
M::A : "Jane, 'Doe'" <jdoe@example.com>
Same : Yes
----------
Input: 'Doe, "Jane' <jdoe@example.com>
Split: 'Doe
     : " Jane' <jdoe@example.com>
M::A : 'Doe
     : " Jane' <jdoe@example.com>
Same : Yes
----------
Input: "Jane" "Do"e <jdoe@example.com>
Split: "Jane" "Do" e <jdoe@example.com>
M::A : "Jane" "Do" e <jdoe@example.com>
Same : Yes
----------
Input: "Jane' Doe" <jdoe@example.com>
Split: "Jane' Doe" <jdoe@example.com>
M::A : "Jane' Doe" <jdoe@example.com>
Same : Yes
----------
Input: "Jane Doe <jdoe@example.com>" <jdoe@example.com>
Split: "Jane Doe <jdoe@example.com>" <jdoe@example.com>
M::A : "Jane Doe <jdoe@example.com>" <jdoe@example.com>
Same : Yes
----------
Input: "Jane\" Doe" <jdoe@example.com>
Split: "Jane\" Doe" <jdoe@example.com>
M::A : "Jane\" Doe" <jdoe@example.com>
Same : Yes
----------
Input: Doe, jane <jdoe@example.com>
Split: Doe
     : jane <jdoe@example.com>
M::A : Doe
     : jane <jdoe@example.com>
Same : Yes
----------
Input: "Jane Doe <jdoe@example.com>
Split: " Jane Doe <jdoe@example.com>
M::A : " Jane Doe <jdoe@example.com>
Same : Yes
----------
Input: "Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>
Split: "Jane  Kat a ri na ,Doe" <jdoe@example.com>
M::A : "Jane " Kat "a" ri "na" ",Doe" <jdoe@example.com>
Same : No
----------
Input: Jane Doe
Split: Jane Doe
M::A : Jane
     : Doe
Same : No
----------
Input: Jane "Doe <jdoe@example.com>"
Split: "Jane Doe <jdoe@example.com>"
M::A : Jane
     : "Doe <jdoe@example.com>"
Same : No
----------
Input: \"Jane Doe <jdoe@example.com>
Split: "\"Jane Doe" <jdoe@example.com>
M::A : \ " Jane Doe <jdoe@example.com>
Same : No
----------
Input: Jane\"\" Doe <jdoe@example.com>
Split: "Jane\"\" Doe" <jdoe@example.com>
M::A : Jane \ " \ " Doe <jdoe@example.com>
Same : No
----------
Input: 'Jane 'Doe' <jdoe@example.com>
Split: 'Jane 'Doe' <jdoe@example.com>
M::A : 'Jane 'Doe' <jdoe@example.com>
Same : Yes
----------
Input: 'Jane "Katarina\" \' Doe' <jdoe@example.com>
Split: "'Jane  Katarina\" \' Doe'" <jdoe@example.com>
M::A : 'Jane " Katarina \ " \ ' Doe' <jdoe@example.com>
Same : No


**********************************************************************
*                          SCRIPTS PART                              *
**********************************************************************


---------------------------- testall.sh ----------------------------

#!/bin/sh

cat >.tmplist <<EOF

Jane
jdoe@example.com
<jdoe@example.com>
Jane <jdoe@example.com>
Jane Doe <jdoe@example.com>
Jane\ Doe <jdoe@example.com>
"Jane" <jdoe@example.com>
"Doe, Jane" <jdoe@example.com>
"Doe, Ja"ne <jdoe@example.com>
"Doe, Katarina" Jane <jdoe@example.com>
"Jane@:;\>.,()<Doe" <jdoe@example.com>
Jane@:;\.,()<>Doe <jdoe@example.com>
Jane!#$%&'*+-/=?^_{|}~Doe' <jdoe@example.com>
"<jdoe@example.com>"
"Jane jdoe@example.com"
Jane Doe <jdoe    @   example.com  >
Jane       Doe <  jdoe@example.com  >
Jane @ Doe @ Jane @ Doe
Jane jdoe@example.com
<jdoe@example.com> Jane Doe
Jane <jdoe@example.com> Doe
"Jane, 'Doe'" <jdoe@example.com>
'Doe, "Jane' <jdoe@example.com>
"Jane" "Do"e <jdoe@example.com>
"Jane' Doe" <jdoe@example.com>
"Jane Doe <jdoe@example.com>" <jdoe@example.com>
"Jane\" Doe" <jdoe@example.com>
Doe, jane <jdoe@example.com>
"Jane Doe <jdoe@example.com>
"Jane "Kat"a" ri"na" ",Doe" <jdoe@example.com>
Jane Doe
Jane "Doe <jdoe@example.com>"
\"Jane Doe <jdoe@example.com>
Jane\"\" Doe <jdoe@example.com>
'Jane 'Doe' <jdoe@example.com>
'Jane "Katarina\" \' Doe' <jdoe@example.com>
EOF


cat .tmplist | while read -r line
do
    echo "Input: $line"
    ./testone.perl "$line"
    echo ----------
done

---------------------------- testone.perl ----------------------------

#!/usr/bin/perl

use strict;
use warnings;

use Term::ANSIColor;
use Mail::Address;
use Text::ParseWords;

my $string = $ARGV[0];

sub split_addrs {
	my $re_comment = qr/\((?:[^)]*)\)/;
	my $re_quote = qr/"(?:[^\"\\]|\\.)*"/;
	my $re_word = qr/(?:[^]["\s()<>:;@\\,.]|\\.)+/;
	my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/;

	my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_;
	push @tokens, ",";

	my (@addr_list, @phrase, @address, @comment, @buffer) = ();
	foreach my $token (@tokens) {
		if ($token =~ /^[,;]$/) {
			if (@address) {
				push @address, @buffer;
			} else {
				push @phrase, @buffer;
			}
		
			my $str_phrase = join ' ', @phrase;
			my $str_address = join '', @address;
			my $str_comment = join ' ', @comment;
		
			if ($str_phrase =~ /[][()<>:;@\\,.\000-\037\177]/) {
				$str_phrase =~ s/(^|[^\\])"/$1/g;
				$str_phrase = qq["$str_phrase"];
			}
		
			if ($str_address ne "" && $str_phrase ne "") {
				$str_address = qq[<$str_address>];
			}
		
			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;
}

sub old_split {
	quotewords('\s*,\s*', 1, $_[0]);
}

my @tab = split_addrs($string);
my @ref = map { $_->format } Mail::Address->parse($string);
# my @old = old_split($string);  #can be printed to see the difference

my $tabstring = join "\n", @tab;
my $refstring = join "\n", @ref;
my $same = ($tabstring eq $refstring);

$tabstring =~ s/\n/\n     : /g;
$refstring =~ s/\n/\n     : /g;

print color 'bold yellow';
print "Split: ", "$tabstring", "\n";

print color 'bold blue';
print "M::A : ", "$refstring", "\n";

if ($same) {
	print color 'bold green';
	print "Same : ", "Yes", "\n";
} else {
	print color 'bold red';
	print "Same : ", "No", "\n";
}

print color 'reset';


 

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

[Matthieu Moy]

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

>> Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:
>> 
>> > I've some more tests, maybe I should put them all in this post ?
>> 
>> Yes, please post as much as you have. Ideally, this should be
>> automatically tested, but if you don't have time to write the automated
>> tests, at least having a track of what you did on the list archives can
>> help someone else to do it.
>
> It may not be easily readable without colors, so there are the scripts
> at the end.

Cool. Then almost all the work is done to get an automated test. Next
step would be to add the tests itself in the code. I would do that by
adding a hidden --selfcheck option to git send-email that would compare
Mail::Address->parse($string); and split_addrs($string); for all your
testcases, and die if they do not match. Then calling it from the
testsuite would be trivial.

I can do that on top of your series if you don't have time.

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

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

[Remi Lespinet]

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

> Cool. Then almost all the work is done to get an automated test. Next
> step would be to add the tests itself in the code. I would do that by
> adding a hidden --selfcheck option to git send-email that would compare
> Mail::Address->parse($string); and split_addrs($string); for all your
> testcases, and die if they do not match. Then calling it from the
> testsuite would be trivial.

Ok, are there such "--selfcheck" options elsewhere? If I understand it
right, you want to put the tests inside the git-send-email script. I
don't feel really good about that but I guess it's hard to test it
otherwise...  Also what will we do with the failing tests?  Just
discard them?  I think there's two sort of failing test:

 - When output provided by parse_address_ without Mail::Address
   is better or has no impact at all on the code. Such as:

    Input: "Doe, Ja"ne <jdoe@example.com>
    Split: "Doe, Ja ne" <jdoe@example.com>
    M::A : "Doe, Ja" ne <jdoe@example.com>

   This output is done on purpose. If it was the same output with
   Mail::Address, we could have avoided commit 8/9 of this serie btw.

   I think we should also test these cases.

 - When we don't really care about the output, because the user entry
   is wrong, and we just expect the script to be aborted somehow... We
   don't need to test that.

We could also add an option to specify whether we want to use
Mail::Address or not and do the tests in t9001* (but this would
take much more time).

> I can do that on top of your series if you don't have time.

Time will become a problem soon, but I think I can handle it unless
you really want to do it !

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

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Cool. Then almost all the work is done to get an automated test. Next
>> step would be to add the tests itself in the code. I would do that by
>> adding a hidden --selfcheck option to git send-email that would compare
>> Mail::Address->parse($string); and split_addrs($string); for all your
>> testcases, and die if they do not match. Then calling it from the
>> testsuite would be trivial.
>
> Ok, are there such "--selfcheck" options elsewhere?

Not as far as I know.

> If I understand it right, you want to put the tests inside the
> git-send-email script. I don't feel really good about that but I guess
> it's hard to test it otherwise...

Hmm, actually there is, I didn't look at the right places yesterday.
git-send-email.perl already does 'use Git;', and there's already a set
of unit-tests for Git.pm: t9700-perl-git.sh, which calls perl
"$TEST_DIRECTORY"/t9700/test.pl.

So, you can just add your code as a function in Git.pm and unit-tests in
t/t9700/test.pl.

> Also what will we do with the failing tests? Just discard them? I
> think there's two sort of failing test:
>
>  - When output provided by parse_address_ without Mail::Address
>    is better or has no impact at all on the code. Such as:

I'm not sure we can be "better" as long as we do use Mail::Address when
available. Any difference is potentially harmfull for the user because
it means that Git will have different behavior on different machines.

Perhaps this is an argument to use your version unconditionally and drop
Mail::Address actually.

But you can still test that with

  is(parse_address_(...), "Doe, Jane", "<description>");

(possibly not calling Mail::Address)

http://search.cpan.org/~exodist/Test-Simple-1.001014/lib/Test/More.pm

The cases where Mail::Address and your version give the same result can
be tested with a foreach loop calling

  is(parse_address_(...), Mail::Address(...), ...);

>  - When we don't really care about the output, because the user entry
>    is wrong, and we just expect the script to be aborted somehow... We
>    don't need to test that.

... but if you already have the tests, you can keep them as known
failure.

See the "TODO: BLOCK" section of the doc of Test::More.

>> I can do that on top of your series if you don't have time.
>
> Time will become a problem soon, but I think I can handle it unless
> you really want to do it !

If you have time, just do it.

Thanks,

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

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

[Remi Lespinet]

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

I don't know if it's an argument for this change, but rfc2822 says:

   Semantically, neither the optional CFWS outside of the quote
   characters nor the quote characters themselves are part of the
   quoted-string...

 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 a1f6c18..8594ab9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1078,15 +1078,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"];
 	}
 
-- 
1.9.1

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

[Remi Lespinet]

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.

Helped-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
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>
---
 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 b48a764..afd9569 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 8594ab9..265299e 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]);
@@ -1101,7 +1087,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 714fcae..3c5b853 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1675,4 +1675,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
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 265299e..9b28dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -836,6 +836,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 3c5b853..8e21fb0 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1719,4 +1719,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
-- 
1.9.1

Re: [PATCH/RFC v4 10/10] send-email: suppress meaningless whitespaces in from field

[Matthieu Moy]

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

>  if (defined $sender) {
> +	$sender =~ s/^\s+|\s$//g;

I would say \s+ also for the second \s. Not really different, but it
feels wrong to iterate the substitution as many times as there are
trailing spaces to remove.

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

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

[Remi Lespinet]

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

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

> > if (defined $sender) { 
> > + $sender =~ s/^\s+|\s$//g; 

> I would say \s+ also for the second \s. Not really different, but it 
> feels wrong to iterate the substitution as many times as there are 
> trailing spaces to remove. 

Oops should have read it one more time... Thanks.

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

[Remi Lespinet]

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>
---
 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 a3663da..eef12e6 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 \
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 6bedf74..8bf38ee 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1560,7 +1560,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 eef12e6..f7d4132 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1579,6 +1579,66 @@ test_expect_success $PREREQ 'sendemail.aliasfiletype=sendmail' '
 	grep "^!o@example\.com!$" commandline1
 '
 
+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 &&
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 f7d4132..714fcae 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"}
 }
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 8bf38ee..2d5c530 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -833,12 +833,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(
@@ -1051,6 +1048,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
@@ -1560,10 +1564,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);
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 2d5c530..f61449d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -555,8 +555,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 {
@@ -801,6 +799,8 @@ if (!$force) {
 	}
 }
 
+($sender) = expand_aliases($sender) if defined $sender;
+
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
 }
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 f61449d..a0cd7ff 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,9 +799,9 @@ if (!$force) {
 	}
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+	($sender) = expand_aliases($sender);
+} else {
 	$sender = $repoauthor || $repocommitter || '';
 }
 
-- 
1.9.1

[PATCH v5 07/10] send-email: reduce dependancies impact on parse_address_line

[Remi Lespinet]

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

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

[Matthieu Moy]

This is the last message I received in the series, and it's labeled 07/10. Is that normal?

> parse_address_line had not the same behavior whether the user had

had not -> did not have

> 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.

Sounds good to me.

> 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.

It seems it's how Test::More works. I'd keep it like this, but I have no real experience with Test::More.

> 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).

You can get the 1..44 at the end with

printf "1..%d\n", Test::More->builder->current_test;

This is what t9700/test.pl does.

> 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.

Imperative tone => Return, not Returns.

I would have put parse_mailbox near ident_person because both functions are somehow about email.

> +BEGIN { use_ok('Git') }
> +BEGIN { use_ok('Mail::Address') }

This will fail if Mail::Address is not available. It would be better to declare Mail::Address as a prerequisite in t9000-address.sh (like what you're already doing for Test::More).

Good job, modulo these minor details, the series looks good to me.

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

[PATCH v5 07/10] send-email: reduce dependancies impact on parse_address_line

[Remi Lespinet]

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

> This is the last message I received in the series, and it's labeled
> 07/10. Is that normal?

No, it wasn't, I have seen no error message though... I'll take a look
at that later.  I just sent 0008, 0009 and 0010 but I seems that I've pasted
the wrong line in the in-reply-to... Maybe I need more sleep.

>> 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).
>
> You can get the 1..44 at the end with
 ...
> I would have put parse_mailbox near ident_person because both
> functions are somehow about email.
>
>> +BEGIN { use_ok('Git') }
>> +BEGIN { use_ok('Mail::Address') }
>
> This will fail if Mail::Address is not available. It would be better
> to declare Mail::Address as a prerequisite in t9000-address.sh (like
> what you're already doing for Test::More).

Ok, will do.

Thanks.

[PATCH v5 07/10] send-email: reduce dependancies impact on parse_address_line

[Remi Lespinet]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> > 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).
> 
> You can get the 1..44 at the end with
> 
> printf "1..%d\n", Test::More->builder->current_test;
> 
> This is what t9700/test.pl does.

I can also get it by removing the line 

 Test::More->builder->no_ending(1);

and replacing

 use Test::More;

by

 use Test::More "no_plan";

I think I'm going to do that, because the no_ending thing makes the
test suite success even if every test fails: at the end we have

# test_external test Perl address parsing function was ok
# test_external_without_stderr test no stderr: Perl address parsing function was ok

in case everything is ok. With the "no_ending" line, only the second
line reports failures, the first is always the same.
I think both must be marked red.

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

[Matthieu Moy]

> Subject: Re: [PATCH v5 07/10] send-email: reduce dependancies impact on parse_address_line

s/dependancies/dependencies/

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

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

[Remi Lespinet]

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>
---
 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 bced78e..a03392c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1028,15 +1028,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"];
 	}
 
-- 
1.9.1

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

[Remi Lespinet]

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.

Helped-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
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>
---
 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 b48a764..afd9569 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 a03392c..8bf6656 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]);
@@ -1051,7 +1037,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 714fcae..3c5b853 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1675,4 +1675,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
-- 
1.9.1

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

[Matthieu Moy]

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

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

???

At this point, it's realonable to consider that you're the main author
of the patch, but you could add a

Original-patch-by:

to credit the initial author.

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

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

[Remi Lespinet]

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>
---
 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 8bf6656..749d809 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -786,6 +786,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 3c5b853..8e21fb0 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1719,4 +1719,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
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 a3663da..eef12e6 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 \
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 6bedf74..8bf38ee 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1560,7 +1560,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 eef12e6..f7d4132 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1579,6 +1579,66 @@ test_expect_success $PREREQ 'sendemail.aliasfiletype=sendmail' '
 	grep "^!o@example\.com!$" commandline1
 '
 
+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 &&
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 f7d4132..714fcae 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"}
 }
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 8bf38ee..2d5c530 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -833,12 +833,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(
@@ -1051,6 +1048,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
@@ -1560,10 +1564,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);
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 2d5c530..f61449d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -555,8 +555,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 {
@@ -801,6 +799,8 @@ if (!$force) {
 	}
 }
 
+($sender) = expand_aliases($sender) if defined $sender;
+
 if (!defined $sender) {
 	$sender = $repoauthor || $repocommitter || '';
 }
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 f61449d..a0cd7ff 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -799,9 +799,9 @@ if (!$force) {
 	}
 }
 
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+	($sender) = expand_aliases($sender);
+} else {
 	$sender = $repoauthor || $repocommitter || '';
 }
 
-- 
1.9.1

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

[Remi Lespinet]

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>
---
 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 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..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);
-- 
1.9.1

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

[Matthieu Moy]

Your git send-email does not seem to like PATCHes 08-10/10 ;-).

Up to PATCH 07, the series looks good.

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

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

[Remi LESPINET]

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

> Your git send-email does not seem to like PATCHes 08-10/10 ;-).
>
> Up to PATCH 07, the series looks good.

Yes, I get "Too many recipients" error... If I specify
--no-signoff-by-cc then this is also aborted but I get no error (at
least I've not seen it last time...). If I rerun git send-email with
only one patch, it works (even if there is no difference with the
number of recipient a priori). I'll investigate asap, not
sure it's a bug, maybe It's just me !

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

[Remi Lespinet]

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>
---
 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 bced78e..a03392c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1028,15 +1028,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"];
 	}
 
-- 
1.9.1

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

[Remi Lespinet]

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.

Helped-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
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>
---
 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 b48a764..afd9569 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 a03392c..8bf6656 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]);
@@ -1051,7 +1037,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 714fcae..3c5b853 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1675,4 +1675,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
-- 
1.9.1

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

[Matthieu Moy]

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

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

This is funny in a patch sent by the same Remi Lespinet ;-).

Anyway, the whole series looks good to me now (I finally got all up to
10/10).

Cheers,

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

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

[Remi Lespinet]

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>
---
 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 8bf6656..749d809 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -786,6 +786,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 3c5b853..8e21fb0 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1719,4 +1719,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
-- 
1.9.1