[PATCH] send-email: apply --suppress-from to S-o-b and cc-cmd

[PATCH] send-email: apply --suppress-from to S-o-b and cc-cmd

[Uwe Kleine-König]

From: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Cc: Ryan Anderson <ryan@michonline.com>
---
Hello,

I don't see the sense in adding the sender to Cc: from Signed-off-by
lines but not from From:.  If someone is convinced it makes sense, I'm
willing to send a new patch that uses a different option.

Cc'd Ryan as the author of --suppress-from.

I already tried to send this from my work's email account, but it seems
to have problems to send mails to vger (and lists.arm.linux.org.uk).
Sorry Ryan if you get this mail once more.

Best regards
Uwe

 Documentation/git-send-email.txt |    3 +--
 git-send-email.perl              |    5 +++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index e38b702..659215a 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -113,8 +113,7 @@ The --cc option must be repeated for each user you want on the cc list.
 	is not set, this will be prompted for.
 
 --suppress-from, --no-suppress-from::
-        If this is set, do not add the From: address to the cc: list, if it
-        shows up in a From: line.
+        If this is set, do not add the From: address to the cc: list.
         Default is the value of 'sendemail.suppressfrom' configuration value;
         if that is unspecified, default to --no-suppress-from.
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 96051bc..f4b8f96 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,8 +88,7 @@ Options:
 
    --smtp-ssl     If set, connects to the SMTP server using SSL.
 
-   --suppress-from Suppress sending emails to yourself if your address
-                  appears in a From: line. Defaults to off.
+   --suppress-from Suppress sending emails to yourself. Defaults to off.
 
    --thread       Specify that the "In-Reply-To:" header should be set on all
                   emails. Defaults to on.
@@ -730,6 +729,7 @@ foreach my $t (@files) {
 			if (/^(Signed-off-by|Cc): (.*)$/i && $signed_off_cc) {
 				my $c = $2;
 				chomp $c;
+				next if ($c eq $sender and $suppress_from);
 				push @cc, $c;
 				printf("(sob) Adding cc: %s from line '%s'\n",
 					$c, $_) unless $quiet;
@@ -745,6 +745,7 @@ foreach my $t (@files) {
 			my $c = $_;
 			$c =~ s/^\s*//g;
 			$c =~ s/\n$//g;
+			next if ($c eq $sender and $suppress_from);
 			push @cc, $c;
 			printf("(cc-cmd) Adding cc: %s from: '%s'\n",
 				$c, $cc_cmd) unless $quiet;
-- 
1.5.3.4

Re: [PATCH] send-email: apply --suppress-from to S-o-b and cc-cmd

[Junio C Hamano]

Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:

> From: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
>
> Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
> Cc: Ryan Anderson <ryan@michonline.com>
> ---
> Hello,
>
> I don't see the sense in adding the sender to Cc: from Signed-off-by
> lines but not from From:.  If someone is convinced it makes sense, I'm
> willing to send a new patch that uses a different option.

I _think_ --suppress-from just means "I know what I'll be
sending out, so do not bother my mailbox with a copy of this
message", so what the patch tries to do makes perfect sense to
me.

> @@ -730,6 +729,7 @@ foreach my $t (@files) {
>  			if (/^(Signed-off-by|Cc): (.*)$/i && $signed_off_cc) {
>  				my $c = $2;
>  				chomp $c;
> +				next if ($c eq $sender and $suppress_from);
>  				push @cc, $c;
>  				printf("(sob) Adding cc: %s from line '%s'\n",
>  					$c, $_) unless $quiet;
> @@ -745,6 +745,7 @@ foreach my $t (@files) {
>  			my $c = $_;
>  			$c =~ s/^\s*//g;
>  			$c =~ s/\n$//g;
> +			next if ($c eq $sender and $suppress_from);
>  			push @cc, $c;
>  			printf("(cc-cmd) Adding cc: %s from: '%s'\n",
>  				$c, $cc_cmd) unless $quiet;

By the way, I noticed that in the header part we pick CC: and
From: address, which are rfc2047 quoted, and unquote it to
compare with the sender.  If they are different, we push the
address, still rfc2047 quoted, to @cc, like this:

	} elsif (/^(Cc|From):\s+(.*)$/) {
		if (unquote_rfc2047($2) eq $sender) {
			next if ($suppress_from);
		}
		elsif ($1 eq 'From') {
			$author = unquote_rfc2047($2);
		}
		printf("(mbox) Adding cc: %s from line '%s'\n",
			$2, $_) unless $quiet;
		push @cc, $2;
	}

However, in the body part, when we see S-o-b: and CC: address,
which are _not_ rfc2047 quoted, do not unquote to compare with
$sender and we push it direct to @cc (the original text in the
first hunk of your patch).  We do the same for output from
$cc_cmd if specified (the second hunk).

This means that @cc list would be a mixed bag.  Some are rfc2047
quoted, and some are not.  This inconsistency of course is taken
care of by the call to sanitize_address over @cc at the very
beginning of send_message(), but it somehow feels dirty.