send-email violates the principle of least surprise by automatically
cc'ing additional recipients without confirming this with the user.
This patch teaches send-email a --confirm option. It takes the
following values:
--confirm=always always confirm before sending
--confirm=never never confirm before sending
--confirm=cc confirm before sending when send-email has
automatically added addresses from the patch to
the Cc list
--confirm=compose confirm before sending the first message when
using --compose. (Needed to maintain backwards
compatibility with existing behavior.)
--confirm=auto 'cc' + 'compose'
The option defaults to 'compose' if any suppress Cc related options have
been used, otherwise it defaults to 'auto'.
Unfortunately, it is impossible to introduce this patch such that it
helps new users without potentially upsetting some existing users. We
attempt to mitigate the latter by:
* Allowing the user to "git config sendemail.config never"
* Allowing the user to say "all" after the first prompt to not be
prompted on remaining emails during the same invocation.
* Telling the user about the "sendemail.confirm" setting whenever they
use "all"
* Only prompting if no --suppress related options have been passed, as
using such an option is likely to indicate an experienced send-email
user.
Signed-off-by: Jay Soffian <jaysoffian@xxxxxxxxx>
---
Changes from v1:
- Added no-confirm tests, and put them early to prevent the rest of
the tests from potentially hanging. Note if one of these tests
fails then we exit t9001-send-email.sh immediately.
- Added verification of the --confirm setting. (I had done this
previously but somehow failed to stage it.)
- Added additional language to the commit messsage in an attempt to
provide justification for the change in default behavior.
- When the user specifies "all" in response to a confirm prompt, we
hint them about how to use "sendemail.confirm" config setting.
Documentation/git-send-email.txt | 16 ++++++
git-send-email.perl | 72 +++++++++++++++++--------
t/t9001-send-email.sh | 108 ++++++++++++++++++++++++++++++++------
3 files changed, 157 insertions(+), 39 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 164d149..bcf7ff1 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
Administering
~~~~~~~~~~~~~
+--confirm::
+ Confirm just before sending:
++
+--
+- 'always' will always confirm before sending
+- 'never' will never confirm before sending
+- 'cc' will confirm before sending when send-email has automatically
+ added addresses from the patch to the Cc list
+- 'compose' will confirm before sending the first message when using --compose.
+- 'auto' is equivalent to 'cc' + 'compose'
+--
++
+Default is the value of 'sendemail.confirm' configuration value; if that
+is unspecified, default to 'auto' unless any of the suppress options
+have been specified, in which case default to 'compose'.
+
--dry-run::
Do everything except actually send the emails.
diff --git a/git-send-email.perl b/git-send-email.perl
index adf7ecb..439b70b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,6 +75,8 @@ git send-email [options] <file | directory | rev-list options >
--[no-]thread * Use In-Reply-To: field. Default on.
Administering:
+ --confirm <str> * Confirm recipients before sending;
+ auto, cc, compose, always, or never.
--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 +183,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);
my (@suppress_cc);
my %config_bool_settings = (
@@ -207,6 +209,7 @@ my %config_settings = (
"suppresscc" => \@suppress_cc,
"envelopesender" => \$envelope_sender,
"multiedit" => \$multiedit,
+ "confirm" => \$confirm,
);
# Handle Uncouth Termination
@@ -258,6 +261,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=s" => \$confirm,
"dry-run" => \$dry_run,
"envelope-sender=s" => \$envelope_sender,
"thread!" => \$thread,
@@ -346,6 +350,13 @@ if ($suppress_cc{'body'}) {
delete $suppress_cc{'body'};
}
+# Set confirm
+if (!defined $confirm) {
+ $confirm = scalar %suppress_cc ? 'compose' : 'auto';
+};
+die "Unknown --confirm setting: '$confirm'\n"
+ unless $confirm =~ /^(?:auto|cc|compose|always|never)/;
+
# Debugging, print out the suppressions.
if (0) {
print "suppressions:\n";
@@ -663,25 +674,13 @@ if (!defined $smtp_server) {
$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
}
-if ($compose) {
- while (1) {
- $_ = $term->readline("Send this email? (y|n) ");
- last if defined $_;
- print "\n";
- }
-
- if (uc substr($_,0,1) ne 'Y') {
- cleanup_compose_files();
- exit(0);
- }
-
- if ($compose > 0) {
- @files = ($compose_filename . ".final", @files);
- }
+if ($compose && $compose > 0) {
+ @files = ($compose_filename . ".final", @files);
}
# Variables we set as part of the loop over files
-our ($message_id, %mail, $subject, $reply_to, $references, $message);
+our ($message_id, %mail, $subject, $reply_to, $references, $message,
+ $needs_confirm, $message_num);
sub extract_valid_address {
my $address = shift;
@@ -837,6 +836,27 @@ X-Mailer: git-send-email $gitversion
unshift (@sendmail_parameters,
'-f', $raw_from) if(defined $envelope_sender);
+ if ($needs_confirm && !$dry_run) {
+ print "\n$header\n";
+ while (1) {
+ chomp ($_ = $term->readline(
+ "Send this email? ([y]es|[n]o|[q]uit|[a]ll): "
+ ));
+ last if /^(?:yes|y|no|n|quit|q|all|a)/i;
+ print "\n";
+ }
+ if (/^n/i) {
+ return;
+ } elsif (/^q/i) {
+ cleanup_compose_files();
+ exit(0);
+ } elsif (/^a/i) {
+ $confirm = 'never';
+ print "You may disable all future prompting via sendemail.confirm;\n";
+ print "Run 'git send-email --help' for details.\n";
+ }
+ }
+
if ($dry_run) {
# We don't want to send the email.
} elsif ($smtp_server =~ m#^/#) {
@@ -935,6 +955,7 @@ X-Mailer: git-send-email $gitversion
$reply_to = $initial_reply_to;
$references = $initial_reply_to || '';
$subject = $initial_subject;
+$message_num = 0;
foreach my $t (@files) {
open(F,"<",$t) or die "can't open file $t";
@@ -943,11 +964,12 @@ 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 = ();
$message = "";
+ $message_num++;
# First unfold multiline header fields
while(<F>) {
last if /^\s*$/;
@@ -1080,6 +1102,13 @@ foreach my $t (@files) {
}
}
+ $needs_confirm = (
+ $confirm eq "always" or
+ ($confirm =~ /^(?:auto|cc)$/ && @cc) or
+ ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
+
+ @cc = (@initial_cc, @cc);
+
send_message();
# set up for the next message
@@ -1094,13 +1123,10 @@ foreach my $t (@files) {
$message_id = undef;
}
-if ($compose) {
- cleanup_compose_files();
-}
+cleanup_compose_files();
sub cleanup_compose_files() {
- unlink($compose_filename, $compose_filename . ".final");
-
+ unlink($compose_filename, $compose_filename . ".final") if $compose;
}
$smtp->quit if $smtp;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4df4f96..08d5b91 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -35,6 +35,47 @@ test_expect_success 'Extract patches' '
patches=`git format-patch -s --cc="One <one@xxxxxxxxxxx>" --cc=two@xxxxxxxxxxx -n HEAD^1`
'
+# Test no confirm early to ensure remaining tests will not hang
+test_no_confirm () {
+ rm -f no_confirm_okay
+ echo n | \
+ GIT_SEND_EMAIL_NOTTY=1 \
+ git send-email \
+ --from="Example <from@xxxxxxxxxxx>" \
+ --to=nobody@xxxxxxxxxxx \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $@ \
+ $patches > stdout &&
+ test_must_fail grep "Send this email" stdout &&
+ > no_confirm_okay
+}
+
+# Exit immediately to prevent hang if a no-confirm test fails
+check_no_confirm () {
+ test -f no_confirm_okay || {
+ say 'No confirm test failed; skipping remaining tests to prevent hanging'
+ test_done
+ }
+}
+
+test_expect_success 'No confirm with --suppress-cc' '
+ test_no_confirm --suppress-cc=sob
+'
+check_no_confirm
+
+test_expect_success 'No confirm with --confirm=never' '
+ test_no_confirm --confirm=never
+'
+check_no_confirm
+
+# leave sendemail.confirm set to never after this so that none of the
+# remaining tests prompt unintentionally.
+test_expect_success 'No confirm with sendemail.confirm=never' '
+ git config sendemail.confirm never &&
+ test_no_confirm --compose --subject=foo
+'
+check_no_confirm
+
test_expect_success 'Send patches' '
git send-email --suppress-cc=sob --from="Example <nobody@xxxxxxxxxxx>" --to=nobody@xxxxxxxxxxx --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
'
@@ -175,15 +216,13 @@ test_set_editor "$(pwd)/fake-editor"
test_expect_success '--compose works' '
clean_fake_sendmail &&
- echo y | \
- GIT_SEND_EMAIL_NOTTY=1 \
- git send-email \
- --compose --subject foo \
- --from="Example <nobody@xxxxxxxxxxx>" \
- --to=nobody@xxxxxxxxxxx \
- --smtp-server="$(pwd)/fake.sendmail" \
- $patches \
- 2>errors
+ git send-email \
+ --compose --subject foo \
+ --from="Example <nobody@xxxxxxxxxxx>" \
+ --to=nobody@xxxxxxxxxxx \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $patches \
+ 2>errors
'
test_expect_success 'first message is compose text' '
@@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' '
test_suppression cc
'
+test_confirm () {
+ echo y | \
+ GIT_SEND_EMAIL_NOTTY=1 \
+ git send-email \
+ --from="Example <nobody@xxxxxxxxxxx>" \
+ --to=nobody@xxxxxxxxxxx \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $@ \
+ $patches | grep "Send this email"
+}
+
+test_expect_success '--confirm=always' '
+ test_confirm --confirm=always --suppress-cc=all
+'
+
+test_expect_success '--confirm=auto' '
+ test_confirm --confirm=auto
+'
+
+test_expect_success '--confirm=cc' '
+ test_confirm --confirm=cc
+'
+
+test_expect_success '--confirm=compose' '
+ test_confirm --confirm=compose --compose
+'
+
+test_expect_success 'confirm by default (due to cc)' '
+ CONFIRM=$(git config --get sendemail.confirm) &&
+ git config --unset sendemail.confirm &&
+ test_confirm &&
+ git config sendemail.confirm $CONFIRM
+'
+
+test_expect_success 'confirm by default (due to --compose)' '
+ CONFIRM=$(git config --get sendemail.confirm) &&
+ git config --unset sendemail.confirm &&
+ test_confirm --suppress-cc=all --compose
+ ret="$?"
+ git config sendemail.confirm ${CONFIRM:-never}
+ test $ret = "0"
+'
+
test_expect_success '--compose adds MIME for utf8 body' '
clean_fake_sendmail &&
(echo "#!$SHELL_PATH" &&
echo "echo utf8 body: àéìöú >>\"\$1\""
) >fake-editor-utf8 &&
chmod +x fake-editor-utf8 &&
- echo y | \
GIT_EDITOR="\"$(pwd)/fake-editor-utf8\"" \
- GIT_SEND_EMAIL_NOTTY=1 \
git send-email \
--compose --subject foo \
--from="Example <nobody@xxxxxxxxxxx>" \
@@ -405,9 +485,7 @@ test_expect_success '--compose respects user mime type' '
echo " echo utf8 body: àéìöú) >\"\$1\""
) >fake-editor-utf8-mime &&
chmod +x fake-editor-utf8-mime &&
- echo y | \
GIT_EDITOR="\"$(pwd)/fake-editor-utf8-mime\"" \
- GIT_SEND_EMAIL_NOTTY=1 \
git send-email \
--compose --subject foo \
--from="Example <nobody@xxxxxxxxxxx>" \
@@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' '
test_expect_success '--compose adds MIME for utf8 subject' '
clean_fake_sendmail &&
- echo y | \
GIT_EDITOR="\"$(pwd)/fake-editor\"" \
- GIT_SEND_EMAIL_NOTTY=1 \
git send-email \
--compose --subject utf8-sübjëct \
--from="Example <nobody@xxxxxxxxxxx>" \
@@ -445,7 +521,7 @@ test_expect_success 'detects ambiguous reference/file conflict' '
test_expect_success 'feed two files' '
rm -fr outdir &&
git format-patch -2 -o outdir &&
- GIT_SEND_EMAIL_NOTTY=1 git send-email \
+ git send-email \
--dry-run \
--from="Example <nobody@xxxxxxxxxxx>" \
--to=nobody@xxxxxxxxxxx \
--
1.6.2.rc1.309.g5f417
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
