threads / patch / 27948

patchgit-send-email: Add auto-cc to all body signatures

Subject: [PATCH] git-send-email: Add auto-cc to all body signatures

## tl;dr

10 messages between Jul 29, 2011 and Dec 2, 2015. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Joe Perches· Jul 29, 2011, 01:34 UTC · lore
Many types of signatures are used by various projects.
The most common type is formatted:
	"[some_signature_type]-by: First Last <email@domain.tld>"
e.g:
	"Reported-by: First Last <email@domain.tld>" (no quotes are used)
Make git-send-email use these signatures as "CC:" entries.

Add command line option --suppress-cc=signatures to avoid adding these entries to the cc.

Signed-off-by: Joe Perches <joe@perches.com>
---
 Documentation/git-send-email.txt |    3 ++-
 git-send-email.perl              |   11 ++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)
Show changes to 2 files +8 −6

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 327233c..17ea825 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -246,8 +246,9 @@ Automating
   patch body (commit message) except for self (use 'self' for that).
 - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
    for self (use 'self' for that).
+- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
 - 'cccmd' will avoid running the --cc-cmd.
-- 'body' is equivalent to 'sob' + 'bodycc'
+- 'body' is equivalent to 'sob' + 'bodycc + signatures'
 - 'all' will suppress all auto cc values.
 --
 +
diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..973ed65 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,7 +75,7 @@ git send-email [options] <file | directory | rev-list options >
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, signatures, all.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
     --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default off.
