{"thread":{"id":"18074","subject":"[PATCH] send-email: add --confirm option","startedAt":"2009-03-01T08:23:21Z","lastAt":"2009-03-03T18:18:01Z","messageCount":22,"participants":["Jay Soffian","Junio C Hamano","Paul Gortmaker","Nanako Shiraishi","Felipe Contreras","Bert Wesarg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"106589","messageId":"1235895801-80414-1-git-send-email-jaysoffian@gmail.com","threadId":"18074","inReplyTo":null,"subject":"[PATCH] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-01T08:23:21Z","receivedAt":"2009-03-01T08:23:21Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"send-email violates the principle of least surprise by automatically\ncc'ing additional recipients without confirming this with the user.\n\nThis patch teaches send-email a --confirm option. It takes the\nfollowing values:\n\n --confirm=always always confirm before sending\n --confirm=never never confirm before sending\n --confirm=cc confirm before sending when send-email has\n automatically added addresses from the patch to\n the Cc list\n --confirm=compose confirm before sending the first message when\n using --compose. (Needed to maintain backwards\n compatibility with existing behavior.)\n --confirm=auto 'cc' + 'compose'\n\nThe option defaults to 'compose' if any suppress Cc related options have\nbeen used, otherwise it defaults to 'auto'.\n\nSigned-off-by: Jay Soffian \n---\nJunio,\n\nI based this on top of js/send-email.\n\nj.\n\n Documentation/git-send-email.txt | 16 ++++++++\n git-send-email.perl | 68 +++++++++++++++++++++++------------\n t/t9001-send-email.sh | 72 +++++++++++++++++++++++++++++--------\n 3 files changed, 117 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 164d149..bcf7ff1 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified.\n Administering\n ~~~~~~~~~~~~~\n \n+--confirm::\n+\tConfirm just before sending:\n++\n+--\n+- 'always' will always confirm before sending\n+- 'never' will never confirm before sending\n+- 'cc' will confirm before sending when send-email has automatically\n+ added addresses from the patch to the Cc list\n+- 'compose' will confirm before sending the first message when using --compose.\n+- 'auto' is equivalent to 'cc' + 'compose'\n+--\n++\n+Default is the value of 'sendemail.confirm' configuration value; if that\n+is unspecified, default to 'auto' unless any of the suppress options\n+have been specified, in which case default to 'compose'.\n+\n --dry-run::\n \tDo everything except actually send the emails.\n \ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex adf7ecb..a777e3f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -75,6 +75,8 @@ git send-email [options] \n --[no-]thread * Use In-Reply-To: field. Default on.\n \n Administering:\n+ --confirm * Confirm recipients before sending;\n+ auto, cc, compose, always, or never.\n --quiet * Output one line of info per email.\n --dry-run * Don't actually send the emails.\n --[no-]validate * Perform patch sanity checks. Default on.\n@@ -181,7 +183,7 @@ sub do_edit {\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n-my ($validate);\n+my ($validate, $confirm);\n my (@suppress_cc);\n \n my %config_bool_settings = (\n@@ -207,6 +209,7 @@ my %config_settings = (\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n \"multiedit\" => \\$multiedit,\n+ \"confirm\" => \\$confirm,\n );\n \n # Handle Uncouth Termination\n@@ -258,6 +261,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"suppress-from!\" => \\$suppress_from,\n \t\t \"suppress-cc=s\" => \\@suppress_cc,\n \t\t \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n+\t\t \"confirm=s\" => \\$confirm,\n \t\t \"dry-run\" => \\$dry_run,\n \t\t \"envelope-sender=s\" => \\$envelope_sender,\n \t\t \"thread!\" => \\$thread,\n@@ -346,6 +350,11 @@ if ($suppress_cc{'body'}) {\n \tdelete $suppress_cc{'body'};\n }\n \n+# Set confirm\n+if (!defined $confirm) {\n+\t$confirm = scalar %suppress_cc ? 'compose' : 'auto';\n+};\n+\n # Debugging, print out the suppressions.\n if (0) {\n \tprint \"suppressions:\\n\";\n@@ -663,25 +672,13 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-if ($compose) {\n-\twhile (1) {\n-\t\t$_ = $term->readline(\"Send this email? (y|n) \");\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\tif (uc substr($_,0,1) ne 'Y') {\n-\t\tcleanup_compose_files();\n-\t\texit(0);\n-\t}\n-\n-\tif ($compose > 0) {\n-\t\t@files = ($compose_filename . \".final\", @files);\n-\t}\n+if ($compose && $compose > 0) {\n+\t@files = ($compose_filename . \".final\", @files);\n }\n \n # Variables we set as part of the loop over files\n-our ($message_id, %mail, $subject, $reply_to, $references, $message);\n+our ($message_id, %mail, $subject, $reply_to, $references, $message,\n+\t$needs_confirm, $message_num);\n \n sub extract_valid_address {\n \tmy $address = shift;\n@@ -837,6 +834,25 @@ X-Mailer: git-send-email $gitversion\n \tunshift (@sendmail_parameters,\n \t\t\t'-f', $raw_from) if(defined $envelope_sender);\n \n+\tif ($needs_confirm && !$dry_run) {\n+\t\tprint \"\\n$header\\n\";\n+\t\twhile (1) {\n+\t\t\tchomp ($_ = $term->readline(\n+\t\t\t\t\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n+\t\t\t));\n+\t\t\tlast if /^(?:yes|y|no|n|quit|q|all|a)/i;\n+\t\t\tprint \"\\n\";\n+\t\t}\n+\t\tif (/^n/i) {\n+\t\t\treturn;\n+\t\t} elsif (/^q/i) {\n+\t\t\tcleanup_compose_files();\n+\t\t\texit(0);\n+\t\t} elsif (/^a/i) {\n+\t\t\t$confirm = 'never';\n+\t\t}\n+\t}\n+\n \tif ($dry_run) {\n \t\t# We don't want to send the email.\n \t} elsif ($smtp_server =~ m#^/#) {\n@@ -935,6 +951,7 @@ X-Mailer: git-send-email $gitversion\n $reply_to = $initial_reply_to;\n $references = $initial_reply_to || '';\n $subject = $initial_subject;\n+$message_num = 0;\n \n foreach my $t (@files) {\n \topen(F,\"<\",$t) or die \"can't open file $t\";\n@@ -943,11 +960,12 @@ foreach my $t (@files) {\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n-\t@cc = @initial_cc;\n+\t@cc = ();\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n \t$message = \"\";\n+\t$message_num++;\n \t# First unfold multiline header fields\n \twhile() {\n \t\tlast if /^\\s*$/;\n@@ -1080,6 +1098,13 @@ foreach my $t (@files) {\n \t\t}\n \t}\n \n+\t$needs_confirm = (\n+\t\t$confirm eq \"always\" or\n+\t\t($confirm =~ /^(?:auto|cc)$/ && @cc) or\n+\t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n+\n+\t@cc = (@initial_cc, @cc);\n+\n \tsend_message();\n \n \t# set up for the next message\n@@ -1094,13 +1119,10 @@ foreach my $t (@files) {\n \t$message_id = undef;\n }\n \n-if ($compose) {\n-\tcleanup_compose_files();\n-}\n+cleanup_compose_files();\n \n sub cleanup_compose_files() {\n-\tunlink($compose_filename, $compose_filename . \".final\");\n-\n+\tunlink($compose_filename, $compose_filename . \".final\") if $compose;\n }\n \n $smtp->quit if $smtp;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4df4f96..2f4a654 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -31,6 +31,11 @@ clean_fake_sendmail() {\n \trm -f commandline* msgtxt*\n }\n \n+# Prevent any tests from hanging\n+test_expect_success 'Set sendemail.confirm=never' '\n+\tgit config sendemail.confirm never\n+'\n+\n test_expect_success 'Extract patches' '\n patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n@@ -175,15 +180,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n \n test_expect_success '--compose works' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n-\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n-\t\tgit send-email \\\n-\t\t--compose --subject foo \\\n-\t\t--from=\"Example \" \\\n-\t\t--to=nobody@example.com \\\n-\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\t$patches \\\n-\t\t2>errors\n+\tgit send-email \\\n+\t--compose --subject foo \\\n+\t--from=\"Example \" \\\n+\t--to=nobody@example.com \\\n+\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t$patches \\\n+\t2>errors\n '\n \n test_expect_success 'first message is compose text' '\n@@ -375,15 +378,56 @@ test_expect_success '--suppress-cc=cc' '\n \ttest_suppression cc\n '\n \n+test_confirm () {\n+\techo y | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t$@ \\\n+\t\t$patches | grep \"Send this email\"\n+}\n+\n+test_expect_success '--confirm=always' '\n+\ttest_confirm --confirm=always --suppress-cc=all\n+'\n+\n+test_expect_success '--confirm=auto' '\n+\ttest_confirm --confirm=auto\n+'\n+\n+test_expect_success '--confirm=cc' '\n+\ttest_confirm --confirm=cc\n+'\n+\n+test_expect_success '--confirm=compose' '\n+\ttest_confirm --confirm=compose --compose\n+'\n+\n+test_expect_success 'confirm by default (due to cc)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm &&\n+\tgit config sendemail.confirm $CONFIRM\n+'\n+\n+test_expect_success 'confirm by default (due to --compose)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm --suppress-cc=all --compose\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n+'\n+\n test_expect_success '--compose adds MIME for utf8 body' '\n \tclean_fake_sendmail &&\n \t(echo \"#!$SHELL_PATH\" &&\n \t echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n \t) >fake-editor-utf8 &&\n \tchmod +x fake-editor-utf8 &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -405,9 +449,7 @@ test_expect_success '--compose respects user mime type' '\n \t echo \" echo utf8 body: àéìöú) >\\\"\\$1\\\"\"\n \t) >fake-editor-utf8-mime &&\n \tchmod +x fake-editor-utf8-mime &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8-mime\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -421,9 +463,7 @@ test_expect_success '--compose respects user mime type' '\n \n test_expect_success '--compose adds MIME for utf8 subject' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject utf8-sübjëct \\\n \t --from=\"Example \" \\\n@@ -445,7 +485,7 @@ test_expect_success 'detects ambiguous reference/file conflict' '\n test_expect_success 'feed two files' '\n \trm -fr outdir &&\n \tgit format-patch -2 -o outdir &&\n-\tGIT_SEND_EMAIL_NOTTY=1 git send-email \\\n+\tgit send-email \\\n \t--dry-run \\\n \t--from=\"Example \" \\\n \t--to=nobody@example.com \\\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106592","messageId":"7vhc2d8vjk.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"1235895801-80414-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] send-email: add --confirm option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-01T09:03:11Z","receivedAt":"2009-03-01T09:03:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> send-email violates the principle of least surprise by automatically\n> cc'ing additional recipients without confirming this with the user.\n>\n> This patch teaches send-email a --confirm option. It takes the\n> following values:\n>\n> --confirm=always always confirm before sending\n> --confirm=never never confirm before sending\n> --confirm=cc confirm before sending when send-email has\n> automatically added addresses from the patch to\n> the Cc list\n> --confirm=compose confirm before sending the first message when\n> using --compose. (Needed to maintain backwards\n> compatibility with existing behavior.)\n> --confirm=auto 'cc' + 'compose'\n>\n> The option defaults to 'compose' if any suppress Cc related options have\n> been used, otherwise it defaults to 'auto'.\n\nIt is hard to judge if this is a good thing to do _at this point_. Those\nwho complained may welcome it but that is hardly the point. You need to\nconvince those who stayed silent (because they thought the default won't\nchange and were not paying attention) that it is a good change.\n\nEspecially the changes (not additions) to existing tests worries me; each\nchange to the existing one is a demonstration of breaking existing users.\n\nYou cannot retrofit safety by disallowing things once you used to allow,\nwithout upsetting existing users.\n\n> @@ -837,6 +834,25 @@ X-Mailer: git-send-email $gitversion\n> \tunshift (@sendmail_parameters,\n> \t\t\t'-f', $raw_from) if(defined $envelope_sender);\n> \n> +\tif ($needs_confirm && !$dry_run) {\n> +\t\tprint \"\\n$header\\n\";\n> +\t\twhile (1) {\n> +\t\t\tchomp ($_ = $term->readline(\n> +\t\t\t\t\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n> +\t\t\t));\n> +\t\t\tlast if /^(?:yes|y|no|n|quit|q|all|a)/i;\n> +\t\t\tprint \"\\n\";\n> +\t\t}\n> +\t\tif (/^n/i) {\n> +\t\t\treturn;\n> +\t\t} elsif (/^q/i) {\n> +\t\t\tcleanup_compose_files();\n> +\t\t\texit(0);\n> +\t\t} elsif (/^a/i) {\n> +\t\t\t$confirm = 'never';\n> +\t\t}\n> +\t}\n\nI think \"[a]ll\" would make it a bit less objectionable to people who hate\nunsolicited confirmation dialogs. Nice touch.\n\n> @@ -1094,13 +1119,10 @@ foreach my $t (@files) {\n> \t$message_id = undef;\n> }\n> \n> -if ($compose) {\n> -\tcleanup_compose_files();\n> -}\n> +cleanup_compose_files();\n> \n> sub cleanup_compose_files() {\n> -\tunlink($compose_filename, $compose_filename . \".final\");\n> -\n> +\tunlink($compose_filename, $compose_filename . \".final\") if $compose;\n> }\n\nDoes this hunk have anything to do with this topic, or is it just a churn\nthat does not change any behaviour?\n\n> @@ -175,15 +180,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n> \n> test_expect_success '--compose works' '\n> \tclean_fake_sendmail &&\n> -\techo y | \\\n> -\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n> -\t\tgit send-email \\\n> -\t\t--compose --subject foo \\\n> -\t\t--from=\"Example \" \\\n> -\t\t--to=nobody@example.com \\\n> -\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> -\t\t$patches \\\n> -\t\t2>errors\n> +\tgit send-email \\\n> +\t--compose --subject foo \\\n> +\t--from=\"Example \" \\\n> +\t--to=nobody@example.com \\\n> +\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t$patches \\\n> +\t2>errors\n> '\n\nHow would test this break without this hunk? \"echo\" dies of sigpipe, or\nsomething?\n\nI am not objecting to this particular change; just asking why this change\nis here. \"It does not break, but the command shouldn't ask for\nconfirmation, and giving 'y' into it is unnecessary\" is a perfectly\nacceptable answer, but if that is the case you probably would want to\nverify that the command indeed does go ahead without asking.\n\n> @@ -375,15 +378,56 @@ test_expect_success '--suppress-cc=cc' '\n> \ttest_suppression cc\n> '\n> \n> +test_confirm () {\n> +\techo y | \\\n> +\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n> +\t\tgit send-email \\\n> +\t\t--from=\"Example \" \\\n> +\t\t--to=nobody@example.com \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t$@ \\\n> +\t\t$patches | grep \"Send this email\"\n> +}\n> +\n> +test_expect_success '--confirm=always' '\n> +\ttest_confirm --confirm=always --suppress-cc=all\n> +'\n> + ...\n> +test_expect_success 'confirm by default (due to --compose)' '\n> +\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tgit config --unset sendemail.confirm &&\n> +\ttest_confirm --suppress-cc=all --compose\n> +\tret=\"$?\"\n> +\tgit config sendemail.confirm ${CONFIRM:-never}\n> +\ttest $ret = \"0\"\n> +'\n\nThese all test you would get a prompt when you as the author expect the\ncode to give one. Do you also need tests that verify you do not ask\nneedless confirmation when the code shouldn't?\n\n> test_expect_success '--compose adds MIME for utf8 body' '\n> \tclean_fake_sendmail &&\n> \t(echo \"#!$SHELL_PATH\" &&\n> \t echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n> \t) >fake-editor-utf8 &&\n> \tchmod +x fake-editor-utf8 &&\n> -\techo y | \\\n> \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n> -\t GIT_SEND_EMAIL_NOTTY=1 \\\n> \t git send-email \\\n> \t --compose --subject foo \\\n> \t --from=\"Example \" \\\n\nIf the change you made to git-send-email.perl is later broken and the\ncommand (incorrectly) starts asking for confirmation with these command\nline options, what does this test do? Does it get stuck, forbidding the\ntest suite to be run unattended?\n"},{"id":"106607","messageId":"76718490903010605o2083ba24oda781fb7053c4321@mail.gmail.com","threadId":"18074","inReplyTo":"7vhc2d8vjk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-01T14:05:12Z","receivedAt":"2009-03-01T14:05:12Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Mar 1, 2009 at 4:03 AM, Junio C Hamano wrote:\n> It is hard to judge if this is a good thing to do _at this point_.  Those\n> who complained may welcome it but that is hardly the point.  You need to\n> convince those who stayed silent (because they thought the default won't\n> change and were not paying attention) that it is a good change.\n\nNo, I won't. This is not my itch, I don't know who those users are,\nand I think I provided any such users several easy ways out:\n\n1) They can type \"a\" after the first prompt.\n2) They can git config --global sendemail.confirm never\n3) They won't even notice if they're using any suppresscc option, such\nas yourself.\n\nI know you're sensitive to existing users after getting burned by the\ndashless-git issue, but I think you're over applying that lesson in\n*this* case.\n\n> Especially the changes (not additions) to existing tests worries me; each\n> change to the existing one is a demonstration of breaking existing users.\n\nThere were two classes of changes:\n\n1) Many of the tests Cc'd w/o warning. Those now prompt. Hence the\n\"git config sendemail.suppress never\" at the top.\n\n2) Since I added (1), the \"echo y |\" in the other tests was redundant.\nI believe had a no-confirm option been available in the past, that the\ntest authors would not have used \"echo y\". And I thought it confusing\nto leave the \"echo y|\" as someone might ask \"if it's not supposed to\nconfirm, why is there an echo y here?\"\n\n> You cannot retrofit safety by disallowing things once you used to allow,\n> without upsetting existing users.\n\nAnd you hamper your ability to improve the product by not allowing\neven minor backward incompatible changes.\n\nBut I anticipated this objection and I thought very much about how an\nexisting user might hypothetically be upset, while accommodating the\nvery real concern of a new user.\n\nAside, when I upgrade a product, I expect it to change in some ways.\nIf I really don't want change, I don't upgrade.\n\nOn balance, I think this change will be silently appreciated by many\nmore users than the few, if any, vocal objectors.\n\n>> @@ -1094,13 +1119,10 @@ foreach my $t (@files) {\n>>       $message_id = undef;\n>>  }\n>>\n>> -if ($compose) {\n>> -     cleanup_compose_files();\n>> -}\n>> +cleanup_compose_files();\n>>\n>>  sub cleanup_compose_files() {\n>> -     unlink($compose_filename, $compose_filename . \".final\");\n>> -\n>> +     unlink($compose_filename, $compose_filename . \".final\") if $compose;\n>>  }\n>\n> Does this hunk have anything to do with this topic, or is it just a churn\n> that does not change any behaviour?\n\nThe former. You'll notice there is a \"quit\" option in the prompt (in\naddition to yes/no/all). The quit option needs to call\ncleanup_compose_files(), so it now has two callers.\n\nInstead of both callers having to do this:\n\n if ($compose) {\n cleanup_compose_files();\n }\n\nI just moved the if compose test into the function so each caller can\nsimply ask:\n\n cleanup_compose_files();\n\n\n>> @@ -175,15 +180,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n>>\n>>  test_expect_success '--compose works' '\n>>       clean_fake_sendmail &&\n>> -     echo y | \\\n>> -             GIT_SEND_EMAIL_NOTTY=1 \\\n>> -             git send-email \\\n>> -             --compose --subject foo \\\n>> -             --from=\"Example \" \\\n>> -             --to=nobody@example.com \\\n>> -             --smtp-server=\"$(pwd)/fake.sendmail\" \\\n>> -             $patches \\\n>> -             2>errors\n>> +     git send-email \\\n>> +     --compose --subject foo \\\n>> +     --from=\"Example \" \\\n>> +     --to=nobody@example.com \\\n>> +     --smtp-server=\"$(pwd)/fake.sendmail\" \\\n>> +     $patches \\\n>> +     2>errors\n>>  '\n>\n> How would test this break without this hunk?  \"echo\" dies of sigpipe, or\n> something?\n>\n> I am not objecting to this particular change; just asking why this change\n> is here.  \"It does not break, but the command shouldn't ask for\n> confirmation, and giving 'y' into it is unnecessary\" is a perfectly\n> acceptable answer, but if that is the case you probably would want to\n> verify that the command indeed does go ahead without asking.\n\nThe hunk is there because the \"echo y\" is redundant with confirm=never\nwhich is now set early.\n\n> These all test you would get a prompt when you as the author expect the\n> code to give one.  Do you also need tests that verify you do not ask\n> needless confirmation when the code shouldn't?\n\nYes, I couldn't think how to test that last night for some reason, but\nit is obvious to me now. I can send a followup.\n\n>>  test_expect_success '--compose adds MIME for utf8 body' '\n>>       clean_fake_sendmail &&\n>>       (echo \"#!$SHELL_PATH\" &&\n>>        echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n>>       ) >fake-editor-utf8 &&\n>>       chmod +x fake-editor-utf8 &&\n>> -     echo y | \\\n>>         GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n>> -       GIT_SEND_EMAIL_NOTTY=1 \\\n>>         git send-email \\\n>>         --compose --subject foo \\\n>>         --from=\"Example \" \\\n>\n> If the change you made to git-send-email.perl is later broken and the\n> command (incorrectly) starts asking for confirmation with these command\n> line options, what does this test do?  Does it get stuck, forbidding the\n> test suite to be run unattended?\n\nMany of the tests (because they automatically cc) will hang waiting on\ninput if git-send-email.perl is later broken in such a way as to start\nignoring --compose.\n\nI could not think of a good way to avoid this. We could possibly \"echo\ny|\" redundantly to every test, but even that does not guarantee that\none of the other prompts cannot cause a hang. So this potential to\nhang, I think, already exists.\n\nThe best I could think of was something like (sleep 60 && kill $$) &;\nsleep_pid=$!; ... kill $sleep_pid; wrapping each test, or even the\nentire script. Suggestions here appreciated.\n\nBut, I think this can largely be addressed by testing that the change\ndoes *not* prompt when it shouldn't, and moving that test before any\nof the existing tests which could prompt.\n\nj.\n"},{"id":"106614","messageId":"1235924234-16923-1-git-send-email-jaysoffian@gmail.com","threadId":"18074","inReplyTo":"7vhc2d8vjk.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-01T16:17:14Z","receivedAt":"2009-03-01T16:17:14Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"send-email violates the principle of least surprise by automatically\ncc'ing additional recipients without confirming this with the user.\n\nThis patch teaches send-email a --confirm option. It takes the\nfollowing values:\n\n --confirm=always always confirm before sending\n --confirm=never never confirm before sending\n --confirm=cc confirm before sending when send-email has\n automatically added addresses from the patch to\n the Cc list\n --confirm=compose confirm before sending the first message when\n using --compose. (Needed to maintain backwards\n compatibility with existing behavior.)\n --confirm=auto 'cc' + 'compose'\n\nThe option defaults to 'compose' if any suppress Cc related options have\nbeen used, otherwise it defaults to 'auto'.\n\nUnfortunately, it is impossible to introduce this patch such that it\nhelps new users without potentially upsetting some existing users. We\nattempt to mitigate the latter by:\n\n * Allowing the user to \"git config sendemail.config never\"\n * Allowing the user to say \"all\" after the first prompt to not be\n prompted on remaining emails during the same invocation.\n * Telling the user about the \"sendemail.confirm\" setting whenever they\n use \"all\"\n * Only prompting if no --suppress related options have been passed, as\n using such an option is likely to indicate an experienced send-email\n user.\n\nSigned-off-by: Jay Soffian \n---\nChanges from v1:\n\n - Added no-confirm tests, and put them early to prevent the rest of\n the tests from potentially hanging. Note if one of these tests\n fails then we exit t9001-send-email.sh immediately.\n\n - Added verification of the --confirm setting. (I had done this\n previously but somehow failed to stage it.)\n\n - Added additional language to the commit messsage in an attempt to\n provide justification for the change in default behavior.\n\n - When the user specifies \"all\" in response to a confirm prompt, we\n hint them about how to use \"sendemail.confirm\" config setting.\n\n Documentation/git-send-email.txt | 16 ++++++\n git-send-email.perl | 72 +++++++++++++++++--------\n t/t9001-send-email.sh | 108 ++++++++++++++++++++++++++++++++------\n 3 files changed, 157 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 164d149..bcf7ff1 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified.\n Administering\n ~~~~~~~~~~~~~\n \n+--confirm::\n+\tConfirm just before sending:\n++\n+--\n+- 'always' will always confirm before sending\n+- 'never' will never confirm before sending\n+- 'cc' will confirm before sending when send-email has automatically\n+ added addresses from the patch to the Cc list\n+- 'compose' will confirm before sending the first message when using --compose.\n+- 'auto' is equivalent to 'cc' + 'compose'\n+--\n++\n+Default is the value of 'sendemail.confirm' configuration value; if that\n+is unspecified, default to 'auto' unless any of the suppress options\n+have been specified, in which case default to 'compose'.\n+\n --dry-run::\n \tDo everything except actually send the emails.\n \ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex adf7ecb..439b70b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -75,6 +75,8 @@ git send-email [options] \n --[no-]thread * Use In-Reply-To: field. Default on.\n \n Administering:\n+ --confirm * Confirm recipients before sending;\n+ auto, cc, compose, always, or never.\n --quiet * Output one line of info per email.\n --dry-run * Don't actually send the emails.\n --[no-]validate * Perform patch sanity checks. Default on.\n@@ -181,7 +183,7 @@ sub do_edit {\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n-my ($validate);\n+my ($validate, $confirm);\n my (@suppress_cc);\n \n my %config_bool_settings = (\n@@ -207,6 +209,7 @@ my %config_settings = (\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n \"multiedit\" => \\$multiedit,\n+ \"confirm\" => \\$confirm,\n );\n \n # Handle Uncouth Termination\n@@ -258,6 +261,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"suppress-from!\" => \\$suppress_from,\n \t\t \"suppress-cc=s\" => \\@suppress_cc,\n \t\t \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n+\t\t \"confirm=s\" => \\$confirm,\n \t\t \"dry-run\" => \\$dry_run,\n \t\t \"envelope-sender=s\" => \\$envelope_sender,\n \t\t \"thread!\" => \\$thread,\n@@ -346,6 +350,13 @@ if ($suppress_cc{'body'}) {\n \tdelete $suppress_cc{'body'};\n }\n \n+# Set confirm\n+if (!defined $confirm) {\n+\t$confirm = scalar %suppress_cc ? 'compose' : 'auto';\n+};\n+die \"Unknown --confirm setting: '$confirm'\\n\"\n+\tunless $confirm =~ /^(?:auto|cc|compose|always|never)/;\n+\n # Debugging, print out the suppressions.\n if (0) {\n \tprint \"suppressions:\\n\";\n@@ -663,25 +674,13 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-if ($compose) {\n-\twhile (1) {\n-\t\t$_ = $term->readline(\"Send this email? (y|n) \");\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\tif (uc substr($_,0,1) ne 'Y') {\n-\t\tcleanup_compose_files();\n-\t\texit(0);\n-\t}\n-\n-\tif ($compose > 0) {\n-\t\t@files = ($compose_filename . \".final\", @files);\n-\t}\n+if ($compose && $compose > 0) {\n+\t@files = ($compose_filename . \".final\", @files);\n }\n \n # Variables we set as part of the loop over files\n-our ($message_id, %mail, $subject, $reply_to, $references, $message);\n+our ($message_id, %mail, $subject, $reply_to, $references, $message,\n+\t$needs_confirm, $message_num);\n \n sub extract_valid_address {\n \tmy $address = shift;\n@@ -837,6 +836,27 @@ X-Mailer: git-send-email $gitversion\n \tunshift (@sendmail_parameters,\n \t\t\t'-f', $raw_from) if(defined $envelope_sender);\n \n+\tif ($needs_confirm && !$dry_run) {\n+\t\tprint \"\\n$header\\n\";\n+\t\twhile (1) {\n+\t\t\tchomp ($_ = $term->readline(\n+\t\t\t\t\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n+\t\t\t));\n+\t\t\tlast if /^(?:yes|y|no|n|quit|q|all|a)/i;\n+\t\t\tprint \"\\n\";\n+\t\t}\n+\t\tif (/^n/i) {\n+\t\t\treturn;\n+\t\t} elsif (/^q/i) {\n+\t\t\tcleanup_compose_files();\n+\t\t\texit(0);\n+\t\t} elsif (/^a/i) {\n+\t\t\t$confirm = 'never';\n+\t\t\tprint \"You may disable all future prompting via sendemail.confirm;\\n\";\n+\t\t\tprint \"Run 'git send-email --help' for details.\\n\";\n+\t\t}\n+\t}\n+\n \tif ($dry_run) {\n \t\t# We don't want to send the email.\n \t} elsif ($smtp_server =~ m#^/#) {\n@@ -935,6 +955,7 @@ X-Mailer: git-send-email $gitversion\n $reply_to = $initial_reply_to;\n $references = $initial_reply_to || '';\n $subject = $initial_subject;\n+$message_num = 0;\n \n foreach my $t (@files) {\n \topen(F,\"<\",$t) or die \"can't open file $t\";\n@@ -943,11 +964,12 @@ foreach my $t (@files) {\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n-\t@cc = @initial_cc;\n+\t@cc = ();\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n \t$message = \"\";\n+\t$message_num++;\n \t# First unfold multiline header fields\n \twhile() {\n \t\tlast if /^\\s*$/;\n@@ -1080,6 +1102,13 @@ foreach my $t (@files) {\n \t\t}\n \t}\n \n+\t$needs_confirm = (\n+\t\t$confirm eq \"always\" or\n+\t\t($confirm =~ /^(?:auto|cc)$/ && @cc) or\n+\t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n+\n+\t@cc = (@initial_cc, @cc);\n+\n \tsend_message();\n \n \t# set up for the next message\n@@ -1094,13 +1123,10 @@ foreach my $t (@files) {\n \t$message_id = undef;\n }\n \n-if ($compose) {\n-\tcleanup_compose_files();\n-}\n+cleanup_compose_files();\n \n sub cleanup_compose_files() {\n-\tunlink($compose_filename, $compose_filename . \".final\");\n-\n+\tunlink($compose_filename, $compose_filename . \".final\") if $compose;\n }\n \n $smtp->quit if $smtp;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4df4f96..08d5b91 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -35,6 +35,47 @@ test_expect_success 'Extract patches' '\n patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n \n+# Test no confirm early to ensure remaining tests will not hang\n+test_no_confirm () {\n+\trm -f no_confirm_okay\n+\techo n | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t$@ \\\n+\t\t$patches > stdout &&\n+\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t> no_confirm_okay\n+}\n+\n+# Exit immediately to prevent hang if a no-confirm test fails\n+check_no_confirm () {\n+\ttest -f no_confirm_okay || {\n+\t\tsay 'No confirm test failed; skipping remaining tests to prevent hanging'\n+\t\ttest_done\n+\t}\n+}\n+\n+test_expect_success 'No confirm with --suppress-cc' '\n+\ttest_no_confirm --suppress-cc=sob\n+'\n+check_no_confirm\n+\n+test_expect_success 'No confirm with --confirm=never' '\n+\ttest_no_confirm --confirm=never\n+'\n+check_no_confirm\n+\n+# leave sendemail.confirm set to never after this so that none of the\n+# remaining tests prompt unintentionally.\n+test_expect_success 'No confirm with sendemail.confirm=never' '\n+\tgit config sendemail.confirm never &&\n+\ttest_no_confirm --compose --subject=foo\n+'\n+check_no_confirm\n+\n test_expect_success 'Send patches' '\n git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n '\n@@ -175,15 +216,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n \n test_expect_success '--compose works' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n-\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n-\t\tgit send-email \\\n-\t\t--compose --subject foo \\\n-\t\t--from=\"Example \" \\\n-\t\t--to=nobody@example.com \\\n-\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\t$patches \\\n-\t\t2>errors\n+\tgit send-email \\\n+\t--compose --subject foo \\\n+\t--from=\"Example \" \\\n+\t--to=nobody@example.com \\\n+\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t$patches \\\n+\t2>errors\n '\n \n test_expect_success 'first message is compose text' '\n@@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' '\n \ttest_suppression cc\n '\n \n+test_confirm () {\n+\techo y | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t$@ \\\n+\t\t$patches | grep \"Send this email\"\n+}\n+\n+test_expect_success '--confirm=always' '\n+\ttest_confirm --confirm=always --suppress-cc=all\n+'\n+\n+test_expect_success '--confirm=auto' '\n+\ttest_confirm --confirm=auto\n+'\n+\n+test_expect_success '--confirm=cc' '\n+\ttest_confirm --confirm=cc\n+'\n+\n+test_expect_success '--confirm=compose' '\n+\ttest_confirm --confirm=compose --compose\n+'\n+\n+test_expect_success 'confirm by default (due to cc)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm &&\n+\tgit config sendemail.confirm $CONFIRM\n+'\n+\n+test_expect_success 'confirm by default (due to --compose)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm --suppress-cc=all --compose\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n+'\n+\n test_expect_success '--compose adds MIME for utf8 body' '\n \tclean_fake_sendmail &&\n \t(echo \"#!$SHELL_PATH\" &&\n \t echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n \t) >fake-editor-utf8 &&\n \tchmod +x fake-editor-utf8 &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -405,9 +485,7 @@ test_expect_success '--compose respects user mime type' '\n \t echo \" echo utf8 body: àéìöú) >\\\"\\$1\\\"\"\n \t) >fake-editor-utf8-mime &&\n \tchmod +x fake-editor-utf8-mime &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8-mime\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' '\n \n test_expect_success '--compose adds MIME for utf8 subject' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject utf8-sübjëct \\\n \t --from=\"Example \" \\\n@@ -445,7 +521,7 @@ test_expect_success 'detects ambiguous reference/file conflict' '\n test_expect_success 'feed two files' '\n \trm -fr outdir &&\n \tgit format-patch -2 -o outdir &&\n-\tGIT_SEND_EMAIL_NOTTY=1 git send-email \\\n+\tgit send-email \\\n \t--dry-run \\\n \t--from=\"Example \" \\\n \t--to=nobody@example.com \\\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106619","messageId":"7d1d9c250903010909h7d92f165oc703a05e819671a4@mail.gmail.com","threadId":"18074","inReplyTo":"1235924234-16923-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2009-03-01T17:09:17Z","receivedAt":"2009-03-01T17:09:17Z","isPatch":true,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"On Sun, Mar 1, 2009 at 11:17 AM, Jay Soffian wrote:\n> send-email violates the principle of least surprise by automatically\n> cc'ing additional recipients without confirming this with the user.\n>\n> This patch teaches send-email a --confirm option. It takes the\n> following values:\n>\n>  --confirm=always   always confirm before sending\n>  --confirm=never    never confirm before sending\n>  --confirm=cc       confirm before sending when send-email has\n>                    automatically added addresses from the patch to\n>                    the Cc list\n>  --confirm=compose  confirm before sending the first message when\n>                    using --compose. (Needed to maintain backwards\n>                    compatibility with existing behavior.)\n>  --confirm=auto     'cc' + 'compose'\n>\n> The option defaults to 'compose' if any suppress Cc related options have\n> been used, otherwise it defaults to 'auto'.\n>\n> Unfortunately, it is impossible to introduce this patch such that it\n> helps new users without potentially upsetting some existing users. We\n> attempt to mitigate the latter by:\n>\n>  * Allowing the user to \"git config sendemail.config never\"\n\nI think it should be sendemail.confirm in the above. Thanks for\ntaking this seriously -- I think lots of new git users (who probably\nwill never make it to this list) will benefit from it without ever\neven knowing.\n\nPaul.\n\n>  * Allowing the user to say \"all\" after the first prompt to not be\n>   prompted on remaining emails during the same invocation.\n>  * Telling the user about the \"sendemail.confirm\" setting whenever they\n>   use \"all\"\n>  * Only prompting if no --suppress related options have been passed, as\n>   using such an option is likely to indicate an experienced send-email\n>   user.\n>\n> Signed-off-by: Jay Soffian \n> ---\n> Changes from v1:\n>\n>  - Added no-confirm tests, and put them early to prevent the rest of\n>   the tests from potentially hanging. Note if one of these tests\n>   fails then we exit t9001-send-email.sh immediately.\n>\n>  - Added verification of the --confirm setting. (I had done this\n>   previously but somehow failed to stage it.)\n>\n>  - Added additional language to the commit messsage in an attempt to\n>   provide justification for the change in default behavior.\n>\n>  - When the user specifies \"all\" in response to a confirm prompt, we\n>   hint them about how to use \"sendemail.confirm\" config setting.\n>\n>  Documentation/git-send-email.txt |   16 ++++++\n>  git-send-email.perl              |   72 +++++++++++++++++--------\n>  t/t9001-send-email.sh            |  108 ++++++++++++++++++++++++++++++++------\n>  3 files changed, 157 insertions(+), 39 deletions(-)\n>\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index 164d149..bcf7ff1 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified.\n>  Administering\n>  ~~~~~~~~~~~~~\n>\n> +--confirm::\n> +       Confirm just before sending:\n> ++\n> +--\n> +- 'always' will always confirm before sending\n> +- 'never' will never confirm before sending\n> +- 'cc' will confirm before sending when send-email has automatically\n> +  added addresses from the patch to the Cc list\n> +- 'compose' will confirm before sending the first message when using --compose.\n> +- 'auto' is equivalent to 'cc' + 'compose'\n> +--\n> ++\n> +Default is the value of 'sendemail.confirm' configuration value; if that\n> +is unspecified, default to 'auto' unless any of the suppress options\n> +have been specified, in which case default to 'compose'.\n> +\n>  --dry-run::\n>        Do everything except actually send the emails.\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index adf7ecb..439b70b 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -75,6 +75,8 @@ git send-email [options] \n>     --[no-]thread                  * Use In-Reply-To: field. Default on.\n>\n>   Administering:\n> +    --confirm                * Confirm recipients before sending;\n> +                                     auto, cc, compose, always, or never.\n>     --quiet                        * Output one line of info per email.\n>     --dry-run                      * Don't actually send the emails.\n>     --[no-]validate                * Perform patch sanity checks. Default on.\n> @@ -181,7 +183,7 @@ sub do_edit {\n>  my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n>  my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n>  my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n> -my ($validate);\n> +my ($validate, $confirm);\n>  my (@suppress_cc);\n>\n>  my %config_bool_settings = (\n> @@ -207,6 +209,7 @@ my %config_settings = (\n>     \"suppresscc\" => \\@suppress_cc,\n>     \"envelopesender\" => \\$envelope_sender,\n>     \"multiedit\" => \\$multiedit,\n> +    \"confirm\"   => \\$confirm,\n>  );\n>\n>  # Handle Uncouth Termination\n> @@ -258,6 +261,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n>                    \"suppress-from!\" => \\$suppress_from,\n>                    \"suppress-cc=s\" => \\@suppress_cc,\n>                    \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n> +                   \"confirm=s\" => \\$confirm,\n>                    \"dry-run\" => \\$dry_run,\n>                    \"envelope-sender=s\" => \\$envelope_sender,\n>                    \"thread!\" => \\$thread,\n> @@ -346,6 +350,13 @@ if ($suppress_cc{'body'}) {\n>        delete $suppress_cc{'body'};\n>  }\n>\n> +# Set confirm\n> +if (!defined $confirm) {\n> +       $confirm = scalar %suppress_cc ? 'compose' : 'auto';\n> +};\n> +die \"Unknown --confirm setting: '$confirm'\\n\"\n> +       unless $confirm =~ /^(?:auto|cc|compose|always|never)/;\n> +\n>  # Debugging, print out the suppressions.\n>  if (0) {\n>        print \"suppressions:\\n\";\n> @@ -663,25 +674,13 @@ if (!defined $smtp_server) {\n>        $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n>  }\n>\n> -if ($compose) {\n> -       while (1) {\n> -               $_ = $term->readline(\"Send this email? (y|n) \");\n> -               last if defined $_;\n> -               print \"\\n\";\n> -       }\n> -\n> -       if (uc substr($_,0,1) ne 'Y') {\n> -               cleanup_compose_files();\n> -               exit(0);\n> -       }\n> -\n> -       if ($compose > 0) {\n> -               @files = ($compose_filename . \".final\", @files);\n> -       }\n> +if ($compose && $compose > 0) {\n> +       @files = ($compose_filename . \".final\", @files);\n>  }\n>\n>  # Variables we set as part of the loop over files\n> -our ($message_id, %mail, $subject, $reply_to, $references, $message);\n> +our ($message_id, %mail, $subject, $reply_to, $references, $message,\n> +       $needs_confirm, $message_num);\n>\n>  sub extract_valid_address {\n>        my $address = shift;\n> @@ -837,6 +836,27 @@ X-Mailer: git-send-email $gitversion\n>        unshift (@sendmail_parameters,\n>                        '-f', $raw_from) if(defined $envelope_sender);\n>\n> +       if ($needs_confirm && !$dry_run) {\n> +               print \"\\n$header\\n\";\n> +               while (1) {\n> +                       chomp ($_ = $term->readline(\n> +                               \"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n> +                       ));\n> +                       last if /^(?:yes|y|no|n|quit|q|all|a)/i;\n> +                       print \"\\n\";\n> +               }\n> +               if (/^n/i) {\n> +                       return;\n> +               } elsif (/^q/i) {\n> +                       cleanup_compose_files();\n> +                       exit(0);\n> +               } elsif (/^a/i) {\n> +                       $confirm = 'never';\n> +                       print \"You may disable all future prompting via sendemail.confirm;\\n\";\n> +                       print \"Run 'git send-email --help' for details.\\n\";\n> +               }\n> +       }\n> +\n>        if ($dry_run) {\n>                # We don't want to send the email.\n>        } elsif ($smtp_server =~ m#^/#) {\n> @@ -935,6 +955,7 @@ X-Mailer: git-send-email $gitversion\n>  $reply_to = $initial_reply_to;\n>  $references = $initial_reply_to || '';\n>  $subject = $initial_subject;\n> +$message_num = 0;\n>\n>  foreach my $t (@files) {\n>        open(F,\"<\",$t) or die \"can't open file $t\";\n> @@ -943,11 +964,12 @@ foreach my $t (@files) {\n>        my $author_encoding;\n>        my $has_content_type;\n>        my $body_encoding;\n> -       @cc = @initial_cc;\n> +       @cc = ();\n>        @xh = ();\n>        my $input_format = undef;\n>        my @header = ();\n>        $message = \"\";\n> +       $message_num++;\n>        # First unfold multiline header fields\n>        while() {\n>                last if /^\\s*$/;\n> @@ -1080,6 +1102,13 @@ foreach my $t (@files) {\n>                }\n>        }\n>\n> +       $needs_confirm = (\n> +               $confirm eq \"always\" or\n> +               ($confirm =~ /^(?:auto|cc)$/ && @cc) or\n> +               ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n> +\n> +       @cc = (@initial_cc, @cc);\n> +\n>        send_message();\n>\n>        # set up for the next message\n> @@ -1094,13 +1123,10 @@ foreach my $t (@files) {\n>        $message_id = undef;\n>  }\n>\n> -if ($compose) {\n> -       cleanup_compose_files();\n> -}\n> +cleanup_compose_files();\n>\n>  sub cleanup_compose_files() {\n> -       unlink($compose_filename, $compose_filename . \".final\");\n> -\n> +       unlink($compose_filename, $compose_filename . \".final\") if $compose;\n>  }\n>\n>  $smtp->quit if $smtp;\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 4df4f96..08d5b91 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -35,6 +35,47 @@ test_expect_success 'Extract patches' '\n>     patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n>  '\n>\n> +# Test no confirm early to ensure remaining tests will not hang\n> +test_no_confirm () {\n> +       rm -f no_confirm_okay\n> +       echo n | \\\n> +               GIT_SEND_EMAIL_NOTTY=1 \\\n> +               git send-email \\\n> +               --from=\"Example \" \\\n> +               --to=nobody@example.com \\\n> +               --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +               $@ \\\n> +               $patches > stdout &&\n> +               test_must_fail grep \"Send this email\" stdout &&\n> +               > no_confirm_okay\n> +}\n> +\n> +# Exit immediately to prevent hang if a no-confirm test fails\n> +check_no_confirm () {\n> +       test -f no_confirm_okay || {\n> +               say 'No confirm test failed; skipping remaining tests to prevent hanging'\n> +               test_done\n> +       }\n> +}\n> +\n> +test_expect_success 'No confirm with --suppress-cc' '\n> +       test_no_confirm --suppress-cc=sob\n> +'\n> +check_no_confirm\n> +\n> +test_expect_success 'No confirm with --confirm=never' '\n> +       test_no_confirm --confirm=never\n> +'\n> +check_no_confirm\n> +\n> +# leave sendemail.confirm set to never after this so that none of the\n> +# remaining tests prompt unintentionally.\n> +test_expect_success 'No confirm with sendemail.confirm=never' '\n> +       git config sendemail.confirm never &&\n> +       test_no_confirm --compose --subject=foo\n> +'\n> +check_no_confirm\n> +\n>  test_expect_success 'Send patches' '\n>      git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n>  '\n> @@ -175,15 +216,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n>\n>  test_expect_success '--compose works' '\n>        clean_fake_sendmail &&\n> -       echo y | \\\n> -               GIT_SEND_EMAIL_NOTTY=1 \\\n> -               git send-email \\\n> -               --compose --subject foo \\\n> -               --from=\"Example \" \\\n> -               --to=nobody@example.com \\\n> -               --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> -               $patches \\\n> -               2>errors\n> +       git send-email \\\n> +       --compose --subject foo \\\n> +       --from=\"Example \" \\\n> +       --to=nobody@example.com \\\n> +       --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +       $patches \\\n> +       2>errors\n>  '\n>\n>  test_expect_success 'first message is compose text' '\n> @@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' '\n>        test_suppression cc\n>  '\n>\n> +test_confirm () {\n> +       echo y | \\\n> +               GIT_SEND_EMAIL_NOTTY=1 \\\n> +               git send-email \\\n> +               --from=\"Example \" \\\n> +               --to=nobody@example.com \\\n> +               --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +               $@ \\\n> +               $patches | grep \"Send this email\"\n> +}\n> +\n> +test_expect_success '--confirm=always' '\n> +       test_confirm --confirm=always --suppress-cc=all\n> +'\n> +\n> +test_expect_success '--confirm=auto' '\n> +       test_confirm --confirm=auto\n> +'\n> +\n> +test_expect_success '--confirm=cc' '\n> +       test_confirm --confirm=cc\n> +'\n> +\n> +test_expect_success '--confirm=compose' '\n> +       test_confirm --confirm=compose --compose\n> +'\n> +\n> +test_expect_success 'confirm by default (due to cc)' '\n> +       CONFIRM=$(git config --get sendemail.confirm) &&\n> +       git config --unset sendemail.confirm &&\n> +       test_confirm &&\n> +       git config sendemail.confirm $CONFIRM\n> +'\n> +\n> +test_expect_success 'confirm by default (due to --compose)' '\n> +       CONFIRM=$(git config --get sendemail.confirm) &&\n> +       git config --unset sendemail.confirm &&\n> +       test_confirm --suppress-cc=all --compose\n> +       ret=\"$?\"\n> +       git config sendemail.confirm ${CONFIRM:-never}\n> +       test $ret = \"0\"\n> +'\n> +\n>  test_expect_success '--compose adds MIME for utf8 body' '\n>        clean_fake_sendmail &&\n>        (echo \"#!$SHELL_PATH\" &&\n>         echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n>        ) >fake-editor-utf8 &&\n>        chmod +x fake-editor-utf8 &&\n> -       echo y | \\\n>          GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n> -         GIT_SEND_EMAIL_NOTTY=1 \\\n>          git send-email \\\n>          --compose --subject foo \\\n>          --from=\"Example \" \\\n> @@ -405,9 +485,7 @@ test_expect_success '--compose respects user mime type' '\n>         echo \" echo utf8 body: àéìöú) >\\\"\\$1\\\"\"\n>        ) >fake-editor-utf8-mime &&\n>        chmod +x fake-editor-utf8-mime &&\n> -       echo y | \\\n>          GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8-mime\\\"\" \\\n> -         GIT_SEND_EMAIL_NOTTY=1 \\\n>          git send-email \\\n>          --compose --subject foo \\\n>          --from=\"Example \" \\\n> @@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' '\n>\n>  test_expect_success '--compose adds MIME for utf8 subject' '\n>        clean_fake_sendmail &&\n> -       echo y | \\\n>          GIT_EDITOR=\"\\\"$(pwd)/fake-editor\\\"\" \\\n> -         GIT_SEND_EMAIL_NOTTY=1 \\\n>          git send-email \\\n>          --compose --subject utf8-sübjëct \\\n>          --from=\"Example \" \\\n> @@ -445,7 +521,7 @@ test_expect_success 'detects ambiguous reference/file conflict' '\n>  test_expect_success 'feed two files' '\n>        rm -fr outdir &&\n>        git format-patch -2 -o outdir &&\n> -       GIT_SEND_EMAIL_NOTTY=1 git send-email \\\n> +       git send-email \\\n>        --dry-run \\\n>        --from=\"Example \" \\\n>        --to=nobody@example.com \\\n> --\n> 1.6.2.rc1.309.g5f417\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"106626","messageId":"76718490903010949h7b64eb97ob567101fbc7e4cd1@mail.gmail.com","threadId":"18074","inReplyTo":"7d1d9c250903010909h7d92f165oc703a05e819671a4@mail.gmail.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-01T17:49:47Z","receivedAt":"2009-03-01T17:49:47Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Mar 1, 2009 at 12:09 PM, Paul Gortmaker\n wrote:\n>\n> On Sun, Mar 1, 2009 at 11:17 AM, Jay Soffian wrote:\n>>  * Allowing the user to \"git config sendemail.config never\"\n>\n> I think it should be sendemail.confirm in the above.\n\nYep, thanks. Junio, please amend my commit message if v2 is acceptable as is.\n\n> Thanks for\n> taking this seriously -- I think lots of new git users (who probably\n> will never make it to this list) will benefit from it without ever\n> even knowing.\n\nWell it's all for naught unless Junio can be convinced that it's an\nacceptable trade-off. On this point I want to elaborate a little more,\nso pardon me while I step up on the soap box.\n\nOnce upon a time, all git commands were git-something and they were\ninstalled in PATH. A smattering of users complained about this.\nEventually git learned \"git something\" in addition to \"git-something\".\nThen some folks decided why not just get rid of \"git-something\"\nentirely. And so it was done. And many other users who were happy with\nthe way it was complained.\n\nAnd rightly so. The change was made w/no escape hatch for those users.\nAnd to plumbing no less. The users who wanted \"git-something\" out of\nPATH (which wasn't really hurting anything) got what they wanted, but\nnothing was done to accommodate the existing users. Eventually a\ncompromise was reached. The git-something commands moved out of PATH,\nbut were still installed, and existing users could relatively easily get\nto them by adding \"git --exec-path\" to their PATH.\n\n(Please correct me if my summary is wrong, but that's how I recall it.)\n\nIf the compromise is how it was done in the first place, perhaps Junio\nwould not be as hyper-sensitive to any change which affects existing\nusers expectations.\n\nBut this patch is not like the git-something situation. This patch\nbenefits new (all?) users, while bending over backwards to accommodate\nexisting users. And it is a porcelain change.\n\nI sympathize with Junio's aversion to accepting patches which affect\nexisting users expectations. But I also think there are times when the\ngreater good is served by such patches, and it would be nice to have\nsome guidelines for how and when such patches can be made. For example:\n\n- No non-backwards compatible changes to plumbing.\n- Non-backwards compatible changes to porcelain IFF:\n - The change provides a notable (non-trivial) benefit to new users.\n Some litmus tests for such a change might be:\n - \"in hindsight, this is how it clearly should've been done.\"\n - \"this is really confusing for new users; existing users users have\n forgotten how confusing it was when they first encountered it.\"\n - \"the default behavior is potentially dangerous/embarrassing.\"\n - Existing users can easily get the prior behavior. i.e. via a config\n setting\n - The change, where possible, maintains previous expectations if it\n appears the command is being used by an experienced user.\n\n(In fairness, there appears to be a framework for non-backwards\ncompatible changes; you introduce a warning about the change in the next\nminor release that the behavior of X will change and inform the user how\nthey can keep the existing behavior of X; wait one patch release, then\nmake the actual change. But my reading of Junio's message is that he\ndoesn't think _this_ patch is even worthy of that step until I can\ndemonstrate that there is not a silent-majority who really likes the\nexisting behavior of send-email. But as I said, this is not my itch, and\nwhile I'm happy to offer up this patch, that's as far as I go.)\n\nStepping off soapbox.\n\nj.\n"},{"id":"106670","messageId":"7vwsb85qe9.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"1235924234-16923-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-02T07:34:54Z","receivedAt":"2009-03-02T07:34:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> send-email violates the principle of least surprise by automatically\n> cc'ing additional recipients without confirming this with the user.\n>\n> This patch teaches send-email a --confirm option. It takes the\n> following values:\n>\n> --confirm=always always confirm before sending\n> --confirm=never never confirm before sending\n> --confirm=cc confirm before sending when send-email has\n> automatically added addresses from the patch to\n> the Cc list\n> --confirm=compose confirm before sending the first message when\n> using --compose. (Needed to maintain backwards\n> compatibility with existing behavior.)\n> --confirm=auto 'cc' + 'compose'\n>\n> The option defaults to 'compose' if any suppress Cc related options have\n> been used, otherwise it defaults to 'auto'.\n>\n> Unfortunately, it is impossible to introduce this patch such that it\n> helps new users without potentially upsetting some existing users. We\n> attempt to mitigate the latter by:\n>\n> * Allowing the user to \"git config sendemail.config never\"\n> * Allowing the user to say \"all\" after the first prompt to not be\n> prompted on remaining emails during the same invocation.\n> * Telling the user about the \"sendemail.confirm\" setting whenever they\n> use \"all\"\n> * Only prompting if no --suppress related options have been passed, as\n> using such an option is likely to indicate an experienced send-email\n> user.\n>\n> Signed-off-by: Jay Soffian \n\nOther than that the \"sendemail.config never\" is probably a typo, I think\nthis is the best you (or anybody) could do at this moment, unless we take\nthe route to introduce an \"improved and different command\", which I\nactually am slightly in favor.\n\nIn any case, with the lesson I learned from the post v1.6.0 fiasco, it\nmight make sense to make the command louder when it needs to look at the\nsetting of $confirm option and when the user does not have anything in the\nconfig nor command line.\n\nWhat I mean is this.\n\nIn this codepath,\n\n> +\t$needs_confirm = (\n> +\t\t$confirm eq \"always\" or\n> +\t\t($confirm =~ /^(?:auto|cc)$/ && @cc) or\n> +\t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n> +\n\nif @cc is not empty, or $compose is true, you _need to know_ what the user\nwants to happen, and you need to look at $confirm to decide (otherwise the\nvalue of $confirm does not matter). In such a case, if the repository is\nunconfigured with a sendemail.confirm configuration and the there was no\nexplicit --confirm from the command line, you do not know what the user\nwants. Instead of silently assuming 'auto' to confirm and potentially\nannoying the users with this new extra step, the command could also say\n\"by the way, if you do not want to see this message, you can squelch it by\n'git config sendemail.confirm never'\" when it defaults to 'auto'.\n\nThe command could also refuse to work in such a case and instead explain\nwhat the user's choices are, and instruct \"say 'git config\nsendemail.confirm auto' if you are not sure\" or something like that, but I\nthink for this particular new option it is a bit overkill and I wouldn't\nsuggest going that far.\n"},{"id":"106679","messageId":"20090302172401.6117@nanako3.lavabit.com","threadId":"18074","inReplyTo":"76718490903010949h7b64eb97ob567101fbc7e4cd1@mail.gmail.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-02T08:24:01Z","receivedAt":"2009-03-02T08:24:01Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Jay Soffian :\n\n> (Please correct me if my summary is wrong, but that's how I recall it.)\n\nI think your summary has some things right.\n\n - Many people complained git-foo being on their paths in the past;\n - Version 1.6.0 removed git-foo from users' path; and\n - Many people didn't like the change after the fact.\n\nHowever, you are completely wrong about the escape hatch.\n\nIf your judgement on how seriously you need to treat the backward compatibility is based on your understanding of the issue you summarized in your message, you need to reconsider what you are proposing, after learning from the true history.\n\nThe transition plan was first announced officially as part of the\nrelease notes for version 1.5.4. It said three things:\n\n - Using dashed forms of git commands (e.g. \"git-commit\") from the\n command line has been informally deprecated since early 2006, but\n now it officially is, and will be removed in the future. Use\n dash-less forms (e.g. \"git commit\") instead.\n\n - Using dashed forms from your scripts, without first prepending the\n return value from \"git --exec-path\" to the scripts' PATH, has been\n informally deprecated since early 2006, but now it officially is.\n\n - Use of dashed forms with \"PATH=$(git --exec-path):$PATH; export\n PATH\" early in your script is not deprecated with this change.\n\nNotice that:\n\n - \"now\" is December 1st, 2007.\n\n - \"will be removed in the future\" happened on August 17th, 2008 at version 1.6.0.\n\n - The notice EXPLICITLY promises to keep supporting the use of git-foo if you prepend output of 'git --exec-path'.\n\nIn other words, there was an escape hatch that had been very carefully in preparation for nine months. The same escape hach, and the promise to keep supporting it, was repeated in the release notes for version 1.6.0.\n\nYou can refresh your memory (or read it for the first time, if you weren't around back then) with a message from Junio on August 24th, 2008 after version 1.6.0 was released:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/93511\n\nIt summarized \"the original plan\" (read the message and find the phrase in the middle) and outlined how it should be implemented if the git community still wanted to go through with the plan.\n\nWith the discussion that followed the message, the community decided not to do anything (also known as the \"alternative A\").\n\nThe escape hatch was there from the beginning, is still there, and it will remain there. I should also add that it was Junio's veto of Linus'es proposal to stop installing git-foo commands for builtins that enabled this escape hatch.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/51245/focus=51272\n\nBy the way, I don't think the lesson you should take home is the need for an escape hatch. Read the message by Junio on August 24th, 2008. Being nice and not too loud during the deprecation period kept users complacent about upcoming changes and upset them when the change finally came. Being un-nice and too loud during the deprecation period would have upset them early instead. You cannot avoid upsetting users either way whenever you change the behavior. That's the lesson you should learn.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"106683","messageId":"7v7i385meo.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"20090302172401.6117@nanako3.lavabit.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-02T09:01:03Z","receivedAt":"2009-03-02T09:01:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> The escape hatch was there from the beginning, is still there, and it\n> will remain there. I should also add that it was Junio's veto of\n> Linus'es proposal to stop installing git-foo commands for builtins\n> that enabled this escape hatch.\n\nI think veto is too strong a word to describe what really happened, but in\nretrospect, if we went ahead and removed built-ins from the filesystem as\nLinus and other people advocated, the escape hatch wouldn't have worked at\nall, so in that sense you are correct. But I do not think I deserve the\ncredit for that---I do not see myself making an argument based on that\n\"possible escape-hatch\" value in that old thread.\n\nBy the way, how are you researching these old discussions? Do you have\na huge list of bookmarks?\n\n> By the way, I don't think the lesson you should take home is the need\n> for an escape hatch. Read the message by Junio on August 24th,\n> 2008. Being nice and not too loud during the deprecation period kept\n> users complacent about upcoming changes and upset them when the change\n> finally came. Being un-nice and too loud during the deprecation period\n> would have upset them early instead. You cannot avoid upsetting users\n> either way whenever you change the behavior.\n\nYup.\n\nAnd the most scary part of all is that you cannot try both. We now know\nthat for 1.6.0 transition people _claimed_ that they would have liked\nlouder deprecation period than the way 1.6.0 transition was handled, but\nthat is not (and cannot be) backed by real world experience. Nobody tried\nversions of git that warned loudly about the upcoming change every time he\ntyped \"git-commit\" to see if the louder deprecation period was really\npreferrable.\n\nWe are taking that route for 1.7.0 to warn very loudly about pushing into\nthe currently checked-out branch in 1.6.2 and onwards. We may now find\nout that people hate a loud deprecation period. Then what?\n"},{"id":"106685","messageId":"20090302182310.6117@nanako3.lavabit.com","threadId":"18074","inReplyTo":"7v7i385meo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-02T09:23:10Z","receivedAt":"2009-03-02T09:23:10Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n\n> By the way, how are you researching these old discussions? Do you have\n> a huge list of bookmarks?\n\nThere weren't that many threads that became the turning points for the project; the list need not to be huge even if somebody were to keep one.\n\nBut I don't keep such a list; I just ask the gmane archive or google.\n\n> And the most scary part of all is that you cannot try both. We now know\n> that for 1.6.0 transition people _claimed_ that they would have liked\n> louder deprecation period than the way 1.6.0 transition was handled, but\n> that is not (and cannot be) backed by real world experience. Nobody tried\n> versions of git that warned loudly about the upcoming change every time he\n> typed \"git-commit\" to see if the louder deprecation period was really\n> preferrable.\n>\n> We are taking that route for 1.7.0 to warn very loudly about pushing into\n> the currently checked-out branch in 1.6.2 and onwards. We may now find\n> out that people hate a loud deprecation period. Then what?\n\nThen you get flamed.\n\nBut isn't that what the maintainer is there for? To take blame on behalf of others, so that contributors can propose what they genuinely believe improvements without fearing what the end users would say?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"106690","messageId":"94a0d4530903020235n28baf0c8m5a9fdf01edfc85ec@mail.gmail.com","threadId":"18074","inReplyTo":"7v7i385meo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-03-02T10:35:17Z","receivedAt":"2009-03-02T10:35:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Mar 2, 2009 at 11:01 AM, Junio C Hamano wrote:\n> We are taking that route for 1.7.0 to warn very loudly about pushing into\n> the currently checked-out branch in 1.6.2 and onwards.  We may now find\n> out that people hate a loud deprecation period.  Then what?\n\nThe problem is not the 'loud deprecation period' it's the deprecation\nitself. You cannot avoid deprecation, so you cannot avoid users\ncomplaining, but you can avoid surprises, and that's what the 'loud\ndeprecation period' is for.\n\nThe 'loud deprecation period' allows users to find out *earlier* so\nthat they can comment on the issue. If a huge amount of users\ncomplain, maybe the deprecation should not proceed, or maybe someone\ncomes up with a plan B. Sill, some people would not be happy, but at\nleast their voice would have been heard.\n\nSure, it doesn't matter how it's handled, some people will still not be happy...\n\n-- \nFelipe Contreras\n"},{"id":"106695","messageId":"76718490903020433l21b98a77s574847e7484fda05@mail.gmail.com","threadId":"18074","inReplyTo":"20090302172401.6117@nanako3.lavabit.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-02T12:33:45Z","receivedAt":"2009-03-02T12:33:45Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 2, 2009 at 3:24 AM, Nanako Shiraishi wrote:\n> By the way, I don't think the lesson you should take home is the need for an escape hatch. Read the message by Junio on August 24th, 2008. Being nice and not too loud during the deprecation period kept users complacent about upcoming changes and upset them when the change finally came. Being un-nice and too loud during the deprecation period would have upset them early instead. You cannot avoid upsetting users either way whenever you change the behavior. That's the lesson you should learn.\n\nThank you for correcting me, and I apologize for the misinformation.\n\nIt is very difficult to balance between the users who are upset enough\nby the change to be vocal about it, and the users who benefit from the\nchange but that this list never hears from, but I appreciate that\nJunio is trying to accommodate both.\n\nj.\n"},{"id":"106697","messageId":"76718490903020435t3b54863yc662a3e61e517087@mail.gmail.com","threadId":"18074","inReplyTo":"7vwsb85qe9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-02T12:35:20Z","receivedAt":"2009-03-02T12:35:20Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 2, 2009 at 2:34 AM, Junio C Hamano wrote:\n> In any case, with the lesson I learned from the post v1.6.0 fiasco, it\n> might make sense to make the command louder when it needs to look at the\n> setting of $confirm option and when the user does not have anything in the\n> config nor command line.\n>\n> What I mean is this.\n\nOkay, I'll re-send. Thanks for the feedback.\n\nj.\n"},{"id":"106751","messageId":"7v1vtfjpad.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"1235924234-16923-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] send-email: add --confirm option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-03T02:47:22Z","receivedAt":"2009-03-03T02:47:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> - When the user specifies \"all\" in response to a confirm prompt, we\n> hint them about how to use \"sendemail.confirm\" config setting.\n\nI think the logic is wrong. Even when you have configured to usually ask,\nyou may want to say \"I know Cc settings for this particular series is all\ngood, but I still want the safety in my future invocations.\".\n\nHow about doing it this way instead?\n\n> @@ -346,6 +350,13 @@ if ($suppress_cc{'body'}) {\n> \tdelete $suppress_cc{'body'};\n> }\n> \n> +# Set confirm\n> +if (!defined $confirm) {\n> +\t$confirm = scalar %suppress_cc ? 'compose' : 'auto';\n\nThis is not \"Set confirm\" but \"set the default value to $confirm\". Also\nset $confirm_unconfigured = \"true\" here.\n\n> @@ -837,6 +836,27 @@ X-Mailer: git-send-email $gitversion\n> \tunshift (@sendmail_parameters,\n> \t\t\t'-f', $raw_from) if(defined $envelope_sender);\n> +\tif ($needs_confirm && !$dry_run) {\n\nAnd use it here; give some help for people who haven't seen this new\nmessage and haven't decided:\n\n\t if (!$confirm_unconfigured) {\n\t\tprint \"By default the command asks if you are sure about\\n\";\n\t\tprint \"the CC list computed from various places. If you\\n\";\n print \"do not want this confirmation step, you can say\\n\";\n print \" $ git config --global sendemail.confirm never\\n\";\n print \"to squelch it.\\n\";\n\t\tprint \"A good setting might be:\\n\";\n print \" $ git config --global sendemail.confirm auto\\n\";\n\t\tprint \"to squelch this help text that is given until you\\n\";\n\t\tprint \"configure the variable to some value.\\n\";\n\t }\n\n> +\t\tprint \"\\n$header\\n\";\n> +\t\twhile (1) {\n> +\t\t\tchomp ($_ = $term->readline(\n> +\t\t\t\t\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n> +\t\t\t));\n> +\t\t\tlast if /^(?:yes|y|no|n|quit|q|all|a)/i;\n> +\t\t\tprint \"\\n\";\n> +\t\t}\n> +\t\tif (/^n/i) {\n> +\t\t\treturn;\n> +\t\t} elsif (/^q/i) {\n> +\t\t\tcleanup_compose_files();\n> +\t\t\texit(0);\n> +\t\t} elsif (/^a/i) {\n> +\t\t\t$confirm = 'never';\n\nAnd drop the following two:\n\n> +\t\t\tprint \"You may disable all future prompting via sendemail.confirm;\\n\";\n> +\t\t\tprint \"Run 'git send-email --help' for details.\\n\";\n> +\t\t}\n> +\t}\n"},{"id":"106757","messageId":"1236055938-65407-1-git-send-email-jaysoffian@gmail.com","threadId":"18074","inReplyTo":"7v1vtfjpad.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-03T04:52:18Z","receivedAt":"2009-03-03T04:52:18Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"send-email violates the principle of least surprise by automatically\ncc'ing additional recipients without confirming this with the user.\n\nThis patch teaches send-email a --confirm option. It takes the\nfollowing values:\n\n --confirm=always always confirm before sending\n --confirm=never never confirm before sending\n --confirm=cc confirm before sending when send-email has\n automatically added addresses from the patch to\n the Cc list\n --confirm=compose confirm before sending the first message when\n using --compose. (Needed to maintain backwards\n compatibility with existing behavior.)\n --confirm=auto 'cc' + 'compose'\n\nIf sendemail.confirm is unconfigured, the option defaults to 'compose'\nif any suppress-Cc related options have been used, otherwise it defaults\nto 'auto'.\n\nUnfortunately, it is impossible to introduce this patch such that it\nhelps new users without potentially annoying some existing users. We\nattempt to mitigate the latter by:\n\n * Allowing the user to set 'git config sendemail.confirm never'\n * Allowing the user to say 'all' after the first prompt to not be\n prompted on remaining emails during the same invocation.\n * Telling the user about the 'sendemail.confirm' setting if it is\n unconfigured whenever we prompt due to Cc before sending.\n * Only prompting if no --suppress related options have been passed, as\n using such an option is likely to indicate an experienced send-email\n user.\n\nThere is a slight fib in message informing the user of the\nsendemail.confirm setting and this is intentional. Setting 'auto'\ndiffers from leaving sendemail.confirm unset in two ways: 1) 'auto'\nobviously squelches the informational message; 2) 'auto' prompts when\nthe Cc list has been expanded even in the presence of a --suppress\nrelated option, where leaving sendemail.confirm unset does not. This is\nintentional to keep the message simple, and to avoid adding another\nsendemail.confirm value ('auto-except-suppress'?).\n\nSigned-off-by: Jay Soffian \n---\nDifferences from v2:\n\n * Inform user of sendemail.confirm if it is unconfigured and we are\n in a prompting situation where the Cc list has been expanded, per\n Junio's suggestion. This is instead of informing everytime \"all\"\n is used.\n\n * Fixed typo in commit message\n\n * Documented sendemail.confirm configuration setting.\n\n Documentation/git-send-email.txt | 21 +++++++\n git-send-email.perl | 84 +++++++++++++++++++++--------\n t/t9001-send-email.sh | 108 ++++++++++++++++++++++++++++++++------\n 3 files changed, 174 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 164d149..0335727 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified.\n Administering\n ~~~~~~~~~~~~~\n \n+--confirm::\n+\tConfirm just before sending:\n++\n+--\n+- 'always' will always confirm before sending\n+- 'never' will never confirm before sending\n+- 'cc' will confirm before sending when send-email has automatically\n+ added addresses from the patch to the Cc list\n+- 'compose' will confirm before sending the first message when using --compose.\n+- 'auto' is equivalent to 'cc' + 'compose'\n+--\n++\n+Default is the value of 'sendemail.confirm' configuration value; if that\n+is unspecified, default to 'auto' unless any of the suppress options\n+have been specified, in which case default to 'compose'.\n+\n --dry-run::\n \tDo everything except actually send the emails.\n \n@@ -242,6 +258,11 @@ sendemail.multiedit::\n \tsummary when '--compose' is used). If false, files will be edited one\n \tafter the other, spawning a new editor each time.\n \n+sendemail.confirm::\n+\tSets the default for whether to confirm before sending. Must be\n+\tone of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'\n+\tin the previous section for the meaning of these values.\n+\n \n Author\n ------\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex adf7ecb..57127aa 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -75,6 +75,8 @@ git send-email [options] \n --[no-]thread * Use In-Reply-To: field. Default on.\n \n Administering:\n+ --confirm * Confirm recipients before sending;\n+ auto, cc, compose, always, or never.\n --quiet * Output one line of info per email.\n --dry-run * Don't actually send the emails.\n --[no-]validate * Perform patch sanity checks. Default on.\n@@ -181,7 +183,7 @@ sub do_edit {\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n-my ($validate);\n+my ($validate, $confirm);\n my (@suppress_cc);\n \n my %config_bool_settings = (\n@@ -207,6 +209,7 @@ my %config_settings = (\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n \"multiedit\" => \\$multiedit,\n+ \"confirm\" => \\$confirm,\n );\n \n # Handle Uncouth Termination\n@@ -258,6 +261,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"suppress-from!\" => \\$suppress_from,\n \t\t \"suppress-cc=s\" => \\@suppress_cc,\n \t\t \"signed-off-cc|signed-off-by-cc!\" => \\$signed_off_by_cc,\n+\t\t \"confirm=s\" => \\$confirm,\n \t\t \"dry-run\" => \\$dry_run,\n \t\t \"envelope-sender=s\" => \\$envelope_sender,\n \t\t \"thread!\" => \\$thread,\n@@ -346,6 +350,14 @@ if ($suppress_cc{'body'}) {\n \tdelete $suppress_cc{'body'};\n }\n \n+# Set confirm's default value\n+my $confirm_unconfigured = !defined $confirm;\n+if ($confirm_unconfigured) {\n+\t$confirm = scalar %suppress_cc ? 'compose' : 'auto';\n+};\n+die \"Unknown --confirm setting: '$confirm'\\n\"\n+\tunless $confirm =~ /^(?:auto|cc|compose|always|never)/;\n+\n # Debugging, print out the suppressions.\n if (0) {\n \tprint \"suppressions:\\n\";\n@@ -663,25 +675,13 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-if ($compose) {\n-\twhile (1) {\n-\t\t$_ = $term->readline(\"Send this email? (y|n) \");\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\tif (uc substr($_,0,1) ne 'Y') {\n-\t\tcleanup_compose_files();\n-\t\texit(0);\n-\t}\n-\n-\tif ($compose > 0) {\n-\t\t@files = ($compose_filename . \".final\", @files);\n-\t}\n+if ($compose && $compose > 0) {\n+\t@files = ($compose_filename . \".final\", @files);\n }\n \n # Variables we set as part of the loop over files\n-our ($message_id, %mail, $subject, $reply_to, $references, $message);\n+our ($message_id, %mail, $subject, $reply_to, $references, $message,\n+\t$needs_confirm, $message_num);\n \n sub extract_valid_address {\n \tmy $address = shift;\n@@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion\n \tunshift (@sendmail_parameters,\n \t\t\t'-f', $raw_from) if(defined $envelope_sender);\n \n+\tif ($needs_confirm && !$dry_run) {\n+\t\tprint \"\\n$header\\n\";\n+\t\tif ($needs_confirm eq \"inform\") {\n+\t\t\t$confirm_unconfigured = 0; # squelch this message for the rest of this run\n+\t\t\tprint \" The Cc list above has been expanded by additional\\n\";\n+\t\t\tprint \" addresses found in the patch commit message. By default\\n\";\n+\t\t\tprint \" send-email prompts before sending whenever this occurs.\\n\";\n+\t\t\tprint \" This behavior is controlled by the sendemail.confirm\\n\";\n+\t\t\tprint \" configuration setting.\\n\";\n+\t\t\tprint \"\\n\";\n+\t\t\tprint \" For additional information, run 'git send-email --help'.\\n\";\n+\t\t\tprint \" To retain the current behavior, but squelch this message,\\n\";\n+\t\t\tprint \" run 'git config --global sendemail.confirm auto'.\\n\\n\";\n+\t\t}\n+\t\twhile (1) {\n+\t\t\tchomp ($_ = $term->readline(\n+\t\t\t\t\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \"\n+\t\t\t));\n+\t\t\tlast if /^(?:yes|y|no|n|quit|q|all|a)/i;\n+\t\t\tprint \"\\n\";\n+\t\t}\n+\t\tif (/^n/i) {\n+\t\t\treturn;\n+\t\t} elsif (/^q/i) {\n+\t\t\tcleanup_compose_files();\n+\t\t\texit(0);\n+\t\t} elsif (/^a/i) {\n+\t\t\t$confirm = 'never';\n+\t\t}\n+\t}\n+\n \tif ($dry_run) {\n \t\t# We don't want to send the email.\n \t} elsif ($smtp_server =~ m#^/#) {\n@@ -935,6 +966,7 @@ X-Mailer: git-send-email $gitversion\n $reply_to = $initial_reply_to;\n $references = $initial_reply_to || '';\n $subject = $initial_subject;\n+$message_num = 0;\n \n foreach my $t (@files) {\n \topen(F,\"<\",$t) or die \"can't open file $t\";\n@@ -943,11 +975,12 @@ foreach my $t (@files) {\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n-\t@cc = @initial_cc;\n+\t@cc = ();\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n \t$message = \"\";\n+\t$message_num++;\n \t# First unfold multiline header fields\n \twhile() {\n \t\tlast if /^\\s*$/;\n@@ -1080,6 +1113,14 @@ foreach my $t (@files) {\n \t\t}\n \t}\n \n+\t$needs_confirm = (\n+\t\t$confirm eq \"always\" or\n+\t\t($confirm =~ /^(?:auto|cc)$/ && @cc) or\n+\t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n+\t$needs_confirm = \"inform\" if ($needs_confirm && $confirm_unconfigured && @cc);\n+\n+\t@cc = (@initial_cc, @cc);\n+\n \tsend_message();\n \n \t# set up for the next message\n@@ -1094,13 +1135,10 @@ foreach my $t (@files) {\n \t$message_id = undef;\n }\n \n-if ($compose) {\n-\tcleanup_compose_files();\n-}\n+cleanup_compose_files();\n \n sub cleanup_compose_files() {\n-\tunlink($compose_filename, $compose_filename . \".final\");\n-\n+\tunlink($compose_filename, $compose_filename . \".final\") if $compose;\n }\n \n $smtp->quit if $smtp;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4df4f96..08d5b91 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -35,6 +35,47 @@ test_expect_success 'Extract patches' '\n patches=`git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1`\n '\n \n+# Test no confirm early to ensure remaining tests will not hang\n+test_no_confirm () {\n+\trm -f no_confirm_okay\n+\techo n | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t$@ \\\n+\t\t$patches > stdout &&\n+\t\ttest_must_fail grep \"Send this email\" stdout &&\n+\t\t> no_confirm_okay\n+}\n+\n+# Exit immediately to prevent hang if a no-confirm test fails\n+check_no_confirm () {\n+\ttest -f no_confirm_okay || {\n+\t\tsay 'No confirm test failed; skipping remaining tests to prevent hanging'\n+\t\ttest_done\n+\t}\n+}\n+\n+test_expect_success 'No confirm with --suppress-cc' '\n+\ttest_no_confirm --suppress-cc=sob\n+'\n+check_no_confirm\n+\n+test_expect_success 'No confirm with --confirm=never' '\n+\ttest_no_confirm --confirm=never\n+'\n+check_no_confirm\n+\n+# leave sendemail.confirm set to never after this so that none of the\n+# remaining tests prompt unintentionally.\n+test_expect_success 'No confirm with sendemail.confirm=never' '\n+\tgit config sendemail.confirm never &&\n+\ttest_no_confirm --compose --subject=foo\n+'\n+check_no_confirm\n+\n test_expect_success 'Send patches' '\n git send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n '\n@@ -175,15 +216,13 @@ test_set_editor \"$(pwd)/fake-editor\"\n \n test_expect_success '--compose works' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n-\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n-\t\tgit send-email \\\n-\t\t--compose --subject foo \\\n-\t\t--from=\"Example \" \\\n-\t\t--to=nobody@example.com \\\n-\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\t$patches \\\n-\t\t2>errors\n+\tgit send-email \\\n+\t--compose --subject foo \\\n+\t--from=\"Example \" \\\n+\t--to=nobody@example.com \\\n+\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t$patches \\\n+\t2>errors\n '\n \n test_expect_success 'first message is compose text' '\n@@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' '\n \ttest_suppression cc\n '\n \n+test_confirm () {\n+\techo y | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t$@ \\\n+\t\t$patches | grep \"Send this email\"\n+}\n+\n+test_expect_success '--confirm=always' '\n+\ttest_confirm --confirm=always --suppress-cc=all\n+'\n+\n+test_expect_success '--confirm=auto' '\n+\ttest_confirm --confirm=auto\n+'\n+\n+test_expect_success '--confirm=cc' '\n+\ttest_confirm --confirm=cc\n+'\n+\n+test_expect_success '--confirm=compose' '\n+\ttest_confirm --confirm=compose --compose\n+'\n+\n+test_expect_success 'confirm by default (due to cc)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm &&\n+\tgit config sendemail.confirm $CONFIRM\n+'\n+\n+test_expect_success 'confirm by default (due to --compose)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\ttest_confirm --suppress-cc=all --compose\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n+'\n+\n test_expect_success '--compose adds MIME for utf8 body' '\n \tclean_fake_sendmail &&\n \t(echo \"#!$SHELL_PATH\" &&\n \t echo \"echo utf8 body: àéìöú >>\\\"\\$1\\\"\"\n \t) >fake-editor-utf8 &&\n \tchmod +x fake-editor-utf8 &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -405,9 +485,7 @@ test_expect_success '--compose respects user mime type' '\n \t echo \" echo utf8 body: àéìöú) >\\\"\\$1\\\"\"\n \t) >fake-editor-utf8-mime &&\n \tchmod +x fake-editor-utf8-mime &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor-utf8-mime\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject foo \\\n \t --from=\"Example \" \\\n@@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' '\n \n test_expect_success '--compose adds MIME for utf8 subject' '\n \tclean_fake_sendmail &&\n-\techo y | \\\n \t GIT_EDITOR=\"\\\"$(pwd)/fake-editor\\\"\" \\\n-\t GIT_SEND_EMAIL_NOTTY=1 \\\n \t git send-email \\\n \t --compose --subject utf8-sübjëct \\\n \t --from=\"Example \" \\\n@@ -445,7 +521,7 @@ test_expect_success 'detects ambiguous reference/file conflict' '\n test_expect_success 'feed two files' '\n \trm -fr outdir &&\n \tgit format-patch -2 -o outdir &&\n-\tGIT_SEND_EMAIL_NOTTY=1 git send-email \\\n+\tgit send-email \\\n \t--dry-run \\\n \t--from=\"Example \" \\\n \t--to=nobody@example.com \\\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106767","messageId":"7vsklvgkrx.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"1236055938-65407-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-03T06:53:06Z","receivedAt":"2009-03-03T06:53:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> There is a slight fib in message informing the user of the\n> sendemail.confirm setting and this is intentional. Setting 'auto'\n> differs from leaving sendemail.confirm unset in two ways: 1) 'auto'\n> obviously squelches the informational message; 2) 'auto' prompts when\n> the Cc list has been expanded even in the presence of a --suppress\n> related option, where leaving sendemail.confirm unset does not. This is\n> intentional to keep the message simple, and to avoid adding another\n> sendemail.confirm value ('auto-except-suppress'?).\n\nI suspect you may get bug reports and refinement patches on top of this to\nfix the last issue up, but I think that better should happen once the base\npatch is in-tree.\n\nLet's queue this to 'pu' and see how people react. \n\nThanks.\n"},{"id":"106811","messageId":"20090303191128.6117@nanako3.lavabit.com","threadId":"18074","inReplyTo":"1236055938-65407-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-03T10:11:28Z","receivedAt":"2009-03-03T10:11:28Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Jay Soffian :\n\n> Unfortunately, it is impossible to introduce this patch such that it\n> helps new users without potentially annoying some existing users. We\n> attempt to mitigate the latter by:\n>\n> * Allowing the user to set 'git config sendemail.confirm never'\n> * Allowing the user to say 'all' after the first prompt to not be\n> prompted on remaining emails during the same invocation.\n> * Telling the user about the 'sendemail.confirm' setting if it is\n> unconfigured whenever we prompt due to Cc before sending.\n> * Only prompting if no --suppress related options have been passed, as\n> using such an option is likely to indicate an experienced send-email\n> user.\n>\n> There is a slight fib in message informing the user of the\n> sendemail.confirm setting and this is intentional. Setting 'auto'\n> differs from leaving sendemail.confirm unset in two ways: 1) 'auto'\n> obviously squelches the informational message; 2) 'auto' prompts when\n> the Cc list has been expanded even in the presence of a --suppress\n> related option, where leaving sendemail.confirm unset does not. This is\n> intentional to keep the message simple, and to avoid adding another\n> sendemail.confirm value ('auto-except-suppress'?).\n\nFor what it's worth, I think this much more carefully addresses the issues to migrate existing users by giving adequate help where it matters than the previous patches.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"106823","messageId":"36ca99e90903030354j33de0313n41d8a95ff898798b@mail.gmail.com","threadId":"18074","inReplyTo":"1236055938-65407-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-03-03T11:54:24Z","receivedAt":"2009-03-03T11:54:24Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Mar 3, 2009 at 05:52, Jay Soffian wrote:\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index adf7ecb..57127aa 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion\n>        unshift (@sendmail_parameters,\n>                        '-f', $raw_from) if(defined $envelope_sender);\n>\n> +       if ($needs_confirm && !$dry_run) {\nSo, the output is now differnt with and without --dry-run?\n\nBert\n"},{"id":"106871","messageId":"76718490903030822j690cb97blea292d391c0d0112@mail.gmail.com","threadId":"18074","inReplyTo":"36ca99e90903030354j33de0313n41d8a95ff898798b@mail.gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-03T16:22:53Z","receivedAt":"2009-03-03T16:22:53Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Mar 3, 2009 at 6:54 AM, Bert Wesarg wrote:\n> On Tue, Mar 3, 2009 at 05:52, Jay Soffian wrote:\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index adf7ecb..57127aa 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion\n>>        unshift (@sendmail_parameters,\n>>                        '-f', $raw_from) if(defined $envelope_sender);\n>>\n>> +       if ($needs_confirm && !$dry_run) {\n> So, the output is now differnt with and without --dry-run?\n\nThere doesn't seem to be any point in having the user confirm before\nsending the message if the message is not actually going to be sent.\nAm I missing something?\n\nj.\n"},{"id":"106879","messageId":"7vvdqqblij.fsf@gitster.siamese.dyndns.org","threadId":"18074","inReplyTo":"76718490903030822j690cb97blea292d391c0d0112@mail.gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-03T16:48:20Z","receivedAt":"2009-03-03T16:48:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> On Tue, Mar 3, 2009 at 6:54 AM, Bert Wesarg wrote:\n>> On Tue, Mar 3, 2009 at 05:52, Jay Soffian wrote:\n>>> diff --git a/git-send-email.perl b/git-send-email.perl\n>>> index adf7ecb..57127aa 100755\n>>> --- a/git-send-email.perl\n>>> +++ b/git-send-email.perl\n>>> @@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion\n>>>        unshift (@sendmail_parameters,\n>>>                        '-f', $raw_from) if(defined $envelope_sender);\n>>>\n>>> +       if ($needs_confirm && !$dry_run) {\n>> So, the output is now differnt with and without --dry-run?\n>\n> There doesn't seem to be any point in having the user confirm before\n> sending the message if the message is not actually going to be sent.\n> Am I missing something?\n\nI do not think you are missing anything.\n\nIIRC, the --dry-run mode shows more clearly to whom you would be CC'ing\nthe messages; in other words, the behaviour would be different, but it\ngives an uninteractive way to confirm, and not pausing for confirmation is\na good thing.\n"},{"id":"106901","messageId":"36ca99e90903031005g3d54436cs486133a340c1eec1@mail.gmail.com","threadId":"18074","inReplyTo":"7vvdqqblij.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-03-03T18:05:47Z","receivedAt":"2009-03-03T18:05:47Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Mar 3, 2009 at 17:48, Junio C Hamano wrote:\n> Jay Soffian writes:\n>\n>> On Tue, Mar 3, 2009 at 6:54 AM, Bert Wesarg wrote:\n>>> On Tue, Mar 3, 2009 at 05:52, Jay Soffian wrote:\n>>>> diff --git a/git-send-email.perl b/git-send-email.perl\n>>>> index adf7ecb..57127aa 100755\n>>>> --- a/git-send-email.perl\n>>>> +++ b/git-send-email.perl\n>>>> @@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion\n>>>>        unshift (@sendmail_parameters,\n>>>>                        '-f', $raw_from) if(defined $envelope_sender);\n>>>>\n>>>> +       if ($needs_confirm && !$dry_run) {\n>>> So, the output is now differnt with and without --dry-run?\n>>\n>> There doesn't seem to be any point in having the user confirm before\n>> sending the message if the message is not actually going to be sent.\n>> Am I missing something?\n>\n> I do not think you are missing anything.\n>\n> IIRC, the --dry-run mode shows more clearly to whom you would be CC'ing\n> the messages; in other words, the behaviour would be different, but it\n> gives an uninteractive way to confirm, and not pausing for confirmation is\n> a good thing.\n>\nJust to clarify: A user who runs a --dry-run before every sending\n(like me) would check the Cc list anyway (like me), so he either would\nhave sendmail.confirm=never in the config, so that he will not\nbothered by send-email while sending or he sees some Cc's that he\ndon't want and can remove them in the sending process.\n\nOk, than I'm fine with this.\n\nRegards,\nBert\n"},{"id":"106903","messageId":"76718490903031018o57d5e33fv9936a3a3234558fc@mail.gmail.com","threadId":"18074","inReplyTo":"36ca99e90903031005g3d54436cs486133a340c1eec1@mail.gmail.com","subject":"Re: [PATCH v3] send-email: add --confirm option and configuration setting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-03T18:18:01Z","receivedAt":"2009-03-03T18:18:01Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Mar 3, 2009 at 1:05 PM, Bert Wesarg wrote:\n>\n> Just to clarify: A user who runs a --dry-run before every sending\n> (like me) would check the Cc list anyway (like me), so he either would\n> have sendmail.confirm=never in the config, so that he will not\n> bothered by send-email while sending or he sees some Cc's that he\n> don't want and can remove them in the sending process.\n\nCorrect. This confirm addition was motivated by new users who do not know about\n--dry-run and are surprised when their email is sent to unintended recipients.\n\nIf you always use --dry-run, and don't want to be prompted when you actually\nsend, then setting sendemail.confirm=never is what you want.\n\n> Ok, than I'm fine with this.\n\nPhew. :-)\n\nj.\n"}]}