threads / discuss / 18067

Changing the defaults for send-email / suppress-cc ?

Subject: Changing the defaults for send-email / suppress-cc ?

## tl;dr

10 messages between Feb 28, 2009 and Mar 1, 2009.

replies: 9people: 6as markdown or json

Paul Gortmaker· Feb 28, 2009, 19:29 UTC · lore

I've been involved in helping people who are new git users, and the one thing that seems to violate the principle of least surprise for them is the default setting for the sendemail.suppresscc -- in that the new users don't expect the additional CC lines to be automatically added based on what is present in the content of the mbox (format-patch output).

The messages from send-email that indicate it is going to add CC lines based on SOB etc. come *after* the last input from the user, and so they don't have an opportunity to jump in and prevent the extra e-mails from going out to whoever happens to be listed in the patch. (Lets assume for the moment, that they didn't see "--dry-run", or simply figured the process looked fairly straightforward, and didn't see the need for it.)

Here is the use case which I suspect is fairly pervasive, and that I've already seen several times:

1) User is working on something involving kernel version X, which is
some amount behind the current mainstream HEAD.  (Okay, doesn't have
to be kernel, could even be git itself.)
2) They've created a branch off of X and they've added their own
commits, and also cherry picked relevant commits from upstream that
happened between X and HEAD into their branch.   One of the features
they've cherry picked onto their branch is a 25 patch series that has
"Signed-off-by: miserable@bofh.com" in it, a miserable person who
hates extra-emails.
3) They run "git format-patch -n --thread -o foo X..mybranch"
4) They run "git send-email --to coworker@mycompany.com foo"  so their
buddy within the company can have an mbox patchset.
5) They recoil in horror while smashing ^C as they try to stop
send-email from spamming miserable@bofh.com with 25 of his own
patches.
In light of this, I've simply advised new users to run something like:
git config --global sendemail.suppresscc all
...just so that they won't accidentally do what I've described in the above.

Apologies if this has been discussed before; I took a quick scan of my archive and didn't see any discussions on it. With the recent thread about warning people of non-back compatible changes that will appear post 1.6.2 -- I thought perhaps this was a good time to mention/consider it.

I'm not sure what the right thing to do here is -- I suspect if you made suppress-cc=all the default, then there would be more experienced users that would complain about having to explicitly add a suppress-cc=self to get the old behaviour? Would that be acceptable? I don't know...

Thanks, Paul.

Junio C Hamano· Mar 1, 2009, 00:21 UTC · re: Paul Gortmaker · lore

Re: Changing the defaults for send-email / suppress-cc ?

Paul Gortmaker <paul.gortmaker@windriver.com> writes:
Show 5 quoted lines
> In light of this, I've simply advised new users to run something like:
>
> git config --global sendemail.suppresscc all
>
> ...just so that they won't accidentally do what I've described in the above.
> Apologies if this has been discussed before; I took a quick scan of my
> archive and didn't see any discussions on it.

I think I've seen discussions in the distant past but not very recently. IIRC some people had exactly the same reaction as you did, and were loud, while others were happy with the default, but weren't as loud.

But loudness depends on how annoyed they are, and unhappy people are louder than others, so we cannot judge much from the loudness alone.

> With the recent thread
> about warning people of non-back compatible changes that will appear
> post 1.6.2 -- I thought perhaps this was a good time to
> mention/consider it.

Unless you are a single project person, or have groups of hosts that share the home directory that is dedicated for working only on a single project, using $HOME/.gitconfig for this is a bad advice to give in general; as I think this really depends on what you are working on.

I do have "sendemail.suppressfrom" and "sendemail.signedoffcc = false" in my repository configuration, but not in global configuration.

My point is that the default does not suit *me* at least. But it may or may not match what you want as a default. I suspect many people are in the same boat as I am.

> I'm not sure what the right thing to do here is -- I suspect if you made
> suppress-cc=all the default, then there would be more experienced users
> that would complain about having to explicitly add a suppress-cc=self to
> get the old behaviour?  Would that be acceptable?

