{"thread":{"id":"45155","subject":"body-CC-comment regression","startedAt":"2017-02-16T17:49:32Z","lastAt":"2017-02-26T20:45:55Z","messageCount":22,"participants":["Johan Hovold","Junio C Hamano","Matthieu Moy","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"311781","messageId":"20170216174924.GB2625@localhost","threadId":"45155","inReplyTo":null,"subject":"body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-16T17:49:24Z","receivedAt":"2017-02-16T17:49:32Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"Hi,\n\nI recently noticed that after an upgrade, git-send-email (2.10.2)\nstarted aborting when trying to send patches that had a linux-kernel\nstable-tag in its body. For example,\n\n\tCc: \t# 4.4\n\nwas now parsed as\n\n\t\"stable@vger.kernel.org#4.4\"\n\nwhich resulted in\n\n\tDied at /usr/libexec/git-core/git-send-email line 1332, line 1.\n\nThis tends to happen in the middle of a series after the cover letter\nhad been sent just to make things worse...\n\nI finally got around to looking into this today and discovered this\nthread (\"Re: Formatting problem send_mail in version 2.10.0\"):\n\n\thttps://marc.info/?l=git&m=147633706724793&w=2\n\nThe problem with the resulting fixes that are now in 2.11.1 is that\ngit-send-email no longer discards the trailing comment but rather\nshoves it into the name after adding some random white space:\n\n\t\"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle\" \"\n\nThis example is based on the example from\nDocumentation/process/stable-kernel-rules.rst:\n\n\tCc: # 3.3.x: 1b9508f: sched: Rate-limit newidle\n\nand this format for stable-tags has been documented at least since 2009\nand 8e9b9362266d (\"Doc/stable rules: add new cherry-pick logic\"), and\nhas been supported by git since 2012 and 831a488b76e0 (\"git-send-email:\nremove garbage after email address\") I believe.\n\nCan we please revert to the old behaviour of simply discarding such\ncomments (from body-CC:s) or at least make it configurable through a\nconfiguration option?\n\nThanks,\nJohan\n"},{"id":"311783","messageId":"xmqq60kayq2q.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"20170216174924.GB2625@localhost","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-16T17:59:25Z","receivedAt":"2017-02-16T17:59:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Hovold writes:\n\n> I recently noticed that after an upgrade, git-send-email (2.10.2)\n> started aborting when trying to send patches that had a linux-kernel\n> stable-tag in its body. For example,\n>\n> \tCc: \t# 4.4\n>\n> was now parsed as\n>\n> \t\"stable@vger.kernel.org#4.4\"\n> ...\n\nIt sounds like a fallout of this:\n\n https://public-inbox.org/git/41164484-309b-bfff-ddbb-55153495d41a@lwfinger.net/#t \n\nand any change to \"fix\" you may break the other person.\n\n> Can we please revert to the old behaviour of simply discarding such\n> comments (from body-CC:s) or at least make it configurable through a\n> configuration option?\n\nIf I recall the old thread correctly, it was reported that using\nMail::Address without forcing git-send-email fall back to its own\nnon-parsing-but-paste-address-looking-things-together code would\nsolve it, so can the \"make it configurable\" be just \"install\nMail::Address\"?\n"},{"id":"311788","messageId":"20170216181418.GC2625@localhost","threadId":"45155","inReplyTo":"xmqq60kayq2q.fsf@gitster.mtv.corp.google.com","subject":"Re: body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-16T18:14:18Z","receivedAt":"2017-02-16T18:14:24Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"On Thu, Feb 16, 2017 at 09:59:25AM -0800, Junio C Hamano wrote:\n> Johan Hovold writes:\n> \n> > I recently noticed that after an upgrade, git-send-email (2.10.2)\n> > started aborting when trying to send patches that had a linux-kernel\n> > stable-tag in its body. For example,\n> >\n> > \tCc: \t# 4.4\n> >\n> > was now parsed as\n> >\n> > \t\"stable@vger.kernel.org#4.4\"\n> > ...\n> \n> It sounds like a fallout of this:\n> \n> https://public-inbox.org/git/41164484-309b-bfff-ddbb-55153495d41a@lwfinger.net/#t \n> \n> and any change to \"fix\" you may break the other person.\n\nYes, that's the thread I was referring to as well, and the reported\nbreakage is the same even if the reporter used a non-standard stable-tag\nformat (e.g. a \"[4.8+]\" suffix).\n\nWhat I'm wondering is whether the alternative fix proposed in that\nthread, to revert to the old behaviour of discarding trailing comments,\nshould be considered instead of what was implemented.\n\n> > Can we please revert to the old behaviour of simply discarding such\n> > comments (from body-CC:s) or at least make it configurable through a\n> > configuration option?\n> \n> If I recall the old thread correctly, it was reported that using\n> Mail::Address without forcing git-send-email fall back to its own\n> non-parsing-but-paste-address-looking-things-together code would\n> solve it, so can the \"make it configurable\" be just \"install\n> Mail::Address\"?\n\nI believe git-send-email's parser was changed to mimic Mail::Address,\nand installing it does not seem to change the behaviour of including any\ntrailing comments in the name.\n\nThanks,\nJohan\n"},{"id":"311789","messageId":"vpqlgt6hug6.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"20170216174924.GB2625@localhost","subject":"Re: body-CC-comment regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-16T18:16:57Z","receivedAt":"2017-02-16T18:17:10Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johan Hovold writes:\n\n> Hi,\n>\n> I recently noticed that after an upgrade, git-send-email (2.10.2)\n> started aborting when trying to send patches that had a linux-kernel\n> stable-tag in its body. For example,\n>\n> \tCc: \t# 4.4\n>\n> was now parsed as\n>\n> \t\"stable@vger.kernel.org#4.4\"\n>\n> which resulted in\n>\n> \tDied at /usr/libexec/git-core/git-send-email line 1332, line 1.\n\nThis has changed in e3fdbcc8e1 (parse_mailboxes: accept extra text after\n<...> address, 2016-10-13), released v2.11.0 as you noticed:\n\n> The problem with the resulting fixes that are now in 2.11.1 is that\n> git-send-email no longer discards the trailing comment but rather\n> shoves it into the name after adding some random white space:\n>\n> \t\"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle\" \"\n>\n> This example is based on the example from\n> Documentation/process/stable-kernel-rules.rst:\n>\n> \tCc: # 3.3.x: 1b9508f: sched: Rate-limit newidle\n>\n> and this format for stable-tags has been documented at least since 2009\n> and 8e9b9362266d (\"Doc/stable rules: add new cherry-pick logic\"), and\n> has been supported by git since 2012 and 831a488b76e0 (\"git-send-email:\n> remove garbage after email address\") I believe.\n>\n> Can we please revert to the old behaviour of simply discarding such\n> comments (from body-CC:s) or at least make it configurable through a\n> configuration option?\n\nThe problem is that we now accept list of emails instead of just one\nemail, so it's hard to define what \"comments after the email\", for\nexample\n\nCc: # , \n\nIs not accepted as two emails.\n\nSo, just stripping whatever comes after # before parsing the list of\nemails would change the behavior once more, and possibly break other\nuser's flow. Dropping the garbage after the email while parsing is\npossible, but only when we use our in-house parser (and we currently use\nPerl's Mail::Address when available).\n\nSo, a proper fix is far from obvious, and unfortunately I won't have\ntime to work on that, at least not before a while.\n\nOTOH, the current behavior isn't that bad. It accepts the input, and\nextracts a valid email out of it. Just the display name is admitedly\nsuboptimal ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"311857","messageId":"20170217110642.GD2625@localhost","threadId":"45155","inReplyTo":"vpqlgt6hug6.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-17T11:06:42Z","receivedAt":"2017-02-17T11:07:12Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"On Thu, Feb 16, 2017 at 07:16:57PM +0100, Matthieu Moy wrote:\n> Johan Hovold writes:\n> \n> > Hi,\n> >\n> > I recently noticed that after an upgrade, git-send-email (2.10.2)\n> > started aborting when trying to send patches that had a linux-kernel\n> > stable-tag in its body. For example,\n> >\n> > \tCc: \t# 4.4\n> >\n> > was now parsed as\n> >\n> > \t\"stable@vger.kernel.org#4.4\"\n> >\n> > which resulted in\n> >\n> > \tDied at /usr/libexec/git-core/git-send-email line 1332, line 1.\n> \n> This has changed in e3fdbcc8e1 (parse_mailboxes: accept extra text after\n> <...> address, 2016-10-13), released v2.11.0 as you noticed:\n> \n> > The problem with the resulting fixes that are now in 2.11.1 is that\n> > git-send-email no longer discards the trailing comment but rather\n> > shoves it into the name after adding some random white space:\n> >\n> > \t\"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle\" \"\n> >\n> > This example is based on the example from\n> > Documentation/process/stable-kernel-rules.rst:\n> >\n> > \tCc: # 3.3.x: 1b9508f: sched: Rate-limit newidle\n> >\n> > and this format for stable-tags has been documented at least since 2009\n> > and 8e9b9362266d (\"Doc/stable rules: add new cherry-pick logic\"), and\n> > has been supported by git since 2012 and 831a488b76e0 (\"git-send-email:\n> > remove garbage after email address\") I believe.\n> >\n> > Can we please revert to the old behaviour of simply discarding such\n> > comments (from body-CC:s) or at least make it configurable through a\n> > configuration option?\n> \n> The problem is that we now accept list of emails instead of just one\n> email, so it's hard to define what \"comments after the email\", for\n> example\n> \n> Cc: # , \n> \n> Is not accepted as two emails.\n> \n> So, just stripping whatever comes after # before parsing the list of\n> emails would change the behavior once more, and possibly break other\n> user's flow. Dropping the garbage after the email while parsing is\n> possible, but only when we use our in-house parser (and we currently use\n> Perl's Mail::Address when available).\n> \n> So, a proper fix is far from obvious, and unfortunately I won't have\n> time to work on that, at least not before a while.\n\nThere is another option, namely to only accept a single address for tags\nin the body. I understand that being able to copy a CC-header to either\nthe header section or to the command line could be useful, but I don't\nreally see the point in allowing this in the tags in the body (a SoB\nalways has one address, and so should a CC-tag).\n\nAnd since this is a regression for something that has been working for\nyears that was introduced by a new feature, I also think it's reasonable\nto (partially) revert the feature.\n\n> OTOH, the current behavior isn't that bad. It accepts the input, and\n> extracts a valid email out of it. Just the display name is admitedly\n> suboptimal ...\n\nYeah, but the display name can end up with so much noise that auto-cc is\neffectively broken for people submitting kernel patches (with stable\ntags) as the only way to avoid it is to suppress all bodycc.\n\nSo what I'm proposing is to revert to the earlier behaviour of only\nallowing one address per body tag by simply discarding anything\nafter the address.\n\nSomething like the below seems to do the trick.\n\nThanks,\nJohan\n\n\n\nFrom f551b4ca9926624dc7af6c286d7cf0f97af39541 Mon Sep 17 00:00:00 2001\nFrom: Johan Hovold \nDate: Fri, 17 Feb 2017 11:55:47 +0100\nSubject: [PATCH] send-email: only allow one address per body tag\n\nAdding comments after a tag in the body is a common practise (e.g. in\nthe Linux kernel) and git-send-email has been supporting this for years\nby removing any trailing cruft after the address.\n\nAfter some recent changes, any trailing comment is now instead appended\nto the recipient name (with some random white space inserted) resulting\nin undesirable noise in the headers, for example:\n\nCC: \"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle\" \n\nRevert to the earlier behaviour of discarding anything after the (first)\naddress in a tag while parsing the body.\n\nNote that multiple addresses after are still allowed after a\ncommand-line switch (and in a CC-header).\n\nFixes: b1c8a11c8024 (\"send-email: allow multiple emails using --cc, --to\nand --bcc\")\nFixes: e3fdbcc8e164 (\"parse_mailboxes: accept extra text after <...>\naddress\")\nSigned-off-by: Johan Hovold \n---\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 068d60b3e698..eea0a517f71b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1563,7 +1563,7 @@ foreach my $t (@files) {\n \t# Now parse the message body\n \twhile(<$fh>) {\n \t\t$message .= $_;\n-\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n+\t\tif (/^(Signed-off-by|Cc): ([^>]*>?)/i) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n-- \n2.11.1\n"},{"id":"311860","messageId":"vpq7f4pdkjp.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"20170217110642.GD2625@localhost","subject":"Re: body-CC-comment regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-17T13:16:42Z","receivedAt":"2017-02-17T13:16:50Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johan Hovold writes:\n\n> There is another option, namely to only accept a single address for tags\n> in the body. I understand that being able to copy a CC-header to either\n> the header section or to the command line could be useful, but I don't\n> really see the point in allowing this in the tags in the body (a SoB\n> always has one address, and so should a CC-tag).\n\nI mostly agree for the SoB, but why should a Cc tag have only one email?\n\nThe \"multiple emails per Cc: field\" has been there for a while already\n(b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got\nused to it. What you are proposing breaks their flow.\n\n> And since this is a regression for something that has been working for\n> years that was introduced by a new feature, I also think it's reasonable\n> to (partially) revert the feature.\n\nI'd find it rather ironic to fix your case by breaking a feature that\nhas been working for more than a year :-(. What would you answer to a\ncontributor comming one year from now and proposing to revert your\nreversion because it breaks his flow?\n\nAll that said, I think another fix would be both satisfactory for\neveryone and rather simple:\n\n1) Stop calling Mail::Address even if available. It used to make sense\n to do that when our in-house parser was really poor, but we now have\n something essentially as good as Mail::Address. We test our parser\n against Mail::Address and we do have a few known differences (see\n t9000), but they are really corner-cases and shouldn't matter.\n\n A good consequence of this is that we stop depending on the way Perl\n is installed to parse emails. Regardless of the current issue, I\n think it is a good thing.\n\n2) Modify our in-house parser to discard garbage after the >. The patch\n should look like (untested):\n\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -903,11 +903,11 @@ sub parse_mailboxes {\n my (@addr_list, @phrase, @address, @comment, @buffer) = ();\n foreach my $token (@tokens) {\n if ($token =~ /^[,;]$/) {\n- # if buffer still contains undeterminated strings\n- # append it at the end of @address or @phrase\n- if ($end_of_addr_seen) {\n- push @phrase, @buffer;\n- } else {\n+ # if buffer still contains undeterminated\n+ # strings append it at the end of @address,\n+ # unless we already saw the closing >, in\n+ # which case we discard it.\n+ if (!$end_of_addr_seen) {\n push @address, @buffer;\n }\n \nWhat do you think?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"311900","messageId":"20170217164241.GE2625@localhost","threadId":"45155","inReplyTo":"vpq7f4pdkjp.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-17T16:42:41Z","receivedAt":"2017-02-17T16:42:51Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:\n> Johan Hovold writes:\n> \n> > There is another option, namely to only accept a single address for tags\n> > in the body. I understand that being able to copy a CC-header to either\n> > the header section or to the command line could be useful, but I don't\n> > really see the point in allowing this in the tags in the body (a SoB\n> > always has one address, and so should a CC-tag).\n> \n> I mostly agree for the SoB, but why should a Cc tag have only one email?\n\nFor symmetry (with SoB) and readability reasons (one tag per line).\nThese are body tags, not mail headers, after all.\n\n> The \"multiple emails per Cc: field\" has been there for a while already\n> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got\n> used to it. What you are proposing breaks their flow.\n\nNote that that commit never mentions multiple addresses in either\nheaders or body-tags -- it's all about being able to specify multiple\nentries on the command line.\n\nThere does not seem to be single commit in the kernel where multiple\naddress are specified in a CC tag since after git-send-email started\nallowing it, but there are ten commits before (to my surprise), and that\nshould be contrasted with at least 4178 commits with trailing comments\nincluding a # sign.\n\n> > And since this is a regression for something that has been working for\n> > years that was introduced by a new feature, I also think it's reasonable\n> > to (partially) revert the feature.\n> \n> I'd find it rather ironic to fix your case by breaking a feature that\n> has been working for more than a year :-(. What would you answer to a\n> contributor comming one year from now and proposing to revert your\n> reversion because it breaks his flow?\n\nSuch conflicts are not uncommon when dealing with regressions introduced\nby new features, and need to be dealt with on a case-by-case basis. But\nthe fact that trailing comments have been properly supported for more\nthan four years should carry some weight.\n\n> All that said, I think another fix would be both satisfactory for\n> everyone and rather simple:\n> \n> 1) Stop calling Mail::Address even if available. It used to make sense\n> to do that when our in-house parser was really poor, but we now have\n> something essentially as good as Mail::Address. We test our parser\n> against Mail::Address and we do have a few known differences (see\n> t9000), but they are really corner-cases and shouldn't matter.\n>\n> A good consequence of this is that we stop depending on the way Perl\n> is installed to parse emails. Regardless of the current issue, I\n> think it is a good thing.\n\nRight, that sounds like the right thing to do regardless.\n\n> 2) Modify our in-house parser to discard garbage after the >. The patch\n> should look like (untested):\n> \n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -903,11 +903,11 @@ sub parse_mailboxes {\n> my (@addr_list, @phrase, @address, @comment, @buffer) = ();\n> foreach my $token (@tokens) {\n> if ($token =~ /^[,;]$/) {\n> - # if buffer still contains undeterminated strings\n> - # append it at the end of @address or @phrase\n> - if ($end_of_addr_seen) {\n> - push @phrase, @buffer;\n> - } else {\n> + # if buffer still contains undeterminated\n> + # strings append it at the end of @address,\n> + # unless we already saw the closing >, in\n> + # which case we discard it.\n> + if (!$end_of_addr_seen) {\n> push @address, @buffer;\n> }\n> \n> What do you think?\n\nSounds perfectly fine to me, and seems to work too after quick test.\n\nNote however that there's another minor issue with using multiple\naddresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess\nthat can be fixed separately.\n\nThanks,\nJohan\n"},{"id":"311903","messageId":"vpq4lzs7o0s.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"20170217164241.GE2625@localhost","subject":"Re: body-CC-comment regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-17T16:58:11Z","receivedAt":"2017-02-17T16:58:19Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"> On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:\n>> Johan Hovold writes:\n>\n>> The \"multiple emails per Cc: field\" has been there for a while already\n>> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got\n>> used to it. What you are proposing breaks their flow.\n>\n> Note that that commit never mentions multiple addresses in either\n> headers or body-tags -- it's all about being able to specify multiple\n> entries on the command line.\n\nIndeed. I'm not the author of the patch, but I was supervising the\nstudents who wrote it and \"multiple addresses in Cc:\" was not the goal,\nbut a (IMHO positive) side effect we discovered after the fact.\n\nIf I had a time machine, I'd probably go back then and forbid multiple\naddresses there, but ...\n\n> There does not seem to be single commit in the kernel where multiple\n> address are specified in a CC tag since after git-send-email started\n> allowing it, but there are ten commits before (to my surprise), and that\n> should be contrasted with at least 4178 commits with trailing comments\n> including a # sign.\n\nHey, there's a life outside the kernel ;-).\n\n>> 1) Stop calling Mail::Address even if available.[...]\n>\n> Right, that sounds like the right thing to do regardless.\n>\n>> 2) Modify our in-house parser to discard garbage after the >. [...]\n>\n> Sounds perfectly fine to me, and seems to work too after quick test.\n\nOK, sounds like the way to go.\n\nDo you want to work on a patch? If not, I should be able to do that\nmyself. The code changes are straightforward, but we probably want a\nproper test for that.\n\n> addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess\n> that can be fixed separately.\n\nOK. If it's unrelated enough, please start a separate thread to explain\nthe problem (and/or write a patch ;-) ).\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"311912","messageId":"20170217173004.GF2625@localhost","threadId":"45155","inReplyTo":"vpq4lzs7o0s.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-17T17:30:04Z","receivedAt":"2017-02-17T17:30:12Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"On Fri, Feb 17, 2017 at 05:58:11PM +0100, Matthieu Moy wrote:\n> > On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:\n> >> Johan Hovold writes:\n> >\n> >> The \"multiple emails per Cc: field\" has been there for a while already\n> >> (b1c8a11c8024 released in 2.6.0, sept 2015), some users may have got\n> >> used to it. What you are proposing breaks their flow.\n> >\n> > Note that that commit never mentions multiple addresses in either\n> > headers or body-tags -- it's all about being able to specify multiple\n> > entries on the command line.\n> \n> Indeed. I'm not the author of the patch, but I was supervising the\n> students who wrote it and \"multiple addresses in Cc:\" was not the goal,\n> but a (IMHO positive) side effect we discovered after the fact.\n\nYeah, and the broken --suppress-cc=self I mention below is indicative\nof that too.\n\n> If I had a time machine, I'd probably go back then and forbid multiple\n> addresses there, but ...\n> \n> > There does not seem to be single commit in the kernel where multiple\n> > address are specified in a CC tag since after git-send-email started\n> > allowing it, but there are ten commits before (to my surprise), and that\n> > should be contrasted with at least 4178 commits with trailing comments\n> > including a # sign.\n> \n> Hey, there's a life outside the kernel ;-).\n\nSure, but it's the origin of git as well as the tags we're discussing (I\nbelieve).\n\nMy point of bringing it up was that the multiple addresses in a CC-tag\nwas indeed an unintended (and undocumented) side-effect and I doubt many\npeople have started using it given that it's sort of counter-intuitive\n(again, compare with SoB).\n\nIf either the trailing comments or multiple addresses in a CC-tag has to\ngo, I think dropping the latter is clearly the best choice.\n\n> >> 1) Stop calling Mail::Address even if available.[...]\n> >\n> > Right, that sounds like the right thing to do regardless.\n> >\n> >> 2) Modify our in-house parser to discard garbage after the >. [...]\n> >\n> > Sounds perfectly fine to me, and seems to work too after quick test.\n> \n> OK, sounds like the way to go.\n> \n> Do you want to work on a patch? If not, I should be able to do that\n> myself. The code changes are straightforward, but we probably want a\n> proper test for that.\n\nFeel free to implement it this way if that's what people prefer. As long\nas trailing comments are supported and discarded, I don't really have a\npreference.\n\n> > addresses in a Cc-tag in that it breaks --suppress-cc=self, but I guess\n> > that can be fixed separately.\n> \n> OK. If it's unrelated enough, please start a separate thread to explain\n> the problem (and/or write a patch ;-) ).\n\nWell, it's related to the \"offending\" patch that added support for\nmultiple addresses in tags. By disallowing that, as my fix does, the\nproblem goes away.\n\n\t# Now parse the message body\n\twhile(<$fh>) {\n\t\t$message .= $_;\n\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n\t\t\tchomp;\n\t\t\tmy ($what, $c) = ($1, $2);\n\t\t\tchomp $c;\n\t\t\tmy $sc = sanitize_address($c);\n\n\t\t\tif ($sc eq $sender) {\n\t\t\t\tnext if ($suppress_cc{'self'});\n\nThe problem here is that $sc will never match $sender when there are more\nthan one address in a tag. For example:\n\n\tFrom: Johan Hovold \n\t...\n\n\tCc: alpha , Johan Hovold \n\nresults in\n\n\tsc = alpha , Johan Hovold \n\tsender = Johan Hovold \n\nso that --suppress-cc=self is not honoured.\n\nThanks,\nJohan\n"},{"id":"311913","messageId":"CA+55aFxPsRXKn=4jXUdPy1hh3iHagjoxMcpA5K-ENA-NtdnF-A@mail.gmail.com","threadId":"45155","inReplyTo":"vpq7f4pdkjp.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2017-02-17T17:38:17Z","receivedAt":"2017-02-17T17:38:22Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, Feb 17, 2017 at 5:16 AM, Matthieu Moy\n wrote:\n>\n> I mostly agree for the SoB, but why should a Cc tag have only one email?\n\nBecause changing that clearly broke real and useful behavior.\n\nThe \"multiple email addresses\" thing is bogus and wrong. Just don't do it.\n\nHow would you even parse it sanely? Are the Cc: lines now\nSMTP-compliant with the whole escaping and all the usual \"next line\"\nrules?\n\nFor example, in email, the rule for \"next line\" is that if you're in a\nheader block, and it starts with whitespace, then it's a continuation\nof the last line.\n\nThat's *not* how Cc: lines work in commit messages. They are all\nindividual lines, and we have lots of tools (mainly just scripts with\ngrepping) that simply depend on it.\n\nSo this notion that the bottom of the commit message is some email\nheader crap is WRONG.\n\nStop it. It caused bugs. It's wrong. Don't do it.\n\n Linus\n"},{"id":"311915","messageId":"xmqqd1egu1dl.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"vpq4lzs7o0s.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T18:18:46Z","receivedAt":"2017-02-17T18:18:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n>> On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:\n> ...\n> If I had a time machine, I'd probably go back then and forbid multiple\n> addresses there, but ...\n>\n>> There does not seem to be single commit in the kernel where multiple\n>> address are specified in a CC tag since after git-send-email started\n>> allowing it, but there are ten commits before (to my surprise), and that\n>> should be contrasted with at least 4178 commits with trailing comments\n>> including a # sign.\n>\n> Hey, there's a life outside the kernel ;-).\n> ...\n>>> 1) Stop calling Mail::Address even if available.[...]\n>>\n>> Right, that sounds like the right thing to do regardless.\n>>\n>>> 2) Modify our in-house parser to discard garbage after the >. [...]\n>>\n>> Sounds perfectly fine to me, and seems to work too after quick test.\n>\n> OK, sounds like the way to go.\n>\n> Do you want to work on a patch? If not, I should be able to do that\n> myself. The code changes are straightforward, but we probably want a\n> proper test for that.\n\nThe true headers and the things at the bottom seem to be handled in\na separate loop in send-email, so treating Cc: found in the former\nand in the latter differently should be doable. I think it is OK to\nexplicitly treat the latter as \"these are not e-mail addresses, but\njust a single e-mail address possibly with non-address cruft\",\nwithout losing the ability to have more than one addresses on a\nsingle CC: e-mail header.\n"},{"id":"311916","messageId":"20170217182326.GA479@localhost","threadId":"45155","inReplyTo":"xmqqd1egu1dl.fsf@gitster.mtv.corp.google.com","subject":"Re: body-CC-comment regression","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-17T18:23:26Z","receivedAt":"2017-02-17T18:23:46Z","isPatch":false,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"On Fri, Feb 17, 2017 at 10:18:46AM -0800, Junio C Hamano wrote:\n> Matthieu Moy writes:\n> \n> >> On Fri, Feb 17, 2017 at 02:16:42PM +0100, Matthieu Moy wrote:\n> > ...\n> > If I had a time machine, I'd probably go back then and forbid multiple\n> > addresses there, but ...\n> >\n> >> There does not seem to be single commit in the kernel where multiple\n> >> address are specified in a CC tag since after git-send-email started\n> >> allowing it, but there are ten commits before (to my surprise), and that\n> >> should be contrasted with at least 4178 commits with trailing comments\n> >> including a # sign.\n> >\n> > Hey, there's a life outside the kernel ;-).\n> > ...\n> >>> 1) Stop calling Mail::Address even if available.[...]\n> >>\n> >> Right, that sounds like the right thing to do regardless.\n> >>\n> >>> 2) Modify our in-house parser to discard garbage after the >. [...]\n> >>\n> >> Sounds perfectly fine to me, and seems to work too after quick test.\n> >\n> > OK, sounds like the way to go.\n> >\n> > Do you want to work on a patch? If not, I should be able to do that\n> > myself. The code changes are straightforward, but we probably want a\n> > proper test for that.\n> \n> The true headers and the things at the bottom seem to be handled in\n> a separate loop in send-email, so treating Cc: found in the former\n> and in the latter differently should be doable. I think it is OK to\n> explicitly treat the latter as \"these are not e-mail addresses, but\n> just a single e-mail address possibly with non-address cruft\",\n> without losing the ability to have more than one addresses on a\n> single CC: e-mail header.\n\nThat's precisely what the patch I posted earlier in the thread did.\n\nThanks,\nJohan\n"},{"id":"311919","messageId":"xmqq4lzsu0wo.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"20170217182326.GA479@localhost","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T18:28:55Z","receivedAt":"2017-02-17T18:29:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Hovold writes:\n\n> That's precisely what the patch I posted earlier in the thread did.\n\nThat's good. I didn't see any patch yet but the message you are\nresponding to is a response to Matthieu's message asking if you are\nplanning to work on it, so I'd assume you are and and look forward\nto seeing a patch (or a series?) we can queue.\n\nThanks.\n"},{"id":"311923","messageId":"vpq7f4owtbi.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"xmqq4lzsu0wo.fsf@gitster.mtv.corp.google.com","subject":"Re: body-CC-comment regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-17T18:44:33Z","receivedAt":"2017-02-17T18:44:41Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Johan Hovold writes:\n>\n>> That's precisely what the patch I posted earlier in the thread did.\n>\n> That's good. I didn't see any patch yet \n\nIt's here:\n\nhttp://public-inbox.org/git/20170217110642.GD2625@localhost/\n\nbut as I explained, this removes a feature suported since several major\nreleases and we have no idea how many users may use the \"mupliple emails\nin one field\". The approach I proposed does not suffer from this.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"311929","messageId":"xmqqd1egshv1.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"vpq7f4owtbi.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T20:05:38Z","receivedAt":"2017-02-17T20:08:41Z","isPatch":false,"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>> Johan Hovold writes:\n>>\n>>> That's precisely what the patch I posted earlier in the thread did.\n>>\n>> That's good. I didn't see any patch yet \n>\n> It's here:\n>\n> http://public-inbox.org/git/20170217110642.GD2625@localhost/\n>\n> but as I explained, this removes a feature suported since several major\n> releases and we have no idea how many users may use the \"mupliple emails\n> in one field\". The approach I proposed does not suffer from this.\n\nOK, so you are aiming higher to please both parties, i.e. those who\nplace a single address but cruft at the end would get the cruft\nstripped when we grab a usable address from the field, and those who\nwrite two or more addresses would get all of these addresses?\n\nThat approach may still constrain what those in the former camp can\nwrite in the \"cruft\" part, like they cannot write comma or semicolon\nas part of the \"cruft\", no? If that does not pose a practical problem,\nthen I can imagine it would work well for people in both camps.\n\n"},{"id":"311931","messageId":"vpq1suwvab9.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"xmqqd1egshv1.fsf@gitster.mtv.corp.google.com","subject":"Re: body-CC-comment regression","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-17T20:20:26Z","receivedAt":"2017-02-17T20:20:34Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> That approach may still constrain what those in the former camp can\n> write in the \"cruft\" part, like they cannot write comma or semicolon\n> as part of the \"cruft\", no?\n\nRight. Indeed, this may be a problem since the use of \"#\" for stable\nseem to include commit message, and they may contain commas.\n\nSo, maybe Johan's patch is better indeed.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"311950","messageId":"xmqqbmu0qwyg.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"vpq1suwvab9.fsf@anie.imag.fr","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T22:22:31Z","receivedAt":"2017-02-17T22:22:40Z","isPatch":false,"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>> That approach may still constrain what those in the former camp can\n>> write in the \"cruft\" part, like they cannot write comma or semicolon\n>> as part of the \"cruft\", no?\n>\n> Right. Indeed, this may be a problem since the use of \"#\" for stable\n> seem to include commit message, and they may contain commas.\n>\n> So, maybe Johan's patch is better indeed.\n\nOK, so I'll queue that one with your Ack for now so that we won't\nforget. I guess we still want a few tests?\n\nThanks.\n"},{"id":"311972","messageId":"xmqqbmu0pgg6.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"xmqqbmu0qwyg.fsf@gitster.mtv.corp.google.com","subject":"Re: body-CC-comment regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T23:04:25Z","receivedAt":"2017-02-17T23:10:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> That approach may still constrain what those in the former camp can\n>>> write in the \"cruft\" part, like they cannot write comma or semicolon\n>>> as part of the \"cruft\", no?\n>>\n>> Right. Indeed, this may be a problem since the use of \"#\" for stable\n>> seem to include commit message, and they may contain commas.\n>>\n>> So, maybe Johan's patch is better indeed.\n>\n> OK, so I'll queue that one with your Ack for now so that we won't\n> forget. I guess we still want a few tests?\n\nIt seems that there is an expectation in one of the tests that needs\nto be adjusted.\n"},{"id":"312137","messageId":"20170220114406.19436-1-johan@kernel.org","threadId":"45155","inReplyTo":"xmqqbmu0pgg6.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2] send-email: only allow one address per body tag","fromName":"Johan Hovold","fromEmail":"johan@kernel.org","sentAt":"2017-02-20T11:44:06Z","receivedAt":"2017-02-20T11:46:03Z","isPatch":true,"sender":{"key":"johan@kernel.org","avatar":"https://avatars.githubusercontent.com/u/601896?v=4"},"body":"Adding comments after a tag in the body is a common practise (e.g. in\nthe Linux kernel) and git-send-email has been supporting this for years\nby removing any trailing cruft after the address.\n\nAfter some recent changes, any trailing comment is now instead appended\nto the recipient name (with some random white space inserted) resulting\nin undesirable noise in the headers, for example:\n\nCC: \"# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle\" \n\nRevert to the earlier behaviour of discarding anything after the (first)\naddress in a tag while parsing the body.\n\nNote that multiple addresses after are still allowed after a command\nline switch (and in a CC header field).\n\nAlso note that --suppress-cc=self was never honoured when using multiple\naddresses in a tag.\n\nFixes: b1c8a11c8024 (\"send-email: allow multiple emails using --cc, --to\nand --bcc\")\nFixes: e3fdbcc8e164 (\"parse_mailboxes: accept extra text after <...>\naddress\")\nSigned-off-by: Johan Hovold \n---\n\nv2:\n - update the cc-trailer test\n - amend commit message and mention the broken --suppress-cc=self\n\n\n git-send-email.perl | 2 +-\n t/t9001-send-email.sh | 7 +++----\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 068d60b3e698..eea0a517f71b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1563,7 +1563,7 @@ foreach my $t (@files) {\n \t# Now parse the message body\n \twhile(<$fh>) {\n \t\t$message .= $_;\n-\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n+\t\tif (/^(Signed-off-by|Cc): ([^>]*>?)/i) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0f398dd1603d..60a80f60b268 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -148,7 +148,6 @@ cat >expected-cc <<\\EOF\n !two@example.com!\n !three@example.com!\n !four@example.com!\n-!five@example.com!\n EOF\n \"\n \n@@ -159,9 +158,9 @@ test_expect_success $PREREQ 'cc trailer with various syntax' '\n \tTest Cc: trailers.\n \n \tCc: one@example.com\n-\tCc: # this is part of the name\n-\tCc: , # not.five@example.com\n-\tCc: \"Some # Body\" [part.of.name.too]\n+\tCc: # trailing comments are ignored\n+\tCc: , one address per line\n+\tCc: \"Some # Body\" [ ]\n \tEOF\n \tclean_fake_sendmail &&\n \tgit send-email -1 --to=recipient@example.com \\\n-- \n2.11.1\n\n"},{"id":"312138","messageId":"vpqo9xxkqqo.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"20170220114406.19436-1-johan@kernel.org","subject":"Re: [PATCH v2] send-email: only allow one address per body tag","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-20T12:10:07Z","receivedAt":"2017-02-20T12:10:15Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johan Hovold writes:\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1563,7 +1563,7 @@ foreach my $t (@files) {\n> \t# Now parse the message body\n> \twhile(<$fh>) {\n> \t\t$message .= $_;\n> -\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n> +\t\tif (/^(Signed-off-by|Cc): ([^>]*>?)/i) {\n\nI think this is acceptable, but this doesn't work with trailers like\n\nCc: \"Some > Body\" \n\nA proper management of this kind of weird address should be doable by\nreusing the regexp parsing \"...\" in parse_mailbox:\n\n\tmy $re_quote = qr/\"(?:[^\\\"\\\\]|\\\\.)*\"/;\n\nSo the final regex would look like\n\nif (/^(Signed-off-by|Cc): (([^>]*|\"(?:[^\\\"\\\\]|\\\\.)*\")>?)/i) {\n\nI don't think that should block the patch inclusion, but it may be worth\nconsidering.\n\nAnyway, thanks for the patch!\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"312413","messageId":"xmqqfuj491sc.fsf@gitster.mtv.corp.google.com","threadId":"45155","inReplyTo":"vpqo9xxkqqo.fsf@anie.imag.fr","subject":"Re: [PATCH v2] send-email: only allow one address per body tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-23T18:53:39Z","receivedAt":"2017-02-23T18:53:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Johan Hovold writes:\n>\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -1563,7 +1563,7 @@ foreach my $t (@files) {\n>> \t# Now parse the message body\n>> \twhile(<$fh>) {\n>> \t\t$message .= $_;\n>> -\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n>> +\t\tif (/^(Signed-off-by|Cc): ([^>]*>?)/i) {\n>\n> I think this is acceptable, but this doesn't work with trailers like\n>\n> Cc: \"Some > Body\" \n>\n> A proper management of this kind of weird address should be doable by\n> reusing the regexp parsing \"...\" in parse_mailbox:\n>\n> \tmy $re_quote = qr/\"(?:[^\\\"\\\\]|\\\\.)*\"/;\n>\n> So the final regex would look like\n>\n> if (/^(Signed-off-by|Cc): (([^>]*|\"(?:[^\\\"\\\\]|\\\\.)*\")>?)/i) {\n>\n> I don't think that should block the patch inclusion, but it may be worth\n> considering.\n>\n> Anyway, thanks for the patch!\n\nSomehow this fell off the radar. So your reviewed-by: and then\nwe'll cook this in 'next' for a while?\n\nThanks.\n"},{"id":"312697","messageId":"vpq60jwr891.fsf@anie.imag.fr","threadId":"45155","inReplyTo":"xmqqfuj491sc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] send-email: only allow one address per body tag","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2017-02-26T20:45:46Z","receivedAt":"2017-02-26T20:45:55Z","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>> Johan Hovold writes:\n>>\n>>> --- a/git-send-email.perl\n>>> +++ b/git-send-email.perl\n>>> @@ -1563,7 +1563,7 @@ foreach my $t (@files) {\n>>> \t# Now parse the message body\n>>> \twhile(<$fh>) {\n>>> \t\t$message .= $_;\n>>> -\t\tif (/^(Signed-off-by|Cc): (.*)$/i) {\n>>> +\t\tif (/^(Signed-off-by|Cc): ([^>]*>?)/i) {\n>>\n>> I think this is acceptable, but this doesn't work with trailers like\n>>\n>> Cc: \"Some > Body\" \n>>\n>> A proper management of this kind of weird address should be doable by\n>> reusing the regexp parsing \"...\" in parse_mailbox:\n>>\n>> \tmy $re_quote = qr/\"(?:[^\\\"\\\\]|\\\\.)*\"/;\n>>\n>> So the final regex would look like\n>>\n>> if (/^(Signed-off-by|Cc): (([^>]*|\"(?:[^\\\"\\\\]|\\\\.)*\")>?)/i) {\n>>\n>> I don't think that should block the patch inclusion, but it may be worth\n>> considering.\n>>\n>> Anyway, thanks for the patch!\n>\n> Somehow this fell off the radar. So your reviewed-by: and then\n> we'll cook this in 'next' for a while?\n\nOK.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}