[PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Jorge-Juan.Garcia-Garcia]

From: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>

Make it so that we can use a list of email in flags
instead of having to use one flag per email address.

The format of email list handled is pretty basic for now:
	$ git send-email --to='Foo <foo@example.com>, bar@example.com'
We thought it would be nice to have a "first-step" version which works
before handling more complex ones such as:
	$ git send-email --to='Foo, Bar <foobar@example.com>'

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

Changes in the patch:
 -Update documentation
 -Removal of no-longer needed user input verification
 -New function that splits email list into seperate email addresses
 -New test to make sure it behaves the way intended

 Documentation/git-send-email.txt |   21 +++++++++++++++------
 git-send-email.perl              |   38 ++++++++++++++++++++++++--------------
 t/t9001-send-email.sh            |   37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 75 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 40a9a9a..e3444cf 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -50,16 +50,22 @@ Composing
 	'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.
+	The format supported for email list is the following:
+	"Foo <foo@example.com>, bar@example.com".
+	Please notice that the email list does not handle commas in
+	email names such as "Foo, Bar <foobar@example.com>".
 
 --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.
+	The format supported for email list is the following:
+	"Foo <foo@example.com>, bar@example.com".
+	Please notice that the email list does not handle commas in
+	email names such as "Foo, Bar <foobar@example.com>".
 
 --compose::
 	Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -111,12 +117,15 @@ is not set, this will be prompted for.
 	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.
+	The format supported for email list is the following:
+	"Foo <foo@example.com>, bar@example.com".
+	Please notice that the email list does not handle commas in
+	email names such as "Foo, Bar <foobar@example.com>".
 
 --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 671762b..d7e4887 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -426,20 +426,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]);
@@ -1079,6 +1065,27 @@ sub smtp_auth_maybe {
 	return $auth;
 }
 
+sub split_email_list {
+    my(@list) = @_;
+    my @tmp;
+    my @emails;
+	for (my $i = 0; $i <= $#list; $i++) {
+	    if ($list[$i] =~ /,/) {
+		@emails = split(/,/, $list[$i]);
+	    } else {
+		@emails = $list[$i];
+	    }
+	    # Removal of unwanted spaces
+	    for (my $j = 0; $j <= $#emails; $j++) {
+		$emails[$j] =~ s/^\s+//;
+		$emails[$j] =~ s/\s+$//;
+	    }
+	    @tmp = (@tmp, @emails);
+	}
+    return(@tmp);
+}
+
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1089,6 +1096,9 @@ sub send_message {
 		      not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
 		    }
 	       @cc);
+	@cc = split_email_list(@cc);
+	@bcclist = split_email_list(@bcclist);
+	@recipients = split_email_list(@recipients);
 	my $to = join (",\n\t", @recipients);
 	@recipients = unique_email_list(@recipients,@cc,@bcclist);
 	@recipients = (map { extract_valid_address_or_die($_) } @recipients);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9f46f22..87641bc 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1349,4 +1349,39 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
 	grep "^!someone@example\.org!$" commandline1
 '
 
-test_done
+test_expect_success $PREREQ 'setup expected-list' '
+	git send-email \
+		--dry-run \
+		--suppress-cc=sob \
+		--from="Example <from@example.com>" \
+		--to="to1@example.com" --to="to2@example.com" \
+		--to="to3@example.com" --cc="cc0@example.com" \
+		--cc="Cc 1 <cc1@example.com>" --cc="Cc 2 <cc2@example.com>" \
+		--bcc="bcc1@example.com" --bcc="bcc2@example.com" \
+		-1 >output
+	sed	-e "s/^\(\/tmp\/\).*/\1patch/" \
+		-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		<output >expected-list
+'
+
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '
+	git send-email \
+		--dry-run \
+		--suppress-cc=sob \
+		--from="Example <from@example.com>" \
+		--to="to1@example.com, to2@example.com,to3@example.com" \
+		--cc="cc0@example.com" \
+		--cc="Cc 1 <cc1@example.com>, Cc 2 <cc2@example.com>" \
+		--bcc="bcc1@example.com, bcc2@example.com" \
+		-1 >output
+	sed	-e "s/^\(\/tmp\/\).*/\1patch/" \
+		-e "s/^\(Date:\).*/\1 DATE-STRING/" \
+		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
+		<output >actual-list &&
+	test_cmp expected-list actual-list
+'
+
+test_done
\ No newline at end of file
-- 
1.7.8

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Ramkumar Ramachandra]