No matter what different default you choose, there will be people who complain about it as long as it is not exactly the default they want.

The only thing I could think of offhand that you could do safely to remedy the situation is to make the command fail until you explicitly set your default via the configuration, but even then, it is not entirely safe.

You will hear from angry people who use their own wrapper script to "git send-email" that gives all necessary options from the command line and now they are forced to add configuration to their repository, even though the config is never used because their script overrides it from the command line.

So, I dunno.
Paul Gortmaker· Mar 1, 2009, 01:32 UTC · re: Junio C Hamano · lore

Re: Changing the defaults for send-email / suppress-cc ?

On Sat, Feb 28, 2009 at 7:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
>
>> In light of this, I've simply advised new users to run something like:
>>
>> git config --global sendemail.suppresscc all
>>
>> ...just so that they won't accidentally do what I've described in the above.
>
>> Apologies if this has been discussed before; I took a quick scan of my
>> archive and didn't see any discussions on it.
>
> I think I've seen discussions in the distant past but not very recently.
> IIRC some people had exactly the same reaction as you did, and were loud,
> while others were happy with the default, but weren't as loud.
>
> But loudness depends on how annoyed they are, and unhappy people are
> louder than others, so we cannot judge much from the loudness alone.

Thanks for the reply -- I'm not interested in being "loud" ;-) But I think I'm in an interesting situation where I see people who are not necessarily wanting to use git, but are required to use git in order to do their daily activities. So they approach it with some apprehension and, given what human nature is, are looking for reasons to justify that apprehension.

Show 10 quoted lines
>
>> With the recent thread
>> about warning people of non-back compatible changes that will appear
>> post 1.6.2 -- I thought perhaps this was a good time to
>> mention/consider it.
>
> Unless you are a single project person, or have groups of hosts that share
> the home directory that is dedicated for working only on a single project,
> using $HOME/.gitconfig for this is a bad advice to give in general; as I
> think this really depends on what you are working on.

Yes, well it is the right solution for the new users I'm working with, since I know what their focus is, but that wasn't really where I wanted the discussion to focus.

My point was that I've seen these new users get scared off by having git send-email essentially embarrass them, and that it would be a shame to have people in general get the wrong impression about git from this one small setting, when it is such a useful tool. Really, it does tend to violate the principle of least surprise for the new user. If there is a new user who is a bit apprehensive about git, the defaults for "suppress-cc" can give them all the evidence they need to justify their fear of it, and then the FUD starts. So, I think this is a case where "do nothing, unless explicitly asked" is perhaps the wise choice. That was the only thing I wanted to draw attention to.

Thanks, Paul.

Jay Soffian· Mar 1, 2009, 03:53 UTC · re: Paul Gortmaker · lore

[PATCH] send-email: confirm auto cc before sending

send-email violates the principle of least surprise by automatically cc'ing additional recipients without even bothering to confirm this with the user first.

This patch teaches send-email a new option --confirm-cc. In the case where send-email has automatically added additional Cc recipients, it will confirm the recipients with the user before sending.

It defaults to true unless another of the cc-related suppression options has been specified, in which case it defaults to false. --- Untested patch, just soliciting ideas. I actually don't like this patch as is. I think a more general purpose --confirm option that takes multiple values is in order:

 --confirm=never   never confirms
 --confirm=cc      confirms only when send-email has automagically added
                   additional recipient
 --confirm=always  always confirms

The default would be --confirm=cc unless the user has specified any of the other Cc suppression options, which to me indicates the user knows what they are doing.

j.
 git-send-email.perl |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index adf7ecb..d3e718e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,6 +75,7 @@ git send-email [options] <file | directory | rev-list options >
     --[no-]thread                  * Use In-Reply-To: field. Default on.
 
   Administering:
+    --confirm-cc                   * Confirm automatic cc'd before sending.
     --quiet                        * Output one line of info per email.
     --dry-run                      * Don't actually send the emails.
     --[no-]validate                * Perform patch sanity checks. Default on.
