send-email: add --confirm option and configuration setting

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'

If sendemail.confirm is unconfigured, 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 annoying some existing users. We
attempt to mitigate the latter by:

* Allowing the user to set 'git config sendemail.confirm 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 if it is
unconfigured whenever we prompt due to Cc before sending.
* Only prompting if no --suppress related options have been passed, as
using such an option is likely to indicate an experienced send-email
user.

There is a slight fib in message informing the user of the
sendemail.confirm setting and this is intentional. Setting 'auto'
differs from leaving sendemail.confirm unset in two ways: 1) 'auto'
obviously squelches the informational message; 2) 'auto' prompts when
the Cc list has been expanded even in the presence of a --suppress
related option, where leaving sendemail.confirm unset does not. This is
intentional to keep the message simple, and to avoid adding another
sendemail.confirm value ('auto-except-suppress'?).

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 164d149ea3cca45fb2a7ea365e9bd35b0e7c6ff6..0335727012ed7c3cd7467840b0483be2bddc6a8f 100644 (file)
--- 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
 ~~~~~~~~~~~~~
 
 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.
 
 --dry-run::
        Do everything except actually send the emails.
 


@@ -242,6 +258,11 @@ sendemail.multiedit::
        summary when '--compose' is used). If false, files will be edited one
        after the other, spawning a new editor each time.
 
        summary when '--compose' is used). If false, files will be edited one
        after the other, spawning a new editor each time.
 

+sendemail.confirm::
+       Sets the default for whether to confirm before sending. Must be
+       one of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'
+       in the previous section for the meaning of these values.
+

 
 Author
 ------
 
 Author
 ------

