Re: [PATCH] git-send-email: Generalize auto-cc recipient mechanism.

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 25, 2007, 21:04 UTC
Message-ID: <7vk5n2o58p.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1198532163-25308-1-git-send-email-git@davidb.org>

David Brown <git@davidb.org> writes:

Show 13 quoted lines

> ...
>   self   - patch sender.  Same as --suppress-from.
>   author - patch author.
>   cc     - cc lines mentioned in the patch.
>   cccmd  - avoid running the cccmd.
>   sob    - signed off by lines.
>   all    - all non-explicit recipients
>
> Signed-off-by: David Brown <git@davidb.org>
> ...
> What bothers me most about this change is that --signed-of-cc
> and --suppress-from are silently ignored if --suppress-cc is given, either
> on the command line, or in the config.

The order in which various variables are set in the current code before your patch is like this:

 * my ($var) introduces them -- they are undefined at the
   beginning;

 * GetOptions() may set them to explicit values;

 * read_config(), first for the specific sendemail identity and
   then for the generic ones, fill the ones that are still
   undefined;

 * the built-in default from %config_bool_settings are used to
   fill the ones that are still undefined at this point;

Now, I think you can build on top of the above by adding the following after that sequence:

 * fill %suppress_cc with explicit @suppress_cc GetOptions and
   read_config() read;

 * if the --suppress-from and/or --signed-off-by-cc, either from
   GetOptions() or from read_config() are given, make them
   override what @suppress_cc says.  So giving --suppress-cc=all
   and --signed-off-by-cc at the same time will still send cc to
   people who signed off the patch (because these old-style ones
   are more specific).

Perhaps something like this (untested, of course!) patch on top of yours.

 git-send-email.perl |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 1f03d12..cde5ffb 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -188,8 +188,8 @@ my (@suppress_cc);
 my %config_bool_settings = (
     "thread" => [\$thread, 1],
     "chainreplyto" => [\$chain_reply_to, 1],
-    "suppressfrom" => [\$suppress_from, 0],
-    "signedoffcc" => [\$signed_off_cc, 1],
+    "suppressfrom" => [\$suppress_from, undef],
+    "signedoffcc" => [\$signed_off_cc, undef],
     "smtpssl" => [\$smtp_ssl, 0],
 );
 
@@ -279,18 +279,20 @@ if (@suppress_cc) {
 			unless $entry =~ /^(all|cccmd|cc|author|self|sob)$/;
 		$suppress_cc{$entry} = 1;
 	}
-} else {
-	# Convert the old-style options.
-	$suppress_cc{'self'} = 1 if $suppress_from;
-	$suppress_cc{'sob'} = 1 unless $signed_off_cc;
 }
-
 if ($suppress_cc{'all'}) {
 	foreach my $entry (qw (ccmd cc author self sob)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
 }
+# If explicit old-style ones are specified, they trump supress-cc
+if (defined $suppress_from) {
+	$suppress_cc{'self'} = $suppress_from;
+}
+if (defined $signed_off_cc) {
+	$suppress_cc{'sob'} = !$signed_off_cc;
+}
 
 my ($repoauthor) = $repo->ident_person('author');
 my ($repocommitter) = $repo->ident_person('committer');