On Thu, May 28, 2015 at 11:26 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Thu, May 28, 2015 at 6:42 AM, Remi Lespinet
> <remi.lespinet@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
>> + 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).
s/left-justified/unindented/
> 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
s/Mention/Mentioning/
> 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.
> [...]
>> 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
Oops: s/sanitation/sanitization/
> 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.
--
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