{"thread":{"id":"43962","subject":"[PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","startedAt":"2016-08-30T20:18:50Z","lastAt":"2016-08-31T18:12:36Z","messageCount":5,"participants":["Joe Perches","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"300602","messageId":"b2aa91d59a6cdd468bcbe85b45807cc1b82b23ed.1472588158.git.joe@perches.com","threadId":"43962","inReplyTo":null,"subject":"[PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-30T20:18:29Z","receivedAt":"2016-08-30T20:18:50Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"Many commits have various forms of trailers similar to\n \"Acked-by: Name
\" and \"Reported-by: Name
\"\n\nAdd the ability to cc these trailers when using git send-email.\n\nThis can be suppressed with --suppress-cc=trailers.\n\nSigned-off-by: Joe Perches \n---\n Documentation/git-send-email.txt | 10 ++++++----\n git-send-email.perl | 16 +++++++++++-----\n 2 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 642d0ef..999c842 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -278,9 +278,10 @@ Automating\n \tthe value of `sendemail.identity`.\n \n --[no-]signed-off-by-cc::\n-\tIf this is set, add emails found in Signed-off-by: or Cc: lines to the\n-\tcc list. Default is the value of `sendemail.signedoffbycc` configuration\n-\tvalue; if that is unspecified, default to --signed-off-by-cc.\n+\tIf this is set, add emails found in Signed-off-by: or Cc: or any other\n+\ttrailer -by: lines to the cc list. Default is the value of\n+\t`sendemail.signedoffbycc` configuration value; if that is unspecified,\n+\tdefault to --signed-off-by-cc.\n \n --[no-]cc-cover::\n \tIf this is set, emails found in Cc: headers in the first patch of\n@@ -307,8 +308,9 @@ Automating\n patch body (commit message) except for self (use 'self' for that).\n - 'sob' will avoid including anyone mentioned in Signed-off-by lines except\n for self (use 'self' for that).\n+- 'trailers' will avoid including anyone mentioned in any \"-by:\" lines.\n - 'cccmd' will avoid running the --cc-cmd.\n-- 'body' is equivalent to 'sob' + 'bodycc'\n+- 'body' is equivalent to 'sob' + 'bodycc' + 'trailers'\n - 'all' will suppress all auto cc values.\n --\n +\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex da81be4..255465a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -84,7 +84,7 @@ git send-email --dump-aliases\n --identity * Use the sendemail. options.\n --to-cmd * Email To: via ` \\$patch_path`\n --cc-cmd * Email Cc: via ` \\$patch_path`\n- --suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all.\n+ --suppress-cc * author, self, sob, cc, cccmd, body, bodycc, trailers, all.\n --[no-]cc-cover * Email Cc: addresses in the cover letter.\n --[no-]to-cover * Email To: addresses in the cover letter.\n --[no-]signed-off-by-cc * Send to Signed-off-by: addresses. Default on.\n@@ -431,13 +431,13 @@ my(%suppress_cc);\n if (@suppress_cc) {\n \tforeach my $entry (@suppress_cc) {\n \t\tdie \"Unknown --suppress-cc field: '$entry'\\n\"\n-\t\t\tunless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;\n+\t\t\tunless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|trailers)$/;\n \t\t$suppress_cc{$entry} = 1;\n \t}\n }\n \n if ($suppress_cc{'all'}) {\n-\tforeach my $entry (qw (cccmd cc author self sob body bodycc)) {\n+\tforeach my $entry (qw (cccmd cc author self sob body bodycc trailers)) {\n \t\t$suppress_cc{$entry} = 1;\n \t}\n \tdelete $suppress_cc{'all'};\n@@ -448,7 +448,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;\n $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n \n if ($suppress_cc{'body'}) {\n-\tforeach my $entry (qw (sob bodycc)) {\n+\tforeach my $entry (qw (sob bodycc trailers)) {\n \t\t$suppress_cc{$entry} = 1;\n \t}\n \tdelete $suppress_cc{'body'};\n@@ -1545,7 +1545,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|[^\\s]+[_-]by): (.*)$/i) {\n \t\t\tchomp;\n \t\t\tmy ($what, $c) = ($1, $2);\n \t\t\tchomp $c;\n@@ -1555,6 +1555,12 @@ foreach my $t (@files) {\n \t\t\t} else {\n \t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n \t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n+\t\t\t\tnext if $suppress_cc{'trailers'} and $what !~ /Signed-off-by/i && $what =~ /by$/i;\n+\t\t\t}\n+\t\t\tif ($c !~ /.+@.+/) {\n+\t\t\t\tprintf(\"(body) Ignoring %s from line '%s'\\n\",\n+\t\t\t\t $what, $_) unless $quiet;\n+\t\t\t\tnext;\n \t\t\t}\n \t\t\tpush @cc, $c;\n \t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n-- \n2.10.0.rc2.1.gb2aa91d\n\n"},{"id":"300702","messageId":"xmqqpooo259c.fsf@gitster.mtv.corp.google.com","threadId":"43962","inReplyTo":"b2aa91d59a6cdd468bcbe85b45807cc1b82b23ed.1472588158.git.joe@perches.com","subject":"Re: [PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-31T17:54:39Z","receivedAt":"2016-08-31T17:55:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> Many commits have various forms of trailers similar to\n> \"Acked-by: Name
\" and \"Reported-by: Name
\"\n>\n> Add the ability to cc these trailers when using git send-email.\n\nI thought you were asking what we call these \" followed by\n\" at the end of the log message, and \"footers or trailers\"\nwas the answer.\n\nI do not have a strong objection against limiting to \"-by:\" lines;\nfor one thing, it would automatically avoid having to worry about\n\"Bug-ID:\" and other trailers that won't have e-mail address at all.\n\nBut if you are _only_ picking up \"-by:\" lines, then calling this\noption \"trailers\" is way too wide and confusing. I do not think\nthere is any specific name for \"-by:\" lines, though. Perhaps you\nwould need to invent some name that has \"-by\" as a substring.\n\n\"any-by\"? or just \"by\"? I dunno.\n\n> if ($suppress_cc{'all'}) {\n> -\tforeach my $entry (qw (cccmd cc author self sob body bodycc)) {\n> +\tforeach my $entry (qw (cccmd cc author self sob body bodycc trailers)) {\n> \t\t$suppress_cc{$entry} = 1;\n> \t}\n\nOK.\n\n> @@ -448,7 +448,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;\n> $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;\n> \n> if ($suppress_cc{'body'}) {\n> -\tforeach my $entry (qw (sob bodycc)) {\n> +\tforeach my $entry (qw (sob bodycc trailers)) {\n> \t\t$suppress_cc{$entry} = 1;\n> \t}\n> \tdelete $suppress_cc{'body'};\n\nOK.\n\n> @@ -1545,7 +1545,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|[^\\s]+[_-]by): (.*)$/i) {\n\nMicronits:\n\n (1) do you really want to grab a run of any non-blanks? Don't\n you want to exclude at least a colon?\n (2) allowing an underscore looks a bit unusual. \n\n> @@ -1555,6 +1555,12 @@ foreach my $t (@files) {\n> \t\t\t} else {\n> \t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n> \t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n> +\t\t\t\tnext if $suppress_cc{'trailers'} and $what !~ /Signed-off-by/i && $what =~ /by$/i;\n> +\t\t\t}\n\nIt is a bit unfortunate that S-o-b is a subset of any-by that forces\nyou to do this.\n\n> +\t\t\tif ($c !~ /.+@.+/) {\n> +\t\t\t\tprintf(\"(body) Ignoring %s from line '%s'\\n\",\n> +\t\t\t\t $what, $_) unless $quiet;\n> +\t\t\t\tnext;\n> \t\t\t}\n\nThis check is new and applies to sob/cc, too.\n\nI am aware of the fact that people sometimes write only a name with\nno e-mail address when giving credit to a third-party and we want to\navoid upsetting the underlying MTA by feeding it a non-address.\n\nLooking at existing helper subs like extract_valid_address and\nsanitize_address that all addresses we pass to the MTA go through,\nit appears to me that we try to support an addr-spec with only\nlocal-part without @domain, so this new check might turn out to be\ntoo strict from that point of view, but on the other hand I suspect\nit won't be a huge issue because the addresses in the footers are\nfor public consumption and it may not make much sense to have a\nlocal-only address there. I dunno.\n\n> \t\t\tpush @cc, $c;\n> \t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n"},{"id":"300703","messageId":"1472666194.4176.17.camel@perches.com","threadId":"43962","inReplyTo":"xmqqpooo259c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-31T17:56:34Z","receivedAt":"2016-08-31T17:56:54Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Wed, 2016-08-31 at 10:54 -0700, Junio C Hamano wrote:\n> Joe Perches writes:\n> \n> > \n> > Many commits have various forms of trailers similar to\n> >      \"Acked-by: Name
\" and \"Reported-by: Name
\"\n> > \n> > Add the ability to cc these trailers when using git send-email.\n> I thought you were asking what we call these \" followed by\n> \" at the end of the log message, and \"footers or trailers\"\n> was the answer.\n> \n> I do not have a strong objection against limiting to \"-by:\" lines;\n> for one thing, it would automatically avoid having to worry about\n> \"Bug-ID:\" and other trailers that won't have e-mail address at all.\n> \n> But if you are _only_ picking up \"-by:\" lines, then calling this\n> option \"trailers\" is way too wide and confusing.  I do not think\n> there is any specific name for \"-by:\" lines, though.  Perhaps you\n> would need to invent some name that has \"-by\" as a substring.\n> \n> \"any-by\"?  or just \"by\"?  I dunno.\n\nSignatures?\n\n"},{"id":"300705","messageId":"xmqqlgzc24hq.fsf@gitster.mtv.corp.google.com","threadId":"43962","inReplyTo":"1472666194.4176.17.camel@perches.com","subject":"Re: [PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-31T18:11:13Z","receivedAt":"2016-08-31T18:11:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches writes:\n\n> On Wed, 2016-08-31 at 10:54 -0700, Junio C Hamano wrote:\n>> Joe Perches writes:\n>> \n>> > \n>> > Many commits have various forms of trailers similar to\n>> > \"Acked-by: Name
\" and \"Reported-by: Name
\"\n>> > \n>> > Add the ability to cc these trailers when using git send-email.\n>> I thought you were asking what we call these \" followed by\n>> \" at the end of the log message, and \"footers or trailers\"\n>> was the answer.\n>> \n>> I do not have a strong objection against limiting to \"-by:\" lines;\n>> for one thing, it would automatically avoid having to worry about\n>> \"Bug-ID:\" and other trailers that won't have e-mail address at all.\n>> \n>> But if you are _only_ picking up \"-by:\" lines, then calling this\n>> option \"trailers\" is way too wide and confusing. I do not think\n>> there is any specific name for \"-by:\" lines, though. Perhaps you\n>> would need to invent some name that has \"-by\" as a substring.\n>> \n>> \"any-by\"? or just \"by\"? I dunno.\n>\n> Signatures?\n\nHelped-by: and Reported-by: are often written by the author of the\npatch and not by those who helped or reported, so calling them signatures\nimply the author is forging them, too ;-)\n\nI dunno.\n\nAny name that hints that this applies only to the trailers that ends\nwith \"-by\" is fine but \"Signatures-by\" does not sound very\ngrammatical.\n"},{"id":"300706","messageId":"1472667123.4176.27.camel@perches.com","threadId":"43962","inReplyTo":"xmqqpooo259c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-send-email: Add ability to cc: any \"trailers\" from commit message","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2016-08-31T18:12:03Z","receivedAt":"2016-08-31T18:12:36Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Wed, 2016-08-31 at 10:54 -0700, Junio C Hamano wrote:\n> Joe Perches writes:\n> > \n> > Many commits have various forms of trailers similar to\n> >      \"Acked-by: Name \" and \"Reported-by: Name \"\n> > \n> > Add the ability to cc these trailers when using git send-email.\n> I thought you were asking what we call these \" followed by\n> \" at the end of the log message, and \"footers or trailers\"\n> was the answer.\n> \n> I do not have a strong objection against limiting to \"-by:\" lines;\n> for one thing, it would automatically avoid having to worry about\n> \"Bug-ID:\" and other trailers that won't have e-mail address at all.\n> \n> But if you are _only_ picking up \"-by:\" lines, then calling this\n> option \"trailers\" is way too wide and confusing.  I do not think\n> there is any specific name for \"-by:\" lines, though.  Perhaps you\n> would need to invent some name that has \"-by\" as a substring.\n> \n> \"any-by\"?  or just \"by\"?  I dunno.\n\nThinking about this a little, \"bylines\" seems much better.\n\n> >@@ -1545,7 +1545,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|[^\\s]+[_-]by): (.*)$/i) {\n> Micronits:\n> \n>  (1) do you really want to grab a run of any non-blanks?  Don't\n>      you want to exclude at least a colon?\n\nIt could use [\\w_-]+\n\n>  (2) allowing an underscore looks a bit unusual.  \n\nIt's for typos.  A relatively high percentage of\nthese things in at least the kernel were malformed\nwhen I started this 5 years ago.\n\nI don't have an objection to requiring the proper\nform using only dashes though.\n\nMaybe that'd help reduce the typo frequency anyway.\n\n> I am aware of the fact that people sometimes write only a name with\n> no e-mail address when giving credit to a third-party and we want to\n> avoid upsetting the underlying MTA by feeding it a non-address.\n> \n> Looking at existing helper subs like extract_valid_address and\n> sanitize_address that all addresses we pass to the MTA go through,\n> it appears to me that we try to support an addr-spec with only\n> local-part without @domain, so this new check might turn out to be\n> too strict from that point of view, but on the other hand I suspect\n> it won't be a huge issue because the addresses in the footers are\n> for public consumption and it may not make much sense to have a\n> local-only address there.  I dunno.\n> \n> > \n> >  \t\t\tpush @cc, $c;\n> >  \t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n\nme either but I think it doesn't hurt because\nas you suggest, these are supposed to be public.\n\nThanks for the review.\n\ncheers, Joe\n"}]}