On Thu, May 28, 2015 at 6:42 AM, Remi Lespinet
<remi.lespinet@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> Add the possibility to use a list of emails separated by commas
> in flags --cc --to and --bcc instead of having to use one flag
s/--cc --to/--cc, --to/
Ditto in subject.
> per email address.
>
> The use-case is to copy-paste a list of addresses from an email.
> This change makes it so that we no longer need to cut the list.
>
> The format of email list handled is basic for now:
> $ git send-email --to='Foo <foo@xxxxxxxxxxx>, bar@xxxxxxxxxxx'
> We thought it would be nice to have a "first-step" version which works
> before handling more complex ones such as names with commas:
> $ git send-email --to='Foo, Bar <foobar@xxxxxxxxxxx>'
>
> This artificial limitation is imposed by 79ee555b (Check and document
> the options to prevent mistakes, 2006-06-21).
>
> Signed-off-by: Mathieu Lienard--Mayor <Mathieu.Lienard--Mayor@xxxxxxxxxxxxxxx>
> Signed-off-by: Jorge Juan Garcia Garcia <Jorge-Juan.Garcia-Garcia@xxxxxxxxxxxxxxx>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>
> Contributions-by: Remi Lespinet <remi.lespinet@xxxxxxxxxxxxxxxxxxxxxxx>
Helped-by: may be more appropriate than Contributions-by: on this
project. Also, move it above the sign-offs.
> ---
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 043f345..0aeddcb 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -49,17 +49,21 @@ Composing
> of 'sendemail.annotate'. See the CONFIGURATION section for
> 'sendemail.multiEdit'.
>
> ---bcc=<address>::
> +--bcc="[<address>,...]"::
Adding square brackets indicates that the addresses following '=' are
optional, which is incorrect. It should be sufficient merely to add
the comma and ellipsis. Also, the need to quote strings containing
whitespace is a shell issue not specifically related to this option.
That is, people have to understand quoting issues in general, so it
doesn't make sense to mention them literally here. Thus:
--bcc=<address>,...::
should be sufficient.
> 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.
I think it's harmful to remove this line. Although the "must" is no
longer true following this change, it's still important for people to
know that they can use --bcc multiple times. So, perhaps just reword
it:
This option may be specified multiple times.
> + The format supported for email list is the following:
> + "Foo <foo@xxxxxxxxxxx>, bar@xxxxxxxxxxx".
> + Please notice that the email list does not handle commas in
> + email names such as "Foo, Bar <foobar@xxxxxxxxxxx>".
A few issues:
In order for this to format correctly in Asciidoc, the text needs to
be left-justified (just as was the line which you removed).
The sentence "The format supported..." seems superfluous. It's merely
repeating what is already clearly indicated by "--bcc=<address>,...",
thus it could easily be dropped without harm.
Mention that commas in names are not currently handled is indeed a
good idea, however, "please" tends to be an overused and wasted word
in documentation. More tersely:
Note: Addresses containing commas ("Foo, Bar" <...>)
are not currently supported.
The above comments also apply to --cc and --to.
> diff --git a/git-send-email.perl b/git-send-email.perl
> index ffea500..409ff45 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1052,7 +1038,8 @@ sub sanitize_address {
> }
>
> sub sanitize_address_list {
> - return (map { sanitize_address($_) } @_);
> + my @addr_list = split_address_list(@_);
> + return (map { sanitize_address($_) } @addr_list);
> }
Although, it was convenient from an implementation perspective to plop
the split_address_list() invocation into sanitize_address_list(), it
pollutes sanitize_address_list() by making it do something unrelated
to its purpose.
If you examine places where sanitize_address_list() is called, you will see:
validate_address_list(sanitize_address_list(@xx))
which clearly shows that sanitation and validation are distinct
operations (and each function does only what its name implies). To
continue this idea, the splitting of addresses should be performed
distinctly from the other operations, in this order:
split -> sanitize -> validate
or:
validate_address_list(sanitize_address_list(
split_address_list(@xx))
That's pretty verbose, so introducing a new function to encapsulates
that behavior might be reasonable.
> # Returns the local Fully Qualified Domain Name (FQDN) if available.
> @@ -1193,6 +1180,10 @@ sub file_name_is_absolute {
> return File::Spec::Functions::file_name_is_absolute($path);
> }
>
> +sub split_address_list {
> + return (map { split /\s*,\s*/, $_ } @_);
> +}
This is somewhat misnamed. It's not splitting the address list in the
sense that sanitize_address_list() and validate_address_list() operate
on the list of addresses. The name implies that it's somehow
partitioning the list of addresses, but in fact it's iterating over
the incoming list and (possibly) splitting each item into multiple
pieces. Perhaps a better name would be split_address_list_items() or
something.
> # Returns 1 if the message was sent, and 0 otherwise.
> # In actuality, the whole program dies when there
> # is an error sending a message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html