Re: [PATCH/RFC v2 5/5] send-email: refactor address list process
Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr 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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Matthieu Moy matthieu@grenoble-inp.fr writes: To me, the series is ready now, and I don't think re-rolling it would be a good time investment. Plus, I spent time reviewing this series and with my proposal I'd need to review a relatively different one. Ok, thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Junio C Hamano gits...@pobox.com writes Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 5/5] send-email: refactor address list process
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr 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 think it's too late to change this, though. I'm not sure about the name of the function... process_address_list() sounds good to me. The whole series looks good to me now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr 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 think it's too late to change this, though. Why is it to late? I can still change it if necessary. I'm not sure about the name of the function... process_address_list() sounds good to me. Ok nice. :) Thanks -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 5/5] send-email: refactor address list process
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr 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 think it's too late to change this, though. Why is it to late? I can still change it if necessary. To me, the series is ready now, and I don't think re-rolling it would be a good time investment. Plus, I spent time reviewing this series and with my proposal I'd need to review a relatively different one. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Simplify code by creating a function to transform list of email lists (comma separated, with aliases ...) into a simple list of valid email addresses. Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr --- I'm not sure about the name of the function... git-send-email.perl | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 4bc489d..ea03308 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -794,15 +794,9 @@ sub expand_one_alias { return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias; } -@initial_to = split_at_commas(@initial_to); -@initial_to = expand_aliases(@initial_to); -@initial_to = validate_address_list(sanitize_address_list(@initial_to)); +@initial_to = process_address_list(@initial_to); -@initial_cc = split_at_commas(@initial_cc); -@initial_cc = expand_aliases(@initial_cc); -@initial_cc = validate_address_list(sanitize_address_list(@initial_cc)); +@initial_cc = process_address_list(@initial_cc); -@bcclist = split_at_commas(@bcclist); -@bcclist = expand_aliases(@bcclist); -@bcclist = validate_address_list(sanitize_address_list(@bcclist)); +@bcclist = process_address_list(@bcclist); if ($thread !defined $initial_reply_to $prompting) { $initial_reply_to = ask( @@ -1019,6 +1013,14 @@ sub split_at_commas { return (map { split /\s*,\s*/, $_ } @_); } +sub process_address_list { +my @addr_list = split_at_commas(@_); +@addr_list = expand_aliases(@addr_list); +@addr_list = sanitize_address_list(@addr_list); +@addr_list = validate_address_list(@addr_list); +return @addr_list; +} + # Returns the local Fully Qualified Domain Name (FQDN) if available. # # Tightly configured MTAa require that a caller sends a real DNS @@ -1528,12 +1530,8 @@ foreach my $t (@files) { ($confirm =~ /^(?:auto|compose)$/ $compose $message_num == 1)); $needs_confirm = inform if ($needs_confirm $confirm_unconfigured @cc); - @to = split_at_commas(@to); - @to = expand_aliases(@to); - @to = validate_address_list(sanitize_address_list(@to)); + @to = process_address_list(@to); - @cc = split_at_commas(@cc); - @cc = expand_aliases(@cc); - @cc = validate_address_list(sanitize_address_list(@cc)); + @cc = process_address_list(@cc); @to = (@initial_to, @to); @cc = (@initial_cc, @cc); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html