Jorge-Juan.Garcia-Garcia@ensimag.imag.fr wrote:
> The format of email list handled is pretty basic for now:
>         $ git send-email --to='Foo <foo@example.com>, bar@example.com'
> We thought it would be nice to have a "first-step" version which works
> before handling more complex ones such as:
>         $ git send-email --to='Foo, Bar <foobar@example.com>'

Is this a regression?  I can't send emails to a recipient whose name
contains a comma?

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Mathieu Liénard--Mayor]

Le 2013-06-18 12:12, Ramkumar Ramachandra a écrit :
> Jorge-Juan.Garcia-Garcia@ensimag.imag.fr wrote:
>> The format of email list handled is pretty basic for now:
>>         $ git send-email --to='Foo <foo@example.com>, 
>> bar@example.com'
>> We thought it would be nice to have a "first-step" version which 
>> works
>> before handling more complex ones such as:
>>         $ git send-email --to='Foo, Bar <foobar@example.com>'
>
> Is this a regression?  I can't send emails to a recipient whose name
> contains a comma?
It is not. Previously the input would be considered incorrect:

-# Verify the user input
-
-foreach my $entry (@initial_to) {
-	die "Comma in --to entry: $entry'\n" unless $entry !~ m/,/;
-}
-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Ramkumar Ramachandra]

Mathieu Liénard--Mayor wrote:
>> Is this a regression?  I can't send emails to a recipient whose name
>> contains a comma?
>
> It is not. Previously the input would be considered incorrect:

Right.  It dies with

  Comma in --to entry: ...

This artificial limitation is imposed by 79ee555b (Check and document
the options to prevent mistakes, 2006-06-21).

Perhaps include this information in the commit message?

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[benoît person]

> +sub split_email_list {
> +    my(@list) = @_;
> +    my @tmp;
> +    my @emails;
> +       for (my $i = 0; $i <= $#list; $i++) {
> +           if ($list[$i] =~ /,/) {
> +               @emails = split(/,/, $list[$i]);
> +           } else {
> +               @emails = $list[$i];
> +           }
> +           # Removal of unwanted spaces
> +           for (my $j = 0; $j <= $#emails; $j++) {
> +               $emails[$j] =~ s/^\s+//;
> +               $emails[$j] =~ s/\s+$//;
> +           }
> +           @tmp = (@tmp, @emails);
> +       }
> +    return(@tmp);
> +}
Why two regex ? You could do something like :
$emails[$j] =~ s/^\s+|\s+$//g;
to remove leading and trailing whitespaces at the same time.

I think it's better to use the builin 'push' function to concatenate
your two arrays:
push(@tmp, @emails);

Benoit Person

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Ramkumar Ramachandra]

Jorge-Juan.Garcia-Garcia@ensimag.imag.fr wrote:
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 9f46f22..87641bc 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1349,4 +1349,39 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '
>         grep "^!someone@example\.org!$" commandline1
>  '
>
> -test_done
> +test_expect_success $PREREQ 'setup expected-list' '
> [...]
> +test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '

What is the meaning of this test?  It looks like you've run git
send-email twice in exactly the same way, and compared their outputs
(after smudging the unstable headers).

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Mathieu Liénard--Mayor]

