[PATCH/RFC v2 5/5] send-email: refactor address list process

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes

> Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
> 
> > Remi Lespinet <remi.lespinet@xxxxxxxxxxxxxxxxxxxxxxx> writes:
> >
> >> Simplify code by creating a function to transform list of email lists
> >> (comma separated, with aliases ...)  into a simple list of valid email
> >> addresses.
> >
> > I would have found the series easier to read if this refactoring came
> > earlier (and then PATCH 2/5 would fix the bug as a positive side effect
> > of the refactoring).
> 
> I agree that doing 5/5 sooner would make 4/5 a lot clearer.  
> 
> Introducing the helper of 5/5 before 2/5 happens, and then replacing
> two calls to validate-address-list with process-address-list would
> hide the nature of the change, i.e. fixing a bug, so it is better to
> see it done before the refactoring of 5/5, provided if it is indeed
> a bug that these were not expanded.

Ok thanks, I submit it again soon. Also I think we should swap the
lines 'sanitize_address_list(...)' and 'expand_aliases(...)',
i.e. first sanitize addresses and then expand aliases.

We could then remove leading and trailing whitespaces in the
sanitize_address_list function as aliases file formats supported by git
send-email doesn't take these whitespace into account anyway:

Example which currently can't work:

git send-email --to=" alias" ...

Moreover I think it's more natural to do that so.

I'll do it right after the refactoring patch introducing
process_address_list or maybe I should avoid changing this patch now ?
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]