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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2007, 20:43 UTC
Message-ID: <7vode5aisb.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1194420852-15822-1-git-send-email-ukleinek@informatik.uni-freiburg.de>

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

Show 10 quoted lines

> 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.

Show 16 quoted lines

> @@ -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.