{"thread":{"id":"18607","subject":"[PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","startedAt":"2009-03-29T01:39:10Z","lastAt":"2009-03-31T14:36:04Z","messageCount":17,"participants":["Jay Soffian","Junio C Hamano","Matthieu Moy","Björn Steinbrink"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"109745","messageId":"1238290751-57461-1-git-send-email-jaysoffian@gmail.com","threadId":"18607","inReplyTo":null,"subject":"[PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-29T01:39:10Z","receivedAt":"2009-03-29T01:39:10Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Several places in send-email prompt for input, and will do so forever\nwhen the input is EOF. This is poor behavior when send-email is run\nunattended (say from cron).\n\nThis patch refactors the prompting to an ask() function which takes a\nprompt, an optional default, and an optional regex to validate the\ninput. The function returns on EOF, or if a default is provided and the\nuser simply types return, or if the input passes the validating regex\n(which accepts all input by default). The ask() function gives up after\n10 tries in case of invalid input.\n\nThere are three callers of the function:\n\n1) \"Who should the emails appear to be from?\" which provides a default\nsender. Previously the user would have to type ctrl-d to accept the\ndefault. Now the user can just hit return, or type ctrl-d.\n\n2) \"Who should the emails be sent to?\". Previously this prompt passed a\nsecond argument (\"\") to $term->readline() which was ignored. I believe\nthe intent was to allow the user to just hit return. Now the user\ncan do so, or type ctrl-d.\n\n3) \"Send this email?\". Previously this prompt would loop forever until\nit got a valid reply. Now it stops prompting on EOF or a valid reply. In\nthe case where confirm = \"inform\", it now defaults to \"y\" on EOF or the\nuser hitting return, otherwise an invalid reply causes send-email to\nterminate.\n\nA followup patch adds tests for the new functionality.\n\nSigned-off-by: Jay Soffian \n---\n git-send-email.perl | 66 ++++++++++++++++++++++++++------------------------\n 1 files changed, 34 insertions(+), 32 deletions(-)\n\nThis and the next patch address the bug reported by Matthieu in\nhttp://article.gmane.org/gmane.comp.version-control.git/114577\n\nOf course, I've been wanting to refactor the prompting for a while, so I used\nthe bug fix as an excuse to do so.\n\nj.\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 546d2eb..f0405c8 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -606,32 +606,40 @@ EOT\n \tdo_edit(@files);\n }\n \n+sub ask {\n+\tmy ($prompt, %arg) = @_;\n+\tmy $valid_re =$arg{valid_re} || \"\"; # \"\" matches anything\n+\tmy $default = $arg{default};\n+\tmy $resp;\n+\tmy $i = 0;\n+\twhile ($i++ < 10) {\n+\t\t$resp = $term->readline($prompt);\n+\t\tif (!defined $resp) { # EOF\n+\t\t\tprint \"\\n\";\n+\t\t\treturn defined $default ? $default : undef;\n+\t\t}\n+\t\tif ($resp eq '' and defined $default) {\n+\t\t\treturn $default;\n+\t\t}\n+\t\tif ($resp =~ /$valid_re/) {\n+\t\t\treturn $resp;\n+\t\t}\n+\t}\n+\treturn undef;\n+}\n+\n my $prompting = 0;\n if (!defined $sender) {\n \t$sender = $repoauthor || $repocommitter || '';\n-\n-\twhile (1) {\n-\t\t$_ = $term->readline(\"Who should the emails appear to be from? [$sender] \");\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\t$sender = $_ if ($_);\n+\t$sender = ask(\"Who should the emails appear to be from? [$sender] \",\n+\t default => $sender);\n \tprint \"Emails will be sent from: \", $sender, \"\\n\";\n \t$prompting++;\n }\n \n if (!@to) {\n-\n-\n-\twhile (1) {\n-\t\t$_ = $term->readline(\"Who should the emails be sent to? \", \"\");\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\tmy $to = $_;\n-\tpush @to, parse_address_line($to);\n+\tmy $to = ask(\"Who should the emails be sent to? \");\n+\tpush @to, parse_address_line($to) if defined $to; # sanitized/validated later\n \t$prompting++;\n }\n \n@@ -651,13 +659,8 @@ sub expand_aliases {\n @bcclist = expand_aliases(@bcclist);\n \n if ($thread && !defined $initial_reply_to && $prompting) {\n-\twhile (1) {\n-\t\t$_= $term->readline(\"Message-ID to be used as In-Reply-To for the first email? \", $initial_reply_to);\n-\t\tlast if defined $_;\n-\t\tprint \"\\n\";\n-\t}\n-\n-\t$initial_reply_to = $_;\n+\t$initial_reply_to = ask(\n+\t\t\"Message-ID to be used as In-Reply-To for the first email? \");\n }\n if (defined $initial_reply_to) {\n \t$initial_reply_to =~ s/^\\s*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\t$_ = ask(\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \",\n+\t\t valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,\n+\t\t default => $ask_default);\n+\t\tdie \"Send this email reply required\" unless defined $_;\n \t\tif (/^n/i) {\n \t\t\treturn;\n \t\t} elsif (/^q/i) {\n-- \n1.6.2.313.g33352\n"},{"id":"109746","messageId":"1238290751-57461-2-git-send-email-jaysoffian@gmail.com","threadId":"18607","inReplyTo":"1238290751-57461-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-29T01:39:11Z","receivedAt":"2009-03-29T01:39:11Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"\nSigned-off-by: Jay Soffian \n---\n t/t9001-send-email.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 48 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex e426c96..b4de98c 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -421,8 +421,8 @@ test_confirm () {\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+\t\t$@ $patches > stdout &&\n+\tgrep \"Send this email\" stdout\n }\n \n test_expect_success '--confirm=always' '\n@@ -444,8 +444,10 @@ test_expect_success '--confirm=compose' '\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+\ttest_confirm\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n '\n \n test_expect_success 'confirm by default (due to --compose)' '\n@@ -457,6 +459,48 @@ test_expect_success 'confirm by default (due to --compose)' '\n \ttest $ret = \"0\"\n '\n \n+test_expect_success 'confirm detects EOF (inform assumes y)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config --unset sendemail.confirm &&\n+\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email \\\n+\t\t\t--from=\"Example \" \\\n+\t\t\t--to=nobody@example.com \\\n+\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t\t$patches < /dev/null\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n+'\n+\n+test_expect_success 'confirm detects EOF (auto causes failure)' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config sendemail.confirm auto &&\n+\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\ttest_must_fail git send-email \\\n+\t\t\t--from=\"Example \" \\\n+\t\t\t--to=nobody@example.com \\\n+\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t\t$patches < /dev/null\n+\tret=\"$?\"\n+\tgit config sendemail.confirm ${CONFIRM:-never}\n+\ttest $ret = \"0\"\n+'\n+\n+test_expect_success 'confirm doesnt loop forever' '\n+\tCONFIRM=$(git config --get sendemail.confirm) &&\n+\tgit config sendemail.confirm auto &&\n+\tyes \"bogus\" | GIT_SEND_EMAIL_NOTTY=1 \\\n+\t\ttest_must_fail git send-email \\\n+\t\t\t--from=\"Example \" \\\n+\t\t\t--to=nobody@example.com \\\n+\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t\t$patches\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-- \n1.6.2.313.g33352\n"},{"id":"109747","messageId":"76718490903281848l5102afc3x89eed40ed6083dcc@mail.gmail.com","threadId":"18607","inReplyTo":"1238290751-57461-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-29T01:48:29Z","receivedAt":"2009-03-29T01:48:29Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Mar 28, 2009 at 9:39 PM, Jay Soffian wrote:\n> There are three callers of the function:\n\nWhere by \"three\" I mean \"four\". Please use this commit message instead:\n\n---snip---\nSeveral places in send-email prompt for input, and will do so forever\nwhen the input is EOF. This is poor behavior when send-email is run\nunattended (say from cron).\n\nThis patch refactors the prompting to an ask() function which takes a\nprompt, an optional default, and an optional regex to validate the\ninput. The function returns on EOF, or if a default is provided and the\nuser simply types return, or if the input passes the validating regex\n(which accepts all input by default). The ask() function gives up after\n10 tries in case of invalid input.\n\nThere are four callers of the function:\n\n1) \"Who should the emails appear to be from?\" which provides a default\nsender. Previously the user would have to type ctrl-d to accept the\ndefault. Now the user can just hit return, or type ctrl-d.\n\n2) \"Who should the emails be sent to?\". Previously this prompt passed a\nsecond argument (\"\") to $term->readline() which was ignored. I believe\nthe intent was to allow the user to just hit return. Now the user\ncan do so, or type ctrl-d.\n\n3) \"Message-ID to be used as In-Reply-To for the first email?\".\nPreviously this prompt passed a second argument (effectively undef) to\n$term->readline() which was ignored. I believe the intent was the same\nas for (2), to allow the user to just hit return. Now the user can do\nso, or type ctrl-d.\n\n4) \"Send this email?\". Previously this prompt would loop forever until\nit got a valid reply. Now it stops prompting on EOF or a valid reply. In\nthe case where confirm = \"inform\", it now defaults to \"y\" on EOF or the\nuser hitting return, otherwise an invalid reply causes send-email to\nterminate.\n\nA followup patch adds tests for the new functionality.\n\nSigned-off-by: Jay Soffian \n---snip---\n\nThanks,\n\nj.\n"},{"id":"109839","messageId":"7v3acvjyhw.fsf@gitster.siamese.dyndns.org","threadId":"18607","inReplyTo":"76718490903281848l5102afc3x89eed40ed6083dcc@mail.gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-30T06:50:03Z","receivedAt":"2009-03-30T06:50:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> On Sat, Mar 28, 2009 at 9:39 PM, Jay Soffian wrote:\n>> There are three callers of the function:\n>\n> Where by \"three\" I mean \"four\". Please use this commit message instead:\n\nThanks.\n"},{"id":"109887","messageId":"vpqd4bzjlk1.fsf@bauges.imag.fr","threadId":"18607","inReplyTo":"1238290751-57461-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-03-30T11:29:34Z","receivedAt":"2009-03-30T11:29:34Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jay Soffian writes:\n\n> Several places in send-email prompt for input, and will do so forever\n> when the input is EOF. This is poor behavior when send-email is run\n> unattended (say from cron).\n\nThanks a lot for the patch, it does fix the problem I reported in\nhttp://article.gmane.org/gmane.comp.version-control.git/114577 .\n\nMinor problem: I still (harmless) get error messages in my log:\n\n print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 193.\n readline() on closed filehandle FIN at /usr/share/perl/5.8/Term/ReadLine.pm line 395.\n print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 203.\n \nBut I can very well live with them!\n\n-- \nMatthieu\n"},{"id":"109906","messageId":"76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com","threadId":"18607","inReplyTo":"vpqd4bzjlk1.fsf@bauges.imag.fr","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-30T14:17:43Z","receivedAt":"2009-03-30T14:17:43Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 30, 2009 at 7:29 AM, Matthieu Moy wrote:\n> Minor problem: I still (harmless) get error messages in my log:\n>\n>  print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 193.\n>  readline() on closed filehandle FIN at /usr/share/perl/5.8/Term/ReadLine.pm line 395.\n>  print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 203.\n>\n> But I can very well live with them!\n\nperl send-email ... 2>/dev/null :-)\n\nSeriously though, I am unable to reproduce the messages you are\ngetting from Term::ReadLine, and I tried really hard.\n\nWhat does:\n\n$ perl -e 'use Term::ReadLine; print \"$Term::ReadLine::VERSION\\n\"'\n\ntell you?\n\nAlso, thank you for confirming the fix.\n\nj.\n"},{"id":"109907","messageId":"76718490903300718m7d172a12y4aab63f0ff75f286@mail.gmail.com","threadId":"18607","inReplyTo":"76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-30T14:18:59Z","receivedAt":"2009-03-30T14:18:59Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 30, 2009 at 10:17 AM, Jay Soffian wrote:\n> perl send-email ... 2>/dev/null :-)\n\nHmfph, perl on the mind. That should of course be \"git send-email ...\n2>/dev/null\" but it was only a half-serious suggestion anyway.\n\nj.\n"},{"id":"109910","messageId":"vpqwsa7gjlh.fsf@bauges.imag.fr","threadId":"18607","inReplyTo":"76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-03-30T14:40:10Z","receivedAt":"2009-03-30T14:40:10Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jay Soffian writes:\n\n> On Mon, Mar 30, 2009 at 7:29 AM, Matthieu Moy wrote:\n>\n> perl send-email ... 2>/dev/null :-)\n>\n> Seriously though, I am unable to reproduce the messages you are\n> getting from Term::ReadLine, and I tried really hard.\n\nAnyway, as I said, the warnings do not harm. They just appear messy in\nmy log files, but it's really a matter of polishing the UI.\n\n> What does:\n>\n> $ perl -e 'use Term::ReadLine; print \"$Term::ReadLine::VERSION\\n\"'\n\n$ perl -e 'use Term::ReadLine; print \"$Term::ReadLine::VERSION\\n\"'\n1.02\n$\n\nThat's Debian oldstable (etch), so perhaps newer versions fixed the\nproblem.\n\nThanks again,\n\n-- \nMatthieu\n"},{"id":"109915","messageId":"76718490903300845x14897599jc73f50a16b21605d@mail.gmail.com","threadId":"18607","inReplyTo":"vpqwsa7gjlh.fsf@bauges.imag.fr","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-30T15:45:16Z","receivedAt":"2009-03-30T15:45:16Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 30, 2009 at 10:40 AM, Matthieu Moy wrote:\n> Jay Soffian writes:\n>> Seriously though, I am unable to reproduce the messages you are\n>> getting from Term::ReadLine, and I tried really hard.\n>\n> Anyway, as I said, the warnings do not harm. They just appear messy in\n> my log files, but it's really a matter of polishing the UI.\n\nOkay, well, I figured out how to work the polish. Term::ReadLine is\nattempting to use /dev/tty for input/output, which is closed. And\nbecause send-email enables warnings, its attempt to do so emits the\nmessages you are seeing. Can you confirm that this patch squelches the\nwarnings?\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f0405c8..1b17728 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -612,6 +612,8 @@ sub ask {\n my $default = $arg{default};\n my $resp;\n my $i = 0;\n+ return undef unless defined fileno($term->IN) and\n+ defined fileno($term->OUT);\n while ($i++ < 10) {\n $resp = $term->readline($prompt);\n if (!defined $resp) { # EOF\n\n\nThanks,\n\nj.\n"},{"id":"109920","messageId":"76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com","threadId":"18607","inReplyTo":"76718490903300845x14897599jc73f50a16b21605d@mail.gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-30T16:04:50Z","receivedAt":"2009-03-30T16:04:50Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian wrote:\n> Okay, well, I figured out how to work the polish. Term::ReadLine is\n> attempting to use /dev/tty for input/output, which is closed. And\n> because send-email enables warnings, its attempt to do so emits the\n> messages you are seeing. Can you confirm that this patch squelches the\n> warnings?\n\nUgh, not that. This:\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f0405c8..81240ef 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -612,6 +612,9 @@ sub ask {\n my $default = $arg{default};\n my $resp;\n my $i = 0;\n+ return defined $default ? $default : undef\n+ unless defined $term->IN and defined fileno($term->IN) and\n+ defined $term->OUT and defined fileno($term->OUT);\n while ($i++ < 10) {\n $resp = $term->readline($prompt);\n if (!defined $resp) { # EOF\n\nj.\n"},{"id":"109987","messageId":"vpqvdpq9gww.fsf@bauges.imag.fr","threadId":"18607","inReplyTo":"76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-03-31T09:32:15Z","receivedAt":"2009-03-31T09:32:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jay Soffian writes:\n\n> On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian wrote:\n>> Okay, well, I figured out how to work the polish. Term::ReadLine is\n>> attempting to use /dev/tty for input/output, which is closed. And\n>> because send-email enables warnings, its attempt to do so emits the\n>> messages you are seeing. Can you confirm that this patch squelches the\n>> warnings?\n>\n> Ugh, not that. This:\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index f0405c8..81240ef 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -612,6 +612,9 @@ sub ask {\n> my $default = $arg{default};\n> my $resp;\n> my $i = 0;\n> + return defined $default ? $default : undef\n> + unless defined $term->IN and defined fileno($term->IN) and\n> + defined $term->OUT and defined fileno($term->OUT);\n> while ($i++ < 10) {\n> $resp = $term->readline($prompt);\n> if (!defined $resp) { # EOF\n\nSurprising. I did the test with 3 commits to send (titled \"hello\",\n\"hello-again\" and \"hello-oncemore\"). the log says this\n \nSend this email reply required at /home/moy/local/usr/libexec/git-core//git-send-email line 866.\nOK. Log says:\nSendmail: /usr/sbin/sendmail -i Matthieu.Moy@imag.fr\nFrom: Matthieu.Moy@imag.fr\nTo: Matthieu.Moy@imag.fr\nSubject: [PATCH 1/3] hello\nDate: Tue, 31 Mar 2009 10:47:59 +0200\nMessage-Id: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>\nX-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty\n\nResult: OK\n(mbox) Adding cc: moy from line 'From: moy '\n\nFrom: Matthieu.Moy@imag.fr\nTo: Matthieu.Moy@imag.fr\nSubject: [PATCH 2/3] hello-again\nDate: Tue, 31 Mar 2009 10:48:00 +0200\nMessage-Id: <1238489281-5518-2-git-send-email-Matthieu.Moy@imag.fr>\nX-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty\nIn-Reply-To: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>\nReferences: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr>\n\n(line 866 is this: die \"Send this email reply required\" unless defined $_;)\n\nAnd I received only patch 1/3.\n\nActually, this seems to be a totally separate issue. What happens is\nthat for the first email, the script executes :\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\t$ask_default = \"y\"; # assume yes on EOF since user hasn't explicitly asked for confirmation\n\t\t\t... (show message, ...)\n\t\t}\n\t\t$_ = ask(\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \",\n\t\t valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,\n\t\t default => $ask_default);\n\nand the second time, since $confirm_unconfigured = 0 has been set, the\nmessage is not shown again, _and_ $ask_default not set (since\n$needs_confirm is set according to $confirm_unconfigured). I think the\nsolution is to use a variable different from $confirm_unconfigured to\nsay whether the message has already been displayed, along the lines\nof:\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5916c86..9e36c35 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -357,6 +357,7 @@ if ($confirm_unconfigured) {\n };\n die \"Unknown --confirm setting: '$confirm'\\n\"\n unless $confirm =~ /^(?:auto|cc|compose|always|never)/;\n+my $confirm_msg_shown = undef;\n \n # Debugging, print out the suppressions.\n if (0) {\n@@ -844,17 +849,21 @@ X-Mailer: git-send-email $gitversion\n print \"\\n$header\\n\";\n my $ask_default;\n if ($needs_confirm eq \"inform\") {\n- $confirm_unconfigured = 0; # squelch this message for the rest of this run\n $ask_default = \"y\"; # assume yes on EOF since user hasn't explicitly asked for confirmation\n- print \" The Cc list above has been expanded by additional\\n\";\n- print \" addresses found in the patch commit message. By default\\n\";\n- print \" send-email prompts before sending whenever this occurs.\\n\";\n- print \" This behavior is controlled by the sendemail.confirm\\n\";\n- print \" configuration setting.\\n\";\n- print \"\\n\";\n- print \" For additional information, run 'git send-email --help'.\\n\";\n- print \" To retain the current behavior, but squelch this message,\\n\";\n- print \" run 'git config --global sendemail.confirm auto'.\\n\\n\";\n+ if (!defined $confirm_msg_shown) {\n+ $confirm_msg_shown = 0; # squelch this message for the rest of this run\n+ print \" The Cc list above has been expanded by additional\\n\";\n+ print \" addresses found in the patch commit message. By default\\n\";\n+ print \" send-email prompts before sending whenever this occurs.\\n\";\n+ print \" This behavior is controlled by the sendemail.confirm\\n\";\n+ print \" configuration setting.\\n\";\n+ print \"\\n\";\n+ print \" For additional information, run 'git send-email --help'.\\n\";\n+ print \" To retain the current behavior, but squelch this message,\\n\";\n+ print \" run 'git config --global sendemail.confirm auto'.\\n\\n\";\n+ }\n }\n $_ = ask(\"Send this email? ([y]es|[n]o|[q]uit|[a]ll): \",\n valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,\n\n\n(worksforme at least)\n\n-- \nMatthieu\n"},{"id":"109998","messageId":"20090331103303.GD3307@atjola.homenet","threadId":"18607","inReplyTo":"1238290751-57461-2-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-03-31T10:33:03Z","receivedAt":"2009-03-31T10:33:03Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.03.28 21:39:11 -0400, Jay Soffian wrote:\n> +test_expect_success 'confirm detects EOF (auto causes failure)' '\n> +\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tgit config sendemail.confirm auto &&\n> +\tGIT_SEND_EMAIL_NOTTY=1 \\\n> +\t\ttest_must_fail git send-email \\\n> +\t\t\t--from=\"Example \" \\\n> +\t\t\t--to=nobody@example.com \\\n> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t\t$patches < /dev/null\n> +\tret=\"$?\"\n> +\tgit config sendemail.confirm ${CONFIRM:-never}\n> +\ttest $ret = \"0\"\n> +'\n> +\n> +test_expect_success 'confirm doesnt loop forever' '\n> +\tCONFIRM=$(git config --get sendemail.confirm) &&\n> +\tgit config sendemail.confirm auto &&\n> +\tyes \"bogus\" | GIT_SEND_EMAIL_NOTTY=1 \\\n> +\t\ttest_must_fail git send-email \\\n> +\t\t\t--from=\"Example \" \\\n> +\t\t\t--to=nobody@example.com \\\n> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t\t$patches\n> +\tret=\"$?\"\n> +\tgit config sendemail.confirm ${CONFIRM:-never}\n> +\ttest $ret = \"0\"\n\nThese two cause interactive prompts for me.\n\nBjörn\n"},{"id":"110010","messageId":"76718490903310658w2dffdfa4y4f40e0757cac8034@mail.gmail.com","threadId":"18607","inReplyTo":"20090331103303.GD3307@atjola.homenet","subject":"Re: [PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-31T13:58:12Z","receivedAt":"2009-03-31T13:58:12Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"2009/3/31 Björn Steinbrink :\n>> +     GIT_SEND_EMAIL_NOTTY=1 \\\n>> +             test_must_fail git send-email \\\n>> [...]\n>> +     yes \"bogus\" | GIT_SEND_EMAIL_NOTTY=1 \\\n>> +             test_must_fail git send-email \\\n>\n> These two cause interactive prompts for me.\n\nHmpfh. I think the only way that's possible is if GIT_SEND_EMAIL_NOTTY\nisn't being set by the shell. But I'm not sure why that's unique to\nthese tests.\n\nThanks for the report.\n\nj.\n"},{"id":"110011","messageId":"76718490903310707s75e24e39l63f0d396f60a0583@mail.gmail.com","threadId":"18607","inReplyTo":"20090331103303.GD3307@atjola.homenet","subject":"Re: [PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-31T14:07:25Z","receivedAt":"2009-03-31T14:07:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"2009/3/31 Björn Steinbrink :\n> These two cause interactive prompts for me.\n\nAh, it's because of going through test_must_fail. Grrr, a test farm\nfor git would be nice. :-)\n\nThis should fix it up:\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b4de98c..cd34525 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -476,7 +476,8 @@ test_expect_success 'confirm detects EOF (inform\nassumes y)' '\n test_expect_success 'confirm detects EOF (auto causes failure)' '\n CONFIRM=$(git config --get sendemail.confirm) &&\n git config sendemail.confirm auto &&\n- GIT_SEND_EMAIL_NOTTY=1 \\\n+ GIT_SEND_EMAIL_NOTTY=1 &&\n+ export GIT_SEND_EMAIL_NOTTY &&\n test_must_fail git send-email \\\n --from=\"Example \" \\\n --to=nobody@example.com \\\n@@ -490,8 +491,9 @@ test_expect_success 'confirm detects EOF (auto\ncauses failure)' '\n test_expect_success 'confirm doesnt loop forever' '\n CONFIRM=$(git config --get sendemail.confirm) &&\n git config sendemail.confirm auto &&\n- yes \"bogus\" | GIT_SEND_EMAIL_NOTTY=1 \\\n- test_must_fail git send-email \\\n+ GIT_SEND_EMAIL_NOTTY=1 &&\n+ export GIT_SEND_EMAIL_NOTTY &&\n+ yes \"bogus\" | test_must_fail git send-email \\\n --from=\"Example \" \\\n --to=nobody@example.com \\\n --smtp-server=\"$(pwd)/fake.sendmail\" \\\n\nWhat's your OS and test shell btw?\n\nj.\n"},{"id":"110012","messageId":"76718490903310712w540205b4n3826d2d9277853fa@mail.gmail.com","threadId":"18607","inReplyTo":"vpqvdpq9gww.fsf@bauges.imag.fr","subject":"Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-31T14:12:53Z","receivedAt":"2009-03-31T14:12:53Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Mar 31, 2009 at 5:32 AM, Matthieu Moy wrote:\n> Actually, this seems to be a totally separate issue.\n\nArgh, I need more sleep these days.\n\nj.\n"},{"id":"110013","messageId":"20090331141937.GC14611@atjola.homenet","threadId":"18607","inReplyTo":"76718490903310707s75e24e39l63f0d396f60a0583@mail.gmail.com","subject":"Re: [PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-03-31T14:19:37Z","receivedAt":"2009-03-31T14:19:37Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.03.31 10:07:25 -0400, Jay Soffian wrote:\n> 2009/3/31 Björn Steinbrink :\n> > These two cause interactive prompts for me.\n> \n> Ah, it's because of going through test_must_fail. Grrr, a test farm\n> for git would be nice. :-)\n> \n> This should fix it up:\n> \n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index b4de98c..cd34525 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -476,7 +476,8 @@ test_expect_success 'confirm detects EOF (inform\n> assumes y)' '\n> test_expect_success 'confirm detects EOF (auto causes failure)' '\n> CONFIRM=$(git config --get sendemail.confirm) &&\n> git config sendemail.confirm auto &&\n> - GIT_SEND_EMAIL_NOTTY=1 \\\n> + GIT_SEND_EMAIL_NOTTY=1 &&\n> + export GIT_SEND_EMAIL_NOTTY &&\n> test_must_fail git send-email \\\n> --from=\"Example \" \\\n> --to=nobody@example.com \\\n> @@ -490,8 +491,9 @@ test_expect_success 'confirm detects EOF (auto\n> causes failure)' '\n> test_expect_success 'confirm doesnt loop forever' '\n> CONFIRM=$(git config --get sendemail.confirm) &&\n> git config sendemail.confirm auto &&\n> - yes \"bogus\" | GIT_SEND_EMAIL_NOTTY=1 \\\n> - test_must_fail git send-email \\\n> + GIT_SEND_EMAIL_NOTTY=1 &&\n> + export GIT_SEND_EMAIL_NOTTY &&\n> + yes \"bogus\" | test_must_fail git send-email \\\n> --from=\"Example \" \\\n> --to=nobody@example.com \\\n> --smtp-server=\"$(pwd)/fake.sendmail\" \\\n\nYep, after fixing the linewrapping and restoring the tabs, this works.\n\n> What's your OS and test shell btw?\n\nDebian sid, kernel 2.6.29, dash as /bin/sh\n\nThanks,\nBjörn\n"},{"id":"110014","messageId":"76718490903310736r41acc92cg9b09aff902f9850e@mail.gmail.com","threadId":"18607","inReplyTo":"20090331141937.GC14611@atjola.homenet","subject":"Re: [PATCH 2/2] send-email: add tests for refactored prompting","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-31T14:36:04Z","receivedAt":"2009-03-31T14:36:04Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"2009/3/31 Björn Steinbrink :\n> Yep, after fixing the linewrapping and restoring the tabs, this works.\n\nThanks (sorry about the formatting, gmail...).\n\n> Debian sid, kernel 2.6.29, dash as /bin/sh\n\ndash seems to be a good lowest common denominator. I'll try testing\nwith it from now on.\n\nj.\n"}]}