Le 2013-06-18 12:47, Ramkumar Ramachandra a écrit :
> Jorge-Juan.Garcia-Garcia@ensimag.imag.fr wrote:
>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
>> index 9f46f22..87641bc 100755
>> --- a/t/t9001-send-email.sh
>> +++ b/t/t9001-send-email.sh
>> @@ -1349,4 +1349,39 @@ test_expect_success $PREREQ 
>> 'sendemail.aliasfile=~/.mailrc' '
>>         grep "^!someone@example\.org!$" commandline1
>>  '
>>
>> -test_done
>> +test_expect_success $PREREQ 'setup expected-list' '
>> [...]
>> +test_expect_success $PREREQ 'use email list in --cc --to and --bcc' 
>> '
>
> What is the meaning of this test?  It looks like you've run git
> send-email twice in exactly the same way, and compared their outputs
> (after smudging the unstable headers).
The first one uses one flag per email address, just like we had to do 
so far.
The second one uses one email-list per flag, which is the new feature 
we're introducing.
Then we compare the output of the two, and expect it to be exactly the 
same.

Shouldn't

$ git send-email --cc 'foo@example.com' --cc 'bar@example.com'

and

$ git send-email --cc 'foo@example.com, bar@example.com'

have the exact same effect ?
-- 
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Ramkumar Ramachandra]

Jorge-Juan.Garcia-Garcia@ensimag.imag.fr wrote:
> +sub split_email_list {
> +    my(@list) = @_;
> +    my @tmp;
> +    my @emails;
> +       for (my $i = 0; $i <= $#list; $i++) {
> +           if ($list[$i] =~ /,/) {
> +               @emails = split(/,/, $list[$i]);
> +           } else {
> +               @emails = $list[$i];
> +           }

Perhaps use map like in sanitize_address_list and
validate_address_list to prettify this?

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Ramkumar Ramachandra]

Mathieu Liénard--Mayor wrote:
> Shouldn't
>
> $ git send-email --cc 'foo@example.com' --cc 'bar@example.com'
>
> and
>
> $ git send-email --cc 'foo@example.com, bar@example.com'
>
> have the exact same effect ?

Ah.  Perhaps it would be clearer to check the headers directly like in
the other tests?

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Mathieu Liénard--Mayor wrote:
>> Shouldn't
>>
>> $ git send-email --cc 'foo@example.com' --cc 'bar@example.com'
>>
>> and
>>
>> $ git send-email --cc 'foo@example.com, bar@example.com'
>>
>> have the exact same effect ?
>
> Ah.  Perhaps it would be clearer to check the headers directly like in
> the other tests?

Actually, I find it more elegant like this: it doesn't rely on the
particular layout of headers, so the tests would still pass if something
else is changed in the headers.

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

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Junio C Hamano]

Jorge-Juan.Garcia-Garcia@ensimag.imag.fr writes:

> From: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@ensimag.imag.fr>
>
> Make it so that we can use a list of email in flags
> instead of having to use one flag per email address.
>
> The format of email list handled is pretty basic for now:
> 	$ git send-email --to='Foo <foo@example.com>, bar@example.com'
> We thought it would be nice to have a "first-step" version which works
> before handling more complex ones such as:
> 	$ git send-email --to='Foo, Bar <foobar@example.com>'

Doesn't

	git send-email --to='Foo <foo@example.com>' --to='bar@example.com'

work?  If it does, I do not see much point of this change.  If you
are starting from two pieces of information, why combine it into
one, only have the program to split it again, risking to be bitten
by bugs, and changing the code to do so, risking to add new bugs?

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Matthieu Moy]

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

> Doesn't
>
> 	git send-email --to='Foo <foo@example.com>' --to='bar@example.com'
>
> work?  If it does, I do not see much point of this change.  If you
> are starting from two pieces of information, why combine it into
> one, only have the program to split it again, risking to be bitten
> by bugs, and changing the code to do so, risking to add new bugs?

The obvious use-case is to copy-paste a list of addresses from an email.
Currently, the Cc: list of the email I'm sending looks like

Cc: Jorge-Juan.Garcia-Garcia@ensimag.imag.fr,  git@vger.kernel.org,  Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@ensimag.imag.fr>

If I were to use "git send-email" on it, I'd have to cut the list
myself.

This could be mentionned in the commit message.

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

Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

[Junio C Hamano]

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

> The obvious use-case is to copy-paste a list of addresses from an email.
> ...
> This could be mentionned in the commit message.

OK.