{"thread":{"id":"39577","subject":"[PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses","startedAt":"2015-06-09T18:50:03Z","lastAt":"2015-06-10T16:25:53Z","messageCount":11,"participants":["Remi Lespinet","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"263402","messageId":"1433875804-16007-1-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr","threadId":"39577","inReplyTo":null,"subject":"[PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses","fromName":"Remi Lespinet","fromEmail":"remi.lespinet@ensimag.grenoble-inp.fr","sentAt":"2015-06-09T18:50:03Z","receivedAt":"2015-06-09T18:50:03Z","isPatch":true,"sender":{"key":"remi.lespinet@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/11941160?v=4"},"body":"Remove leading and trailing whitespaces when sanitizing addresses so\nthat git send-email give the same output when passing arguments like\n\" jdoe@example.com \" or \"\\t jdoe@example.com \" as with\n\"jdoe@example.com\".\n\nThe next commit will introduce a test for this aswell.\n\nSigned-off-by: Remi Lespinet \n---\n git-send-email.perl | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ea03308..3d144bd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -978,6 +978,9 @@ sub sanitize_address {\n \t# remove garbage after email address\n \t$recipient =~ s/(.*>).*$/$1/;\n \n+\t# remove leading and trailing whitespace\n+\t$recipient =~ s/^\\s+|\\s+$//g;\n+\n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n \n \tif (not $recipient_name) {\n-- \n1.9.1\n"},{"id":"263403","messageId":"1433875804-16007-2-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr","threadId":"39577","inReplyTo":"1433875804-16007-1-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr","subject":"[PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Remi Lespinet","fromEmail":"remi.lespinet@ensimag.grenoble-inp.fr","sentAt":"2015-06-09T18:50:04Z","receivedAt":"2015-06-09T18:50:04Z","isPatch":true,"sender":{"key":"remi.lespinet@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/11941160?v=4"},"body":"As alias file formats supported by git send-email doesn't take\nwhitespace into account, it is useless to consider whitespaces in\nalias name. remove leading and trailing whitespace before expanding\nallow to recognize strings like \" alias\" or \"alias\\t\" passed by --to,\n--cc, --bcc options or by the git send-email prompt.\n\nSigned-off-by: Remi Lespinet \n---\n git-send-email.perl | 1 +\n t/t9001-send-email.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 3d144bd..34c8b8b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -787,6 +787,7 @@ sub expand_aliases {\n my %EXPANDED_ALIASES;\n sub expand_one_alias {\n \tmy $alias = shift;\n+\t$alias =~ s/^\\s+|\\s+$//g;\n \tif ($EXPANDED_ALIASES{$alias}) {\n \t\tdie \"fatal: alias '$alias' expands to itself\\n\";\n \t}\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 9aee474..bbfed56 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1692,4 +1692,28 @@ test_expect_success $PREREQ 'aliases work with email list' '\n \ttest_cmp expected-list actual-list\n '\n \n+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n+\techo \"alias to2 to2@example.com\" >.mutt &&\n+\techo \"alias cc1 Cc 1 \" >>.mutt &&\n+\ttest_config sendemail.aliasesfile \".mutt\" &&\n+\ttest_config sendemail.aliasfiletype mutt &&\n+\tTO1=$(echo \"QTo 1 \" | q_to_tab) &&\n+\tTO2=$(echo \"QZto2\" | qz_to_tab_space) &&\n+\tCC1=$(echo \"cc1\" | append_cr) &&\n+\tBCC1=$(echo \"Q bcc1@example.com Q\" | q_to_nul) &&\n+\tgit send-email \\\n+\t--dry-run \\\n+\t--from=\"\tExample \" \\\n+\t--to=\"$TO1\" \\\n+\t--to=\"$TO2\" \\\n+\t--to=\" to3@example.com \" \\\n+\t--cc=\"$CC1\" \\\n+\t--cc=\"Cc2 \" \\\n+\t--bcc=\"$BCC1\" \\\n+\t--bcc=\"bcc2@example.com\" \\\n+\t0001-add-master.patch | replace_variable_fields \\\n+\t>actual-list &&\n+\ttest_cmp expected-list actual-list\n+'\n+\n test_done\n-- \n1.9.1\n"},{"id":"263452","messageId":"vpqr3pk9dv3.fsf@anie.imag.fr","threadId":"39577","inReplyTo":"1433875804-16007-1-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-10T08:17:20Z","receivedAt":"2015-06-10T08:17:20Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nothing serious, but you did something weird while sending. This message\ndoes not have a References: or an In-reply-to: field, so it breaks\nthreading. See how it's displayed on\n\n http://thread.gmane.org/gmane.comp.version-control.git\n\nRemi Lespinet writes:\n\n> Remove leading and trailing whitespaces when sanitizing addresses so\n> that git send-email give the same output when passing arguments like\n> \" jdoe@example.com \" or \"\\t jdoe@example.com \" as with\n> \"jdoe@example.com\".\n>\n> The next commit will introduce a test for this aswell.\n\ns/aswell/as well/\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263453","messageId":"vpqa8w89d5x.fsf@anie.imag.fr","threadId":"39577","inReplyTo":"1433875804-16007-2-git-send-email-remi.lespinet@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-10T08:32:26Z","receivedAt":"2015-06-10T08:32:26Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Remi Lespinet writes:\n\n> As alias file formats supported by git send-email doesn't take\n> whitespace into account, it is useless to consider whitespaces in\n> alias name. remove leading and trailing whitespace before expanding\n\ns/remove/Remove/\n\n> allow to recognize strings like \" alias\" or \"alias\\t\" passed by --to,\n> --cc, --bcc options or by the git send-email prompt.\n>\n> Signed-off-by: Remi Lespinet \n> ---\n> git-send-email.perl | 1 +\n> t/t9001-send-email.sh | 24 ++++++++++++++++++++++++\n> 2 files changed, 25 insertions(+)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 3d144bd..34c8b8b 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -787,6 +787,7 @@ sub expand_aliases {\n> my %EXPANDED_ALIASES;\n> sub expand_one_alias {\n> \tmy $alias = shift;\n> +\t$alias =~ s/^\\s+|\\s+$//g;\n> \tif ($EXPANDED_ALIASES{$alias}) {\n> \t\tdie \"fatal: alias '$alias' expands to itself\\n\";\n> \t}\n\nYou should explain why you need that, when the previous patch was\nalready about removing whitespaces around addresses. I finally\nunderstood that this was needed because alias expansion comes before\nsanitize_address, but the commit message should have told me that.\n\nActually, once you have this, PATCH 6/7 becomes useless, right? (at\nleast, the test passes if I revert it)\n\nIt seems to me that doing this space trimming just once, inside or right\nafter split_at_commas would be clearer.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263454","messageId":"1862589361.335676.1433925234231.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39577","inReplyTo":"vpqr3pk9dv3.fsf@anie.imag.fr","subject":"[PATCH v3 6/7] send-email: suppress leading and trailing whitespaces in addresses","fromName":"Remi Lespinet","fromEmail":"remi.lespinet@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T08:33:54Z","receivedAt":"2015-06-10T08:33:54Z","isPatch":true,"sender":{"key":"remi.lespinet@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/11941160?v=4"},"body":"> Nothing serious, but you did something weird while sending. This message\n> does not have a References: or an In-reply-to: field, so it breaks\n> threading. See how it's displayed on\n> \n> http://thread.gmane.org/gmane.comp.version-control.git\n\nYes, send-email was aborted after 5/7, I realized and retry\nsending 6/7 and 7/7 but I didn't noticed that. I'll be\ncareful next time, thanks.\n"},{"id":"263455","messageId":"1281238070.338321.1433928615479.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39577","inReplyTo":"vpqa8w89d5x.fsf@anie.imag.fr","subject":"[PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Remi Lespinet","fromEmail":"remi.lespinet@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T09:30:15Z","receivedAt":"2015-06-10T09:30:15Z","isPatch":true,"sender":{"key":"remi.lespinet@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/11941160?v=4"},"body":"Matthieu Moy writes:\n\n> Actually, once you have this, PATCH 6/7 becomes useless, right? (at\n> least, the test passes if I revert it)\n\n> It seems to me that doing this space trimming just once, inside or right\n> after split_at_commas would be clearer.\n\nYou're right, I put it twice because of there's occurrences of\nsanitize_address which are not associated with expand_aliases, but it\nseems that it's all taken care of separately in different regexp. So\nthere's no point to 6/7.\n\nI agree, I'd like to put it right after split_at_commas in a separate\nfunction \"trim_list\". Is it a good idea even if the function is one\nline long ?\n"},{"id":"263476","messageId":"xmqqa8w7oarl.fsf@gitster.dls.corp.google.com","threadId":"39577","inReplyTo":"1281238070.338321.1433928615479.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T15:15:10Z","receivedAt":"2015-06-10T15:15:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Remi Lespinet writes:\n\n> I agree, I'd like to put it right after split_at_commas in a separate\n> function \"trim_list\". Is it a good idea even if the function is one\n> line long ?\n\nHmph, if I have \"A, B, C\" and call a function that gives an array of\naddresses, treating the input as comma-separated addresses, I would\nexpect (\"A\", \"B\", \"C\") to be returned from that function, instead of\nhaving to later trim the whitespace around what is returned.\n\nIt suggests that split-at-commas _is_ a wrong abstraction, doesn't\nit? In other words, I think whitespace trimming is part of what the\nsplit-a-single-string-into-array-of-addresses helper function should\nbe doing.\n"},{"id":"263485","messageId":"vpqoaknr39r.fsf@anie.imag.fr","threadId":"39577","inReplyTo":"xmqqa8w7oarl.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-10T15:28:48Z","receivedAt":"2015-06-10T15:28:48Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Remi Lespinet writes:\n>\n>> I agree, I'd like to put it right after split_at_commas in a separate\n>> function \"trim_list\". Is it a good idea even if the function is one\n>> line long ?\n>\n> Hmph, if I have \"A, B, C\" and call a function that gives an array of\n> addresses, treating the input as comma-separated addresses, I would\n> expect (\"A\", \"B\", \"C\") to be returned from that function, instead of\n> having to later trim the whitespace around what is returned.\n\nIt is actually doing this. But if you have \" A,B,C \", then you'll get\n\" A\", \"B\", \"C \". But once you're trimming around commas, trimming\nleading and trailing spaces fits well with split itself.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263493","messageId":"595924690.351865.1433952607248.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39577","inReplyTo":"vpqoaknr39r.fsf@anie.imag.fr","subject":"[PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Remi Lespinet","fromEmail":"remi.lespinet@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T16:10:07Z","receivedAt":"2015-06-10T16:10:07Z","isPatch":true,"sender":{"key":"remi.lespinet@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/11941160?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n> \n> > Remi Lespinet writes:\n> >\n> >> I agree, I'd like to put it right after split_at_commas in a separate\n> >> function \"trim_list\". Is it a good idea even if the function is one\n> >> line long ?\n> >\n> > Hmph, if I have \"A, B, C\" and call a function that gives an array of\n> > addresses, treating the input as comma-separated addresses, I would\n> > expect (\"A\", \"B\", \"C\") to be returned from that function, instead of\n> > having to later trim the whitespace around what is returned.\n> \n> It is actually doing this. But if you have \" A,B,C \", then you'll get\n> \" A\", \"B\", \"C \". But once you're trimming around commas, trimming\n> leading and trailing spaces fits well with split itself.\n\nYes and if we have a single address with leading or/and trailing\nwhitespaces, such as \" A \", I think that we don't expect\nsplit_in_commas to suppress these whitespaces as there's no commas in\nthis address. As Junio said, I think I should rename the function.\n\nThanks!\n"},{"id":"263496","messageId":"xmqqfv5zmtf0.fsf@gitster.dls.corp.google.com","threadId":"39577","inReplyTo":"vpqoaknr39r.fsf@anie.imag.fr","subject":"Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T16:15:15Z","receivedAt":"2015-06-10T16:15:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Remi Lespinet writes:\n>>\n>>> I agree, I'd like to put it right after split_at_commas in a separate\n>>> function \"trim_list\". Is it a good idea even if the function is one\n>>> line long ?\n>>\n>> Hmph, if I have \"A, B, C\" and call a function that gives an array of\n>> addresses, treating the input as comma-separated addresses, I would\n>> expect (\"A\", \"B\", \"C\") to be returned from that function, instead of\n>> having to later trim the whitespace around what is returned.\n>\n> It is actually doing this. But if you have \" A,B,C \", then you'll get\n> \" A\", \"B\", \"C \". But once you're trimming around commas, trimming\n> leading and trailing spaces fits well with split itself.\n\nI guess we are saying the same thing, then? That is, trim-list as a\nseparate step does not make sense an it is part of the job for the\nhelper to turn a single list with multiple addresses into an array?\n"},{"id":"263501","messageId":"vpq8ubrwmwe.fsf@anie.imag.fr","threadId":"39577","inReplyTo":"xmqqfv5zmtf0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 7/7] send-email: suppress leading and trailing whitespaces before alias expansion","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-10T16:25:53Z","receivedAt":"2015-06-10T16:25:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Hmph, if I have \"A, B, C\" and call a function that gives an array of\n>>> addresses, treating the input as comma-separated addresses, I would\n>>> expect (\"A\", \"B\", \"C\") to be returned from that function, instead of\n>>> having to later trim the whitespace around what is returned.\n>>\n>> It is actually doing this. But if you have \" A,B,C \", then you'll get\n>> \" A\", \"B\", \"C \". But once you're trimming around commas, trimming\n>> leading and trailing spaces fits well with split itself.\n>\n> I guess we are saying the same thing, then? That is, trim-list as a\n> separate step does not make sense an it is part of the job for the\n> helper to turn a single list with multiple addresses into an array?\n\nYes. I was clarifying what was done and what wasn't, not disagreeing.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}