Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:05:05

Remi Lespinet [off-list ref] writes:
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 [off-list ref], mike@example.com'

However format using commas in names doesn't work:

    $ git send-email --to='"Jane, Doe" [off-list ref]'
That looks as if you are doing "Remi, Lespinet", which is not a good
example.  I think you want "Doe, Jane", the use of comma is when a
name is spelled in the "LastName, FirstName" order.
quoted hunk
diff --git a/git-send-email.perl b/git-send-email.perl
index ffea500..389f19c 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/,/;
-}
So at this point, each element in @initial_to and friends is one
item we got from the user, e.g. "--to=<arg>", where <arg> may want
to include multiple addresses, e.g. "me, you".

And then later, in the more interesting part of the patch, I find a
puzzling code.
 @initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
+@initial_to = extract_address_list(@initial_to);
A question: what is in @initial_to at this point in the code,
immediately before you call expand_aliases() on it?  Isn't one of
the elements be "me, you" when I said --to="me, you"?

Shouldn't you be splitting that into "me" and "you" and expand these
two as potential aliases separately?

In other words, I wonder why the patch needs to be any more complex
than this instead:

	+@initial_to = split_at_comma(@initial_to);
	 @initial_to = expand_aliases(@initial_to);
	 @initial_to = validate_address_list(sanitize_address_list(@initial_to));

As your goal is to treat --to="<address 1>, <address 2>" as if the
user said --to="<address 1>" --to="<address 2>", that would be a
more straight-forward way to enhance the system, and you shouldn't
have to change anything else, no?

 @initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
+@initial_cc = extract_address_list(@initial_cc);
 @bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@bcclist = extract_address_list(@bcclist);
Ditto.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help