diff --git a/git-send-email.perl b/git-send-email.perl
index adf7ecb5c35a978bad5a15463cb9cf54665b9860..57127aa823833f75fb546e738fcb19381fc331f7 100755 (executable)
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,6 +75,8 @@ sub usage {
     --[no-]thread                  * Use In-Reply-To: field. Default on.
 
   Administering:
     --[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.
     --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 ($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 = (
 my (@suppress_cc);
 
 my %config_bool_settings = (


@@ -207,6 +209,7 @@ sub do_edit {
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
     "multiedit" => \$multiedit,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
     "multiedit" => \$multiedit,

+    "confirm"   => \$confirm,

 );
 
 # Handle Uncouth Termination
 );
 
 # Handle Uncouth Termination


@@ -258,6 +261,7 @@ sub signal_handler {
                    "suppress-from!" => \$suppress_from,
                    "suppress-cc=s" => \@suppress_cc,
                    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
                    "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,
                    "dry-run" => \$dry_run,
                    "envelope-sender=s" => \$envelope_sender,
                    "thread!" => \$thread,


@@ -346,6 +350,14 @@ sub read_config {
        delete $suppress_cc{'body'};
 }
 
        delete $suppress_cc{'body'};
 }
 

+# Set confirm's default value
+my $confirm_unconfigured = !defined $confirm;
+if ($confirm_unconfigured) {
+       $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";
 # Debugging, print out the suppressions.
 if (0) {
        print "suppressions:\n";


@@ -663,25 +675,13 @@ sub expand_aliases {
        $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
 }
 
        $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
 }
 
 # 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;
 
 sub extract_valid_address {
        my $address = shift;


@@ -837,6 +837,37 @@ sub send_message
        unshift (@sendmail_parameters,
                        '-f', $raw_from) if(defined $envelope_sender);
 
        unshift (@sendmail_parameters,
                        '-f', $raw_from) if(defined $envelope_sender);
 

+       if ($needs_confirm && !$dry_run) {
+               print "\n$header\n";
+               if ($needs_confirm eq "inform") {
+                       $confirm_unconfigured = 0; # squelch this message for the rest of this run
+                       print "    The Cc list above has been expanded by additional\n";
+                       print "    addresses found in the patch commit message. By default\n";
+                       print "    send-email prompts before sending whenever this occurs.\n";
+                       print "    This behavior is controlled by the sendemail.confirm\n";
+                       print "    configuration setting.\n";
+                       print "\n";
+                       print "    For additional information, run 'git send-email --help'.\n";
+                       print "    To retain the current behavior, but squelch this message,\n";
+                       print "    run 'git config --global sendemail.confirm auto'.\n\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';
+               }
+       }
+

        if ($dry_run) {
                # We don't want to send the email.
        } elsif ($smtp_server =~ m#^/#) {
        if ($dry_run) {
                # We don't want to send the email.
        } elsif ($smtp_server =~ m#^/#) {


@@ -935,6 +966,7 @@ sub send_message
 $reply_to = $initial_reply_to;
 $references = $initial_reply_to || '';
 $subject = $initial_subject;
 $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";
 
 foreach my $t (@files) {
        open(F,"<",$t) or die "can't open file $t";


@@ -943,11 +975,12 @@ sub send_message
        my $author_encoding;
        my $has_content_type;
        my $body_encoding;
        my $author_encoding;
        my $has_content_type;
        my $body_encoding;

-       @cc = @initial_cc;
+       @cc = ();

        @xh = ();
        my $input_format = undef;
        my @header = ();
        $message = "";
        @xh = ();
        my $input_format = undef;
        my @header = ();
        $message = "";

+       $message_num++;

        # First unfold multiline header fields
        while(<F>) {
                last if /^\s*$/;
        # First unfold multiline header fields
        while(<F>) {
                last if /^\s*$/;


@@ -1080,6 +1113,14 @@ sub send_message
                }
        }
 
                }
        }
 

+       $needs_confirm = (
+               $confirm eq "always" or
+               ($confirm =~ /^(?:auto|cc)$/ && @cc) or
+               ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));
+       $needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc);
+
+       @cc = (@initial_cc, @cc);
+

        send_message();
 
        # set up for the next message
        send_message();
 
        # set up for the next message


@@ -1094,13 +1135,10 @@ sub send_message
        $message_id = undef;
 }
 
        $message_id = undef;
 }
 

-if ($compose) {
-       cleanup_compose_files();
-}
+cleanup_compose_files();

 
 sub 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;
 }
 
 $smtp->quit if $smtp;

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4df4f965cb0f2220ea9acf769c4fabf1248ab9a1..08d5b91c9176bd9c6a23ffa72b2729bb1d2ba0d8 100755 (executable)
--- 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@example.com>" --cc=two@example.com -n HEAD^1`
 '
 
     patches=`git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -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@example.com>" \
+               --to=nobody@example.com \
+               --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@example.com>" --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors
 '
 test_expect_success 'Send patches' '
      git send-email --suppress-cc=sob --from="Example <nobody@example.com>" --to=nobody@example.com --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 &&
 
 test_expect_success '--compose works' '
        clean_fake_sendmail &&

-       echo y | \
-               GIT_SEND_EMAIL_NOTTY=1 \
-               git send-email \
-               --compose --subject foo \
-               --from="Example <nobody@example.com>" \
-               --to=nobody@example.com \
-               --smtp-server="$(pwd)/fake.sendmail" \
-               $patches \
-               2>errors
+       git send-email \
+       --compose --subject foo \
+       --from="Example <nobody@example.com>" \
+       --to=nobody@example.com \
+       --smtp-server="$(pwd)/fake.sendmail" \
+       $patches \
+       2>errors

 '
 
 test_expect_success 'first message is compose text' '
 '
 
 test_expect_success 'first message is compose text' '


@@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' '
        test_suppression cc
 '
 
        test_suppression cc
 '
 

+test_confirm () {
+       echo y | \
+               GIT_SEND_EMAIL_NOTTY=1 \
+               git send-email \
+               --from="Example <nobody@example.com>" \
+               --to=nobody@example.com \
+               --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 &&
 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_EDITOR="\"$(pwd)/fake-editor-utf8\"" \

-         GIT_SEND_EMAIL_NOTTY=1 \

          git send-email \
          --compose --subject foo \
          --from="Example <nobody@example.com>" \
          git send-email \
          --compose --subject foo \
          --from="Example <nobody@example.com>" \


@@ -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 " echo utf8 body: àéìöú) >\"\$1\""
        ) >fake-editor-utf8-mime &&
        chmod +x fake-editor-utf8-mime &&

-       echo y | \

          GIT_EDITOR="\"$(pwd)/fake-editor-utf8-mime\"" \
          GIT_EDITOR="\"$(pwd)/fake-editor-utf8-mime\"" \

-         GIT_SEND_EMAIL_NOTTY=1 \

          git send-email \
          --compose --subject foo \
          --from="Example <nobody@example.com>" \
          git send-email \
          --compose --subject foo \
          --from="Example <nobody@example.com>" \


@@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' '
 
 test_expect_success '--compose adds MIME for utf8 subject' '
        clean_fake_sendmail &&
 
 test_expect_success '--compose adds MIME for utf8 subject' '
        clean_fake_sendmail &&

-       echo y | \

          GIT_EDITOR="\"$(pwd)/fake-editor\"" \
          GIT_EDITOR="\"$(pwd)/fake-editor\"" \

-         GIT_SEND_EMAIL_NOTTY=1 \

          git send-email \
          --compose --subject utf8-sübjëct \
          --from="Example <nobody@example.com>" \
          git send-email \
          --compose --subject utf8-sübjëct \
          --from="Example <nobody@example.com>" \


@@ -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 &&
 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@example.com>" \
        --to=nobody@example.com \
        --dry-run \
        --from="Example <nobody@example.com>" \
        --to=nobody@example.com \