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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2015, 16:52 UTC
Message-ID: <xmqqmw0j74mg.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqiob78nij.fsf@gitster.dls.corp.google.com>

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

Show 20 quoted lines

> Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> 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 <jdoe@example.com>, mike@example.com'
>>
>> However format using commas in names doesn't work:
>>
>>     $ git send-email --to='"Jane, Doe" <jdoe@example.com>'
>
> 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.

Having thought about this topic (not how the example should be spelled in the log message ;-) a bit more, I do not think the implementation of split_address_list_items in this patch is acceptable.

The reason why we have the "verify the input" thing, allow users to supply multiple --to/--cc/etc., and do not try to split the addresses ourselves is because we want to avoid mistakenly splitting a single address like the above into two and producing syntactically incorrect addresses. People have relied on the current behaviour for a long time, without manually dropping comma when they send their patches with --to='"Jane, Doe" <jdoe@example.com>'.

Until we can reliably split the address list, accepting this patch will introduce a regression.

Note that I do agree with the goal of this series and appreciate the effort. I am only rejecting the current implementation of split_address_list_items().

Thanks.