* [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses @ 2015-06-09 18:50 Remi Lespinet 2015-06-09 18:50 ` [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion Remi Lespinet 2015-06-10 8:17 ` [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Matthieu Moy 0 siblings, 2 replies; 11+ messages in thread From: Remi Lespinet @ 2015-06-09 18:50 UTC (permalink / raw) To: git Cc: Remi Galan, Remi Lespinet, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite, Matthieu Moy Remove leading and trailing whitespaces when sanitizing addresses so that git send-email give the same output when passing arguments like " jdoe@example•com " or "\t jdoe@example•com " as with "jdoe@example•com". The next commit will introduce a test for this aswell. Signed-off-by: Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> --- git-send-email.perl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-send-email.perl b/git-send-email.perl index ea03308..3d144bd 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -978,6 +978,9 @@ sub sanitize_address { # remove garbage after email address $recipient =~ s/(.*>).*$/$1/; + # remove leading and trailing whitespace + $recipient =~ s/^\s+|\s+$//g; + my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/); if (not $recipient_name) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-09 18:50 [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Remi Lespinet @ 2015-06-09 18:50 ` Remi Lespinet 2015-06-10 8:32 ` Matthieu Moy 2015-06-10 8:17 ` [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Matthieu Moy 1 sibling, 1 reply; 11+ messages in thread From: Remi Lespinet @ 2015-06-09 18:50 UTC (permalink / raw) To: git Cc: Remi Galan, Remi Lespinet, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite, Matthieu Moy As alias file formats supported by git send-email doesn't take whitespace into account, it is useless to consider whitespaces in alias name. remove leading and trailing whitespace before expanding allow to recognize strings like " alias" or "alias\t" passed by --to, --cc, --bcc options or by the git send-email prompt. Signed-off-by: Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> --- git-send-email.perl | 1 + t/t9001-send-email.sh | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/git-send-email.perl b/git-send-email.perl index 3d144bd..34c8b8b 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -787,6 +787,7 @@ sub expand_aliases { my %EXPANDED_ALIASES; sub expand_one_alias { my $alias = shift; + $alias =~ s/^\s+|\s+$//g; if ($EXPANDED_ALIASES{$alias}) { die "fatal: alias '$alias' expands to itself\n"; } diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 9aee474..bbfed56 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1692,4 +1692,28 @@ test_expect_success $PREREQ 'aliases work with email list' ' test_cmp expected-list actual-list ' +test_expect_success $PREREQ 'leading and trailing whitespaces are removed' ' + echo "alias to2 to2@example•com" >.mutt && + echo "alias cc1 Cc 1 <cc1@example•com>" >>.mutt && + test_config sendemail.aliasesfile ".mutt" && + test_config sendemail.aliasfiletype mutt && + TO1=$(echo "QTo 1 <to1@example•com>" | q_to_tab) && + TO2=$(echo "QZto2" | qz_to_tab_space) && + CC1=$(echo "cc1" | append_cr) && + BCC1=$(echo "Q bcc1@example•com Q" | q_to_nul) && + git send-email \ + --dry-run \ + --from=" Example <from@example•com>" \ + --to="$TO1" \ + --to="$TO2" \ + --to=" to3@example•com " \ + --cc="$CC1" \ + --cc="Cc2 <cc2@example•com>" \ + --bcc="$BCC1" \ + --bcc="bcc2@example•com" \ + 0001-add-master.patch | replace_variable_fields \ + >actual-list && + test_cmp expected-list actual-list +' + test_done -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-09 18:50 ` [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion Remi Lespinet @ 2015-06-10 8:32 ` Matthieu Moy 2015-06-10 9:30 ` Remi Lespinet 0 siblings, 1 reply; 11+ messages in thread From: Matthieu Moy @ 2015-06-10 8:32 UTC (permalink / raw) To: Remi Lespinet Cc: git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: > As alias file formats supported by git send-email doesn't take > whitespace into account, it is useless to consider whitespaces in > alias name. remove leading and trailing whitespace before expanding s/remove/Remove/ > allow to recognize strings like " alias" or "alias\t" passed by --to, > --cc, --bcc options or by the git send-email prompt. > > Signed-off-by: Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> > --- > git-send-email.perl | 1 + > t/t9001-send-email.sh | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 3d144bd..34c8b8b 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -787,6 +787,7 @@ sub expand_aliases { > my %EXPANDED_ALIASES; > sub expand_one_alias { > my $alias = shift; > + $alias =~ s/^\s+|\s+$//g; > if ($EXPANDED_ALIASES{$alias}) { > die "fatal: alias '$alias' expands to itself\n"; > } You should explain why you need that, when the previous patch was already about removing whitespaces around addresses. I finally understood that this was needed because alias expansion comes before sanitize_address, but the commit message should have told me that. Actually, once you have this, PATCH 6/7 becomes useless, right? (at least, the test passes if I revert it) It seems to me that doing this space trimming just once, inside or right after split_at_commas would be clearer. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 8:32 ` Matthieu Moy @ 2015-06-10 9:30 ` Remi Lespinet 2015-06-10 15:15 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Remi Lespinet @ 2015-06-10 9:30 UTC (permalink / raw) To: Matthieu Moy Cc: git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Matthieu Moy <Matthieu.Moy@grenoble-inp•fr> writes: > Actually, once you have this, PATCH 6/7 becomes useless, right? (at > least, the test passes if I revert it) > It seems to me that doing this space trimming just once, inside or right > after split_at_commas would be clearer. You're right, I put it twice because of there's occurrences of sanitize_address which are not associated with expand_aliases, but it seems that it's all taken care of separately in different regexp. So there's no point to 6/7. I agree, I'd like to put it right after split_at_commas in a separate function "trim_list". Is it a good idea even if the function is one line long ? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 9:30 ` Remi Lespinet @ 2015-06-10 15:15 ` Junio C Hamano 2015-06-10 15:28 ` Matthieu Moy 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2015-06-10 15:15 UTC (permalink / raw) To: Remi Lespinet Cc: Matthieu Moy, git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: > I agree, I'd like to put it right after split_at_commas in a separate > function "trim_list". Is it a good idea even if the function is one > line long ? Hmph, if I have "A, B, C" and call a function that gives an array of addresses, treating the input as comma-separated addresses, I would expect ("A", "B", "C") to be returned from that function, instead of having to later trim the whitespace around what is returned. It suggests that split-at-commas _is_ a wrong abstraction, doesn't it? In other words, I think whitespace trimming is part of what the split-a-single-string-into-array-of-addresses helper function should be doing. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 15:15 ` Junio C Hamano @ 2015-06-10 15:28 ` Matthieu Moy 2015-06-10 16:10 ` Remi Lespinet 2015-06-10 16:15 ` Junio C Hamano 0 siblings, 2 replies; 11+ messages in thread From: Matthieu Moy @ 2015-06-10 15:28 UTC (permalink / raw) To: Junio C Hamano Cc: Remi Lespinet, git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Junio C Hamano <gitster@pobox•com> writes: > Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: > >> I agree, I'd like to put it right after split_at_commas in a separate >> function "trim_list". Is it a good idea even if the function is one >> line long ? > > Hmph, if I have "A, B, C" and call a function that gives an array of > addresses, treating the input as comma-separated addresses, I would > expect ("A", "B", "C") to be returned from that function, instead of > having to later trim the whitespace around what is returned. It is actually doing this. But if you have " A,B,C ", then you'll get " A", "B", "C ". But once you're trimming around commas, trimming leading and trailing spaces fits well with split itself. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 15:28 ` Matthieu Moy @ 2015-06-10 16:10 ` Remi Lespinet 2015-06-10 16:15 ` Junio C Hamano 1 sibling, 0 replies; 11+ messages in thread From: Remi Lespinet @ 2015-06-10 16:10 UTC (permalink / raw) To: Matthieu Moy Cc: Junio C Hamano, git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Matthieu Moy <Matthieu.Moy@grenoble-inp•fr> writes: > Junio C Hamano <gitster@pobox•com> writes: > > > Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: > > > >> I agree, I'd like to put it right after split_at_commas in a separate > >> function "trim_list". Is it a good idea even if the function is one > >> line long ? > > > > Hmph, if I have "A, B, C" and call a function that gives an array of > > addresses, treating the input as comma-separated addresses, I would > > expect ("A", "B", "C") to be returned from that function, instead of > > having to later trim the whitespace around what is returned. > > It is actually doing this. But if you have " A,B,C ", then you'll get > " A", "B", "C ". But once you're trimming around commas, trimming > leading and trailing spaces fits well with split itself. Yes and if we have a single address with leading or/and trailing whitespaces, such as " A ", I think that we don't expect split_in_commas to suppress these whitespaces as there's no commas in this address. As Junio said, I think I should rename the function. Thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 15:28 ` Matthieu Moy 2015-06-10 16:10 ` Remi Lespinet @ 2015-06-10 16:15 ` Junio C Hamano 2015-06-10 16:25 ` Matthieu Moy 1 sibling, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2015-06-10 16:15 UTC (permalink / raw) To: Matthieu Moy Cc: Remi Lespinet, git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Matthieu Moy <Matthieu.Moy@grenoble-inp•fr> writes: > Junio C Hamano <gitster@pobox•com> writes: > >> Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: >> >>> I agree, I'd like to put it right after split_at_commas in a separate >>> function "trim_list". Is it a good idea even if the function is one >>> line long ? >> >> Hmph, if I have "A, B, C" and call a function that gives an array of >> addresses, treating the input as comma-separated addresses, I would >> expect ("A", "B", "C") to be returned from that function, instead of >> having to later trim the whitespace around what is returned. > > It is actually doing this. But if you have " A,B,C ", then you'll get > " A", "B", "C ". But once you're trimming around commas, trimming > leading and trailing spaces fits well with split itself. I guess we are saying the same thing, then? That is, trim-list as a separate step does not make sense an it is part of the job for the helper to turn a single list with multiple addresses into an array? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion 2015-06-10 16:15 ` Junio C Hamano @ 2015-06-10 16:25 ` Matthieu Moy 0 siblings, 0 replies; 11+ messages in thread From: Matthieu Moy @ 2015-06-10 16:25 UTC (permalink / raw) To: Junio C Hamano Cc: Remi Lespinet, git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Junio C Hamano <gitster@pobox•com> writes: > Matthieu Moy <Matthieu.Moy@grenoble-inp•fr> writes: > >> Junio C Hamano <gitster@pobox•com> writes: >> >>> Hmph, if I have "A, B, C" and call a function that gives an array of >>> addresses, treating the input as comma-separated addresses, I would >>> expect ("A", "B", "C") to be returned from that function, instead of >>> having to later trim the whitespace around what is returned. >> >> It is actually doing this. But if you have " A,B,C ", then you'll get >> " A", "B", "C ". But once you're trimming around commas, trimming >> leading and trailing spaces fits well with split itself. > > I guess we are saying the same thing, then? That is, trim-list as a > separate step does not make sense an it is part of the job for the > helper to turn a single list with multiple addresses into an array? Yes. I was clarifying what was done and what wasn't, not disagreeing. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses 2015-06-09 18:50 [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Remi Lespinet 2015-06-09 18:50 ` [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion Remi Lespinet @ 2015-06-10 8:17 ` Matthieu Moy 2015-06-10 8:33 ` Remi Lespinet 1 sibling, 1 reply; 11+ messages in thread From: Matthieu Moy @ 2015-06-10 8:17 UTC (permalink / raw) To: Remi Lespinet Cc: git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite Nothing serious, but you did something weird while sending. This message does not have a References: or an In-reply-to: field, so it breaks threading. See how it's displayed on http://thread.gmane.org/gmane.comp.version-control.git Remi Lespinet <remi.lespinet@ensimag•grenoble-inp.fr> writes: > Remove leading and trailing whitespaces when sanitizing addresses so > that git send-email give the same output when passing arguments like > " jdoe@example•com " or "\t jdoe@example•com " as with > "jdoe@example•com". > > The next commit will introduce a test for this aswell. s/aswell/as well/ -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses 2015-06-10 8:17 ` [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Matthieu Moy @ 2015-06-10 8:33 ` Remi Lespinet 0 siblings, 0 replies; 11+ messages in thread From: Remi Lespinet @ 2015-06-10 8:33 UTC (permalink / raw) To: Matthieu Moy Cc: git, Remi Galan, Guillaume Pages, Louis-Alexandre Stuber, Antoine Delaite > Nothing serious, but you did something weird while sending. This message > does not have a References: or an In-reply-to: field, so it breaks > threading. See how it's displayed on > > http://thread.gmane.org/gmane.comp.version-control.git Yes, send-email was aborted after 5/7, I realized and retry sending 6/7 and 7/7 but I didn't noticed that. I'll be careful next time, thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-06-10 16:26 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-09 18:50 [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Remi Lespinet 2015-06-09 18:50 ` [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion Remi Lespinet 2015-06-10 8:32 ` Matthieu Moy 2015-06-10 9:30 ` Remi Lespinet 2015-06-10 15:15 ` Junio C Hamano 2015-06-10 15:28 ` Matthieu Moy 2015-06-10 16:10 ` Remi Lespinet 2015-06-10 16:15 ` Junio C Hamano 2015-06-10 16:25 ` Matthieu Moy 2015-06-10 8:17 ` [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses Matthieu Moy 2015-06-10 8:33 ` Remi Lespinet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox