Re: [PATCH 3/3] send-email: also pick up cc addresses from -by trailers

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2018, 06:18 UTC
Message-ID: <xmqqd0shgg66.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181010111351.5045-4-rv@rasmusvillemoes.dk>

Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:

Show 6 quoted lines

> @@ -1681,7 +1681,7 @@ sub process_file {
>  	# Now parse the message body
>  	while(<$fh>) {
>  		$message .=  $_;
> -		if (/^(Signed-off-by|Cc): (.*)/i) {
> +		if (/^([a-z-]*-by|Cc): (.*)/i) {

So this picks up anything-by not just s-o-by, which sort of makes sense.

Show 5 quoted lines

> @@ -1691,7 +1691,9 @@ sub process_file {
>  			if ($sc eq $sender) {
>  				next if ($suppress_cc{'self'});
>  			} else {
> -				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;

We used to only grab CC or Signed-off-by (and specifically not something like "Not-Signed-off-by") upfront above, so matching /Signed-off-by/ was sufficient (it would have been sufficient to just look for 's'). But to suppress s-o-b and keep allowing via misc-by a trailer "Not-signed-off-by:", we now ...

> +				next if $suppress_cc{'sob'} and $what =~ /^Signed-off-by$/i;

... must make sure what we have is _exactly_ "signed-off-by" when 'sob' is suppressed. Makes sense.

> +				next if $suppress_cc{'misc-by'}
> +					and $what =~ /-by$/i and $what !~ /^Signed-off-by$/i;

And this is the opposite side of the same coin, which also makes sense.

I wonder if it would make it easier to grok if we made the logic inside out, i.e.

	if ($sc eq $sender) {
		...
	} else {
		if ($what =~ /^Signed-off-by$/i) {
			next if $suppress_cc{'sob'};
		} elsif ($what =~ /-by$/i) {
			next if $suppress_cc{'misc'};
		} elsif ($what =~ /^Cc$/i) {
			next if $suppress_cc{'bodycc'};
		}
		push @cc, $c;
		...
	}

>  				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
>  			}
>  			if ($c !~ /.+@.+|<.+>/) {