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

[Rasmus Villemoes <rv@rasmusvillemoes.dk>]

On 2018-10-16 07:57, Junio C Hamano wrote:
> Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:
> 
>>> 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'};> 		}
>>
>> ...yes, that's probably more readable.
> 
> OK, unless there is more comments and suggestions for improvements,
> can you send in a final version sometime not in so distant future so
> that we won't forget?

Will do, I was just waiting for more comments to come in.

 It may be surprising to existing users that
> the command now suddenly adds more addresses the user did not think
> would be added, but it would probably be easy enough for them to
> work around. 

Yeah, I thought about that, but unfortunately the whole auto-cc business
is not built around some config options where we can add a new and
default false. Also note that there are also cases currently where the
user sends off a patch series and is surprised that lots of intended
recipients were not cc'ed (that's how I picked this subject up again; I
had a long series where I had put specific Cc's in each patch, at v2,
some of those had given a Reviewed-by, so I changed the tags, and a
--dry-run told me they wouldn't get the new version).

I suppose one could make use of -by addresses dependent on a new opt-in
config option, but that's not very elegant. Another option is, for a
release or two, to make a little (more) noise when picking up a -by
address - something like setting a flag in the ($what =~ /-by/) branch,
and testing that flag somewhere in send_message(). But I suppose the
message printed when needs_confirm eq "inform" is generic enough.

Rasmus