@@ -374,13 +374,13 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
+			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|signatures)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -391,7 +391,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
 $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 if ($suppress_cc{'body'}) {
-	foreach my $entry (qw (sob bodycc)) {
+	foreach my $entry (qw (sob bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'body'};
@@ -1251,7 +1251,7 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<$fh>) {
 		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
+		if (/^(Signed-off-by|Cc|[a-z_-]+by): (.*)$/i) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
@@ -1260,6 +1260,7 @@ foreach my $t (@files) {
 			} else {
 				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
 				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'signatures'} and $what =~ /by$/i;
 			}
 			push @cc, $c;
 			printf("(body) Adding cc: %s from line '%s'\n",
-- 
1.7.6.131.g99019
Jeff Kirsher· Jul 29, 2011, 01:43 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add auto-cc to all body signatures

On Thu, 2011-07-28 at 18:34 -0700, Joe Perches wrote:
Show 14 quoted lines
> Many types of signatures are used by various projects.
> 
> The most common type is formatted:
>         "[some_signature_type]-by: First Last <email@domain.tld>"
> e.g:
>         "Reported-by: First Last <email@domain.tld>" (no quotes are
> used)
> 
> Make git-send-email use these signatures as "CC:" entries.
> 
> Add command line option --suppress-cc=signatures to avoid
> adding these entries to the cc.
> 
> Signed-off-by: Joe Perches <joe@perches.com> 
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Thanks, I was getting tired of having to add CC: <blah> especially since we already had Reported-by: <blah> and Tested-by: <blah>, yet they were never copied on the patch.

Joe Perches· Dec 8, 2011, 02:58 UTC · re: Jeff Kirsher · lore
Many types of signatures are used by various projects.
The most common type is formatted:
	"[some_signature_type]-by: First Last <email@domain.tld>"
e.g:
	"Reported-by: First Last <email@domain.tld>" (no quotes are used)
Make git-send-email use these signatures as "CC:" entries.

Add command line option --suppress-cc=signatures to avoid adding these entries to the cc.

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 Documentation/git-send-email.txt |    3 ++-
 git-send-email.perl              |   11 ++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)
Show changes to 2 files +8 −6

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 327233c..17ea825 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -246,8 +246,9 @@ Automating
   patch body (commit message) except for self (use 'self' for that).
 - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
    for self (use 'self' for that).
+- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
 - 'cccmd' will avoid running the --cc-cmd.
-- 'body' is equivalent to 'sob' + 'bodycc'
+- 'body' is equivalent to 'sob' + 'bodycc + signatures'
 - 'all' will suppress all auto cc values.
 --
 +
diff --git a/git-send-email.perl b/git-send-email.perl
index d491db9..fc5bf41 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,7 +75,7 @@ git send-email [options] <file | directory | rev-list options >
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, signatures, all.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
     --[no-]suppress-from           * Send to self. Default off.
     --[no-]chain-reply-to          * Chain In-Reply-To: fields. Default off.
@@ -393,13 +393,13 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
+			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|signatures)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -410,7 +410,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
 $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 if ($suppress_cc{'body'}) {
-	foreach my $entry (qw (sob bodycc)) {
+	foreach my $entry (qw (sob bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'body'};
@@ -1276,7 +1276,7 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<$fh>) {
 		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
+		if (/^(Signed-off-by|Cc|[a-z_-]+by): (.*)$/i) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
@@ -1285,6 +1285,7 @@ foreach my $t (@files) {
 			} else {
 				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
 				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'signatures'} and $what =~ /by$/i;
 			}
 			push @cc, $c;
 			printf("(body) Adding cc: %s from line '%s'\n",
-- 
1.7.8.dirty
Junio C Hamano· Dec 8, 2011, 19:37 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add auto-cc to all body signatures

Joe Perches <joe@perches.com> writes:
Show 6 quoted lines
> Many types of signatures are used by various projects.
>
> The most common type is formatted:
> 	"[some_signature_type]-by: First Last <email@domain.tld>"
> e.g:
> 	"Reported-by: First Last <email@domain.tld>" (no quotes are used)

This is just a phrasing issue, but I am a bit reluctant about the name "signature". "Acked-by:", "Tested-by:" and "Reviewed-by:" are originally written in the message sent to the author by the person who is giving an Ack, a successful test report, and a review comment, and the author (at least in spirit) copies & pastes them to the final text used in the commit log message, so it would not be incorrect to call them "signatures". But other "Random-by:" would not fall into that pattern.

"Reported-by:" certainly does not. It is almost always added by the author of the patch that is different from the reporter, and the reporter wouldn't have written "Reported-by: me" in the original bug report that triggered the discussion and resulted in the commit to fix the bug. Such a line is certainly not a signature of/by the reporter. Same can be said for "Helped-by:" for the author to share credits.

Also I've seen these "Random-by:", especially the ones that the author adds on his own initiative like "Reported-by:", followed by just a name but not an addresses [*1*].

Does your change do the right thing on such an address-less entry? The answer to this question must start with the definition of "the right thing to do is X", of course.

Show 12 quoted lines
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 327233c..17ea825 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -246,8 +246,9 @@ Automating
>    patch body (commit message) except for self (use 'self' for that).
>  - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
>     for self (use 'self' for that).
> +- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
>  - 'cccmd' will avoid running the --cc-cmd.
> -- 'body' is equivalent to 'sob' + 'bodycc'
> +- 'body' is equivalent to 'sob' + 'bodycc + signatures'
The quotes do not match quite well.
[Footnote]

*1* This seems to be done deliberately so; I understand that this is to avoid running afoul of EU privacy legislation or something.

Joe Perches· Dec 8, 2011, 20:51 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-send-email: Add auto-cc to all body signatures

On Thu, 2011-12-08 at 11:37 -0800, Junio C Hamano wrote:
Show 8 quoted lines
> Joe Perches <joe@perches.com> writes:
> > Many types of signatures are used by various projects.
> > The most common type is formatted:
> > 	"[some_signature_type]-by: First Last <email@domain.tld>"
> > e.g:
> > 	"Reported-by: First Last <email@domain.tld>" (no quotes are used)
> This is just a phrasing issue, but I am a bit reluctant about the name
> "signature".

I've called all these markings signatures. Maybe email-address-tags or another name could be used. I'm not bothered one way or another by any chosen name.

> Does your change do the right thing on such an address-less entry?  The
> answer to this question must start with the definition of "the right thing
> to do is X", of course.

All addresses go through "extract_valid_address". Invalid addresses are not used.

Rasmus Villemoes· Dec 2, 2015, 10:04 UTC · re: Joe Perches · lore

Re: [PATCH] git-send-email: Add auto-cc to all body signatures

On Thu, Dec 08 2011, Joe Perches <joe@perches.com> wrote:
Show 13 quoted lines
> On Thu, 2011-12-08 at 11:37 -0800, Junio C Hamano wrote:
>> Joe Perches <joe@perches.com> writes:
>> > Many types of signatures are used by various projects.
>> > The most common type is formatted:
>> > 	"[some_signature_type]-by: First Last <email@domain.tld>"
>> > e.g:
>> > 	"Reported-by: First Last <email@domain.tld>" (no quotes are used)
>> This is just a phrasing issue, but I am a bit reluctant about the name
>> "signature".
>
> I've called all these markings signatures.
> Maybe email-address-tags or another name could be used.
> I'm not bothered one way or another by any chosen name.

It's been four years, but I recently ran into this. I mistakenly thought that git would actually pick up cc addresses also from Reported-by, so the reporter ended up not being cc'ed. Is there any chance this could be revisited, or should I use a --cc-cmd to do what I want?

Rasmus
Joe Perches· Dec 2, 2015, 17:00 UTC · re: Rasmus Villemoes · lore

[PATCH V3] git-send-email: Add auto-cc to all body signatures

Many types of signatures are used by various projects.
The most common type is formatted:
	"[some_signature_type]-by: First Last <email <at> domain.tld>"
e.g:
	"Reported-by: First Last <email <at> domain.tld>" (no quotes are used)
Make git-send-email use these signatures as "CC:" entries.

Add command line option --suppress-cc=signatures to avoid adding these entries to the cc.

Signed-off-by: Joe Perches <joe <at> perches.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher <at> intel.com>
---
> It's been four years, but I recently ran into this. I mistakenly thought
> that git would actually pick up cc addresses also from Reported-by, so
> the reporter ended up not being cc'ed. Is there any chance this could be
> revisited,
Here's a refresh if desired.  I still think it's sensible.
 Documentation/git-send-email.txt |  3 ++-
 git-send-email.perl              | 11 ++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)
Show changes to 2 files +8 −6

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b9134d2..0866ae2 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -306,8 +306,9 @@ Automating
   patch body (commit message) except for self (use 'self' for that).
 - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
    for self (use 'self' for that).
+- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
 - 'cccmd' will avoid running the --cc-cmd.
-- 'body' is equivalent to 'sob' + 'bodycc'
+- 'body' is equivalent to 'sob' + 'bodycc' + 'signatures'
 - 'all' will suppress all auto cc values.
 --
 +
diff --git a/git-send-email.perl b/git-send-email.perl
index e907e0ea..536e264 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -83,7 +83,7 @@ git send-email [options] <file | directory | rev-list options >
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
     --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
-    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, all.
+    --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, signatures, all.
     --[no-]cc-cover                * Email Cc: addresses in the cover letter.
     --[no-]to-cover                * Email To: addresses in the cover letter.
     --[no-]signed-off-by-cc        * Send to Signed-off-by: addresses. Default on.
@@ -421,13 +421,13 @@ my(%suppress_cc);
 if (@suppress_cc) {
 	foreach my $entry (@suppress_cc) {
 		die "Unknown --suppress-cc field: '$entry'\n"
-			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
+			unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|signatures)$/;
 		$suppress_cc{$entry} = 1;
 	}
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -438,7 +438,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from;
 $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc;
 
 if ($suppress_cc{'body'}) {
-	foreach my $entry (qw (sob bodycc)) {
+	foreach my $entry (qw (sob bodycc signatures)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'body'};
@@ -1516,7 +1516,7 @@ foreach my $t (@files) {
 	# Now parse the message body
 	while(<$fh>) {
 		$message .=  $_;
-		if (/^(Signed-off-by|Cc): (.*)$/i) {
+		if (/^(Signed-off-by|Cc|[^\s]+[_-]by): (.*)$/i) {
 			chomp;
 			my ($what, $c) = ($1, $2);
 			chomp $c;
@@ -1526,6 +1526,7 @@ foreach my $t (@files) {
 			} else {
 				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
 				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
+				next if $suppress_cc{'signatures'} and $what =~ /by$/i;
 			}
 			push @cc, $c;
 			printf("(body) Adding cc: %s from line '%s'\n",
Junio C Hamano· Dec 2, 2015, 17:58 UTC · re: Joe Perches · lore

Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures

Joe Perches <joe@perches.com> writes:
Show 14 quoted lines
> Many types of signatures are used by various projects.
>
> The most common type is formatted:
> 	"[some_signature_type]-by: First Last <email <at> domain.tld>"
> e.g:
> 	"Reported-by: First Last <email <at> domain.tld>" (no quotes are used)
>
> Make git-send-email use these signatures as "CC:" entries.
>
> Add command line option --suppress-cc=signatures to avoid
> adding these entries to the cc.
>
> Signed-off-by: Joe Perches <joe <at> perches.com>
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher <at> intel.com>

I wonder what send-email with this patch does to the above two lines with "<at>" not "@" ;-) How was this patch sent?

In any case, did you mean "Helped-by:" not "Acked-by:"? "git shortlog git-send-email.perl" does not show that name as one of the major stakeholders who would be capable of giving an Ack on it.

Show 7 quoted lines
> ---
>> It's been four years, but I recently ran into this. I mistakenly thought
>> that git would actually pick up cc addresses also from Reported-by, so
>> the reporter ended up not being cc'ed. Is there any chance this could be
>> revisited,
>
> Here's a refresh if desired.  I still think it's sensible.

What the patch tries to achieve may make a lot of sense. I however do not necessarily think this particular implementation does, unfortunately.

These "Random-by:", especially the ones that the author adds on his own initiative like "Reported-by:", are often followed by just a name but not an addresses. A "Signed-off-by:" and "Cc:" that is not followed by a valid e-mail address may deserve to get an error (or perhaps an end-user interaction "This is not a valid address. What do you want to do about it?") so "/^(Signed-off-by|Cc): (.*)$/i" does not need its own sanity check on $2, because a later call to extract-valid-address or extract-valid-address-or-die will take care of it.

It would however be wrong to cause the program to error out or even bother the user upon seeing such random trailer lines that the author did not mean to have an e-mail address on it in the first place. If you have a trailer line

    Random-by: Joe Perches

without an address, I suspect you will end up adding "Joe" and "Perches" as two addresses on the Cc: line, which is most likely not what the user intended [*1*].

As to the lingo, these are still not signatures, but during the past years, it seems that we settled on using the term "trailers" for these e-mail header-like things at the end of the log message. "Trailers" are not limited to "*-by:" so this patch is not about adding auto-cc to all trailers--a retitle would be

    send-email: add auto-cc to addresses that appear on *-by: trailers

or something (and the option and variable names may need to be updated to match).

[Footnote]

*1* I further suspect that the existing code shares a similar issue. Don't Cc: and Signed-off-by: expect a single address on each line in the usual fashion? Perhaps a two-patch series whose first part does

- if (/^(Signed-off-by|Cc): (.*)$/i) { + if (/^(Signed-off-by|Cc): (.*<[^>]*>)\s*$/i) {

to tighten it (so that "Cc: Joe Perches" would not result in two pieces of mail sent to Joe and Perches), with your patch as a follow up, may be a good way forward.

I dunno.
Joe Perches· Dec 2, 2015, 18:20 UTC · re: Junio C Hamano · lore

Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures

On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote:
Show 19 quoted lines
> Joe Perches <joe@perches.com> writes:
> 
> > Many types of signatures are used by various projects.
> > 
> > The most common type is formatted:
> > 	"[some_signature_type]-by: First Last <email <at> domain.tld>"
> > e.g:
> > 	"Reported-by: First Last <email <at> domain.tld>" (no quotes are used)
> > 
> > Make git-send-email use these signatures as "CC:" entries.
> > 
> > Add command line option --suppress-cc=signatures to avoid
> > adding these entries to the cc.
> > 
> > Signed-off-by: Joe Perches  perches.com>
> > Acked-by: Jeff Kirsher  intel.com>
> 
> I wonder what send-email with this patch does to the above two lines
> with "" not "@" ;-)  How was this patch sent?
gnome evolution v3.18.2 email client.

And it seems all newer versions of evolution beyond 3.12 are really, really poor at sending inline patches. <grumble>

I'll update and resend using git-send-email eventually
> In any case, did you mean "Helped-by:" not "Acked-by:"?  "git
> shortlog git-send-email.perl" does not show that name as one of the
> major stakeholders who would be capable of giving an Ack on it.

At least for linux-kernel, "Acked-by:" doesn't mean a maintainer or a contributor to a particular module/file, just someone that has looked at the patch, tried it, and approved of the concept.

I don't know what process git uses for approval/signatures.
Show 21 quoted lines
> > ---
> > > It's been four years, but I recently ran into this. I mistakenly thought
> > > that git would actually pick up cc addresses also from Reported-by, so
> > > the reporter ended up not being cc'ed. Is there any chance this could be
> > > revisited,
> > 
> > Here's a refresh if desired.  I still think it's sensible.
> 
> What the patch tries to achieve may make a lot of sense.  I however
> do not necessarily think this particular implementation does,
> unfortunately.
> 
> These "Random-by:", especially the ones that the author adds on his
> own initiative like "Reported-by:", are often followed by just a
> name but not an addresses.  A "Signed-off-by:" and "Cc:" that is not
> followed by a valid e-mail address may deserve to get an error (or
> perhaps an end-user interaction "This is not a valid address. What
> do you want to do about it?") so "/^(Signed-off-by|Cc): (.*)$/i"
> does not need its own sanity check on $2, because a later call to
> extract-valid-address or extract-valid-address-or-die will take care
> of it.
Show 10 quoted lines
> It would however be wrong to cause the program to error out or even
> bother the user upon seeing such random trailer lines that the
> author did not mean to have an e-mail address on it in the first
> place.  If you have a trailer line
> 
>     Random-by: Joe Perches
> 
> without an address, I suspect you will end up adding "Joe" and
> "Perches" as two addresses on the Cc: line, which is most likely not
> what the user intended [*1*].

At least with new versions of git-send-email.perl that's true so the patch will need to validate that there is an email address following.

Show 26 quoted lines
> As to the lingo, these are still not signatures, but during the past
> years, it seems that we settled on using the term "trailers" for
> these e-mail header-like things at the end of the log message.
> "Trailers" are not limited to "*-by:" so this patch is not about
> adding auto-cc to all trailers--a retitle would be
> 
>     send-email: add auto-cc to addresses that appear on *-by: trailers
> 
> or something (and the option and variable names may need to be
> updated to match).
> 
> 
> [Footnote]
> 
> *1* I further suspect that the existing code shares a similar issue.
> Don't Cc: and Signed-off-by: expect a single address on each line in
> the usual fashion?  Perhaps a two-patch series whose first part does
> 
> -		if (/^(Signed-off-by|Cc): (.*)$/i) {
> +		if (/^(Signed-off-by|Cc): (.*<[^>]*>)\s*$/i) {
> 
> to tighten it (so that "Cc: Joe Perches" would not result in two
> pieces of mail sent to Joe and Perches), with your patch as a follow
> up, may be a good way forward.
> 
> I dunno.

I believe the old git-send-email code required addresses and validated the form after Signed-off-by:'s.

I haven't looked at the code for several years and just refreshed it without much thinking or testing.

I'll do a bit more and resend.
Joe Perches· Dec 2, 2015, 18:28 UTC · re: Junio C Hamano · lore

Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures

On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote:
> "Trailers" are not limited to "*-by:"
btw:  what are "Trailers" limited by?

← back to recent threads