@@ -181,7 +182,7 @@ sub do_edit {
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
 my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
-my ($validate);
+my ($validate, $confirm_cc);
 my (@suppress_cc);
 
 my %config_bool_settings = (
@@ -190,6 +191,7 @@ my %config_bool_settings = (
     "suppressfrom" => [\$suppress_from, undef],
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
+    "confirmcc"   => [\$confirm_cc, undef],
     "validate" => [\$validate, 1],
 );
 
@@ -258,6 +260,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "suppress-from!" => \$suppress_from,
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
+		    "confirm-cc" => \$confirm_cc,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
 		    "thread!" => \$thread,
@@ -346,6 +349,12 @@ if ($suppress_cc{'body'}) {
 	delete $suppress_cc{'body'};
 }
 
+if (!defined $confirm_cc) {
+    # defaults to true unless user has specified any of the other cc options
+    $confirm_cc = scalar %suppress_cc ? 0 : 1;
+}
+
+
 # Debugging, print out the suppressions.
 if (0) {
 	print "suppressions:\n";
@@ -943,7 +952,7 @@ foreach my $t (@files) {
 	my $author_encoding;
 	my $has_content_type;
 	my $body_encoding;
-	@cc = @initial_cc;
+	@cc = ();
 	@xh = ();
 	my $input_format = undef;
 	my @header = ();
@@ -1080,6 +1089,22 @@ foreach my $t (@files) {
 		}
 	}
 
+	if (@cc and $confirm_cc) {
+		print "Automatically cc'ing:\n";
+		print "	  $_\n" for each @cc;
+		print "Okay? "
+		while (1) {
+			$_ = $term->readline("Okay (y/n)?");
+			last if defined $_;
+			print "\n";
+		}
+		if (/n/i) {
+			$message_id = undef;
+			continue;
+		}
+	}
+	push(@cc, @initial_cc);
+
 	send_message();
 
 	# set up for the next message

-- 
1.6.2.rc1.309.g5f417
Sverre Rabbelier· Mar 1, 2009, 18:09 UTC · re: Jay Soffian · lore

Re: [PATCH] send-email: confirm auto cc before sending

Heya,
On Sun, Mar 1, 2009 at 04:53, Jay Soffian <jaysoffian@gmail.com> wrote:
> Untested patch, just soliciting ideas.

With a config option to turn it on permanently this would be very nice to have. It personally annoys me that it decides who to cc (I already do that manually, thank you very much). So, FWIW, I like this idea :).

-- 
Cheers,

Sverre Rabbelier
Nanako Shiraishi· Mar 1, 2009, 06:30 UTC · re: Junio C Hamano · lore

Re: Changing the defaults for send-email / suppress-cc ?

Quoting Junio C Hamano <gitster@pobox.com>:
Show 14 quoted lines
> No matter what different default you choose, there will be people who
> complain about it as long as it is not exactly the default they want.
>
> The only thing I could think of offhand that you could do safely to remedy
> the situation is to make the command fail until you explicitly set your
> default via the configuration, but even then, it is not entirely safe.
>
> You will hear from angry people who use their own wrapper script to "git
> send-email" that gives all necessary options from the command line and now
> they are forced to add configuration to their repository, even though the
> config is never used because their script overrides it from the command
> line.
>
> So, I dunno.

Isn't it an option to introduce a new program, say 'git send', that reimplements what 'git send-email' is meant to be used for, but has a better implementation and a better default setting?

You have a precedence of doing so when you introduced 'git am'. You contined to maintain 'git applymbox' to keep old people happy. You taught new people to use 'git am', and waited until old people learn to like the new command with a shorter name and a saner behavior. After a certain period of time, you deprecated and removed the original program.

d1c5f2a42d7b5c0e3d3862212dea1f09809c4963 "Add git-am, applymbox replacement" http://article.gmane.org/gmane.comp.version-control.git/44376 http://article.gmane.org/gmane.comp.version-control.git/47785

I don't know much about the technical details, but I saw there were a few discussion threads about improving 'git send-email'; one of the recent ones was this.

http://article.gmane.org/gmane.comp.version-control.git/109791

I also saw the original author (Greg?) said that one of the input formats 'git send-email' supports is not worth supporting, or sth like that. A reimplemention to introduce 'git send' may be a good time to deprecate and remove that old input format.

Or is the migration plan like the one you used when you migrated 'git applymbox' to 'git am' considered unacceptable by now, because git is a lot more mature than it used to be back then?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
Jay Soffian· Mar 1, 2009, 06:59 UTC · re: Nanako Shiraishi · lore

Re: Changing the defaults for send-email / suppress-cc ?

On Sun, Mar 1, 2009 at 1:30 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Isn't it an option to introduce a new program, say 'git send', that
> reimplements what 'git send-email' is meant to be used for, but has a
> better implementation and a better default setting?

I would, at some point, like to unify format-patch, send-email, and possible imap-send. I envision being able to "git format-patch --email" (and possibly "git format-patch --imap").

I don't have any illusion that writing my own SMTP client in C is much fun, so I was going to see if it was reasonable to link format-patch against msmtp. OTOH, I don't want to bloat the git binary, so it may just be best to have format-patch be able to run msmtp automagically when it is called with "--email".

Or something. I haven't thought real hard about it yet. :-)
j.
Jay Soffian· Mar 1, 2009, 07:01 UTC · re: Jay Soffian · lore

Re: Changing the defaults for send-email / suppress-cc ?

On Sun, Mar 1, 2009 at 1:59 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
Show 7 quoted lines
> I don't have any illusion that writing my own SMTP client in C is much
> fun, so I was going to see if it was reasonable to link format-patch
> against msmtp. OTOH, I don't want to bloat the git binary, so it may
> just be best to have format-patch be able to run msmtp automagically
> when it is called with "--email".
>
> Or something. I haven't thought real hard about it yet. :-)

OTOH, I like having format-patch spit out the patches so I can give them a final look-over before sending them. So I dunno if it is worth this. Perhaps format-patch should be able to invoke an SMTP server automatically, in the same way that send-email can, but then again, perhaps not.

Maybe I should stop this late night thinking out loud before I annoy one too many maintainers. :-)

j.
Jay Soffian· Mar 1, 2009, 07:05 UTC · re: Jay Soffian · lore

Re: Changing the defaults for send-email / suppress-cc ?

On Sun, Mar 1, 2009 at 1:59 AM, Jay Soffian <jaysoffian@gmail.com> wrote:
> fun, so I was going to see if it was reasonable to link format-patch
> against msmtp. OTOH, I don't want to bloat the git binary, so it may
And msmtp is GPLv3, so nevermind anyway.
j.
Teemu Likonen· Mar 1, 2009, 05:49 UTC · re: Paul Gortmaker · lore

Re: Changing the defaults for send-email / suppress-cc ?

On 2009-02-28 14:29 (-0500), Paul Gortmaker wrote:
Show 6 quoted lines
> I've been involved in helping people who are new git users, and the
> one thing that seems to violate the principle of least surprise for
> them is the default setting for the sendemail.suppresscc -- in that
> the new users don't expect the additional CC lines to be automatically
> added based on what is present in the content of the mbox
> (format-patch output).
Show 6 quoted lines
> In light of this, I've simply advised new users to run something like:
>
> git config --global sendemail.suppresscc all
>
> ...just so that they won't accidentally do what I've described in the
> above.

Agreed. In the past I have sent a couple of test patches to wrong places because of these automatic CCs. I think that send-email or any e-mail client should only send mail for addresses which user has explicitly selected or otherwise confirmed.

With a time machine I would make the opposite option for send-email: something like sendemail.autocc and --autocc=. Then user would configure different recipient categories who she wants to send automatic CCs. It's easier to think this way. But we don't have a time machine so at least some kind confirm message would be nice.

← back to recent threads