# [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever 17 messages from 2009-03-29 to 2009-03-31. Participants: Jay Soffian, Junio C Hamano, Matthieu Moy, Björn Steinbrink. Thread: https://gitlist.dev/t/18607 ## Jay Soffian, 2009-03-29 01:39 Subject: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <1238290751-57461-1-git-send-email-jaysoffian@gmail.com> URL: https://gitlist.dev/e/1238290751-57461-1-git-send-email-jaysoffian%40gmail.com ``` Several places in send-email prompt for input, and will do so forever when the input is EOF. This is poor behavior when send-email is run unattended (say from cron). This patch refactors the prompting to an ask() function which takes a prompt, an optional default, and an optional regex to validate the input. The function returns on EOF, or if a default is provided and the user simply types return, or if the input passes the validating regex (which accepts all input by default). The ask() function gives up after 10 tries in case of invalid input. There are three callers of the function: 1) "Who should the emails appear to be from?" which provides a default sender. Previously the user would have to type ctrl-d to accept the default. Now the user can just hit return, or type ctrl-d. 2) "Who should the emails be sent to?". Previously this prompt passed a second argument ("") to $term->readline() which was ignored. I believe the intent was to allow the user to just hit return. Now the user can do so, or type ctrl-d. 3) "Send this email?". Previously this prompt would loop forever until it got a valid reply. Now it stops prompting on EOF or a valid reply. In the case where confirm = "inform", it now defaults to "y" on EOF or the user hitting return, otherwise an invalid reply causes send-email to terminate. A followup patch adds tests for the new functionality. Signed-off-by: Jay Soffian --- git-send-email.perl | 66 ++++++++++++++++++++++++++------------------------ 1 files changed, 34 insertions(+), 32 deletions(-) This and the next patch address the bug reported by Matthieu in http://article.gmane.org/gmane.comp.version-control.git/114577 Of course, I've been wanting to refactor the prompting for a while, so I used the bug fix as an excuse to do so. j. diff --git a/git-send-email.perl b/git-send-email.perl index 546d2eb..f0405c8 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -606,32 +606,40 @@ EOT do_edit(@files); } +sub ask { + my ($prompt, %arg) = @_; + my $valid_re =$arg{valid_re} || ""; # "" matches anything + my $default = $arg{default}; + my $resp; + my $i = 0; + while ($i++ < 10) { + $resp = $term->readline($prompt); + if (!defined $resp) { # EOF + print "\n"; + return defined $default ? $default : undef; + } + if ($resp eq '' and defined $default) { + return $default; + } + if ($resp =~ /$valid_re/) { + return $resp; + } + } + return undef; +} + my $prompting = 0; if (!defined $sender) { $sender = $repoauthor || $repocommitter || ''; - - while (1) { - $_ = $term->readline("Who should the emails appear to be from? [$sender] "); - last if defined $_; - print "\n"; - } - - $sender = $_ if ($_); + $sender = ask("Who should the emails appear to be from? [$sender] ", + default => $sender); print "Emails will be sent from: ", $sender, "\n"; $prompting++; } if (!@to) { - - - while (1) { - $_ = $term->readline("Who should the emails be sent to? ", ""); - last if defined $_; - print "\n"; - } - - my $to = $_; - push @to, parse_address_line($to); + my $to = ask("Who should the emails be sent to? "); + push @to, parse_address_line($to) if defined $to; # sanitized/validated later $prompting++; } @@ -651,13 +659,8 @@ sub expand_aliases { @bcclist = expand_aliases(@bcclist); if ($thread && !defined $initial_reply_to && $prompting) { - while (1) { - $_= $term->readline("Message-ID to be used as In-Reply-To for the first email? ", $initial_reply_to); - last if defined $_; - print "\n"; - } - - $initial_reply_to = $_; + $initial_reply_to = ask( + "Message-ID to be used as In-Reply-To for the first email? "); } if (defined $initial_reply_to) { $initial_reply_to =~ s/^\s*readline( - "Send this email? ([y]es|[n]o|[q]uit|[a]ll): " - )); - last if /^(?:yes|y|no|n|quit|q|all|a)/i; - print "\n"; - } + $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ", + valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i, + default => $ask_default); + die "Send this email reply required" unless defined $_; if (/^n/i) { return; } elsif (/^q/i) { -- 1.6.2.313.g33352 ``` ## Jay Soffian, 2009-03-29 01:39 Subject: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <1238290751-57461-2-git-send-email-jaysoffian@gmail.com> URL: https://gitlist.dev/e/1238290751-57461-2-git-send-email-jaysoffian%40gmail.com In-Reply-To: <1238290751-57461-1-git-send-email-jaysoffian@gmail.com> ``` Signed-off-by: Jay Soffian --- t/t9001-send-email.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 48 insertions(+), 4 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index e426c96..b4de98c 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -421,8 +421,8 @@ test_confirm () { --from="Example " \ --to=nobody@example.com \ --smtp-server="$(pwd)/fake.sendmail" \ - $@ \ - $patches | grep "Send this email" + $@ $patches > stdout && + grep "Send this email" stdout } test_expect_success '--confirm=always' ' @@ -444,8 +444,10 @@ test_expect_success '--confirm=compose' ' test_expect_success 'confirm by default (due to cc)' ' CONFIRM=$(git config --get sendemail.confirm) && git config --unset sendemail.confirm && - test_confirm && - git config sendemail.confirm $CONFIRM + test_confirm + ret="$?" + git config sendemail.confirm ${CONFIRM:-never} + test $ret = "0" ' test_expect_success 'confirm by default (due to --compose)' ' @@ -457,6 +459,48 @@ test_expect_success 'confirm by default (due to --compose)' ' test $ret = "0" ' +test_expect_success 'confirm detects EOF (inform assumes y)' ' + CONFIRM=$(git config --get sendemail.confirm) && + git config --unset sendemail.confirm && + GIT_SEND_EMAIL_NOTTY=1 \ + git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches < /dev/null + ret="$?" + git config sendemail.confirm ${CONFIRM:-never} + test $ret = "0" +' + +test_expect_success 'confirm detects EOF (auto causes failure)' ' + CONFIRM=$(git config --get sendemail.confirm) && + git config sendemail.confirm auto && + GIT_SEND_EMAIL_NOTTY=1 \ + test_must_fail git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches < /dev/null + ret="$?" + git config sendemail.confirm ${CONFIRM:-never} + test $ret = "0" +' + +test_expect_success 'confirm doesnt loop forever' ' + CONFIRM=$(git config --get sendemail.confirm) && + git config sendemail.confirm auto && + yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \ + test_must_fail git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches + ret="$?" + git config sendemail.confirm ${CONFIRM:-never} + test $ret = "0" +' + test_expect_success '--compose adds MIME for utf8 body' ' clean_fake_sendmail && (echo "#!$SHELL_PATH" && -- 1.6.2.313.g33352 ``` ## Jay Soffian, 2009-03-29 01:48 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903281848l5102afc3x89eed40ed6083dcc@mail.gmail.com> URL: https://gitlist.dev/e/76718490903281848l5102afc3x89eed40ed6083dcc%40mail.gmail.com In-Reply-To: <1238290751-57461-1-git-send-email-jaysoffian@gmail.com> ``` On Sat, Mar 28, 2009 at 9:39 PM, Jay Soffian wrote: > There are three callers of the function: Where by "three" I mean "four". Please use this commit message instead: ---snip--- Several places in send-email prompt for input, and will do so forever when the input is EOF. This is poor behavior when send-email is run unattended (say from cron). This patch refactors the prompting to an ask() function which takes a prompt, an optional default, and an optional regex to validate the input. The function returns on EOF, or if a default is provided and the user simply types return, or if the input passes the validating regex (which accepts all input by default). The ask() function gives up after 10 tries in case of invalid input. There are four callers of the function: 1) "Who should the emails appear to be from?" which provides a default sender. Previously the user would have to type ctrl-d to accept the default. Now the user can just hit return, or type ctrl-d. 2) "Who should the emails be sent to?". Previously this prompt passed a second argument ("") to $term->readline() which was ignored. I believe the intent was to allow the user to just hit return. Now the user can do so, or type ctrl-d. 3) "Message-ID to be used as In-Reply-To for the first email?". Previously this prompt passed a second argument (effectively undef) to $term->readline() which was ignored. I believe the intent was the same as for (2), to allow the user to just hit return. Now the user can do so, or type ctrl-d. 4) "Send this email?". Previously this prompt would loop forever until it got a valid reply. Now it stops prompting on EOF or a valid reply. In the case where confirm = "inform", it now defaults to "y" on EOF or the user hitting return, otherwise an invalid reply causes send-email to terminate. A followup patch adds tests for the new functionality. Signed-off-by: Jay Soffian ---snip--- Thanks, j. ``` ## Junio C Hamano, 2009-03-30 06:50 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <7v3acvjyhw.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v3acvjyhw.fsf%40gitster.siamese.dyndns.org In-Reply-To: <76718490903281848l5102afc3x89eed40ed6083dcc@mail.gmail.com> ``` Jay Soffian writes: > On Sat, Mar 28, 2009 at 9:39 PM, Jay Soffian wrote: >> There are three callers of the function: > > Where by "three" I mean "four". Please use this commit message instead: Thanks. ``` ## Matthieu Moy, 2009-03-30 11:29 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: URL: https://gitlist.dev/e/vpqd4bzjlk1.fsf%40bauges.imag.fr In-Reply-To: <1238290751-57461-1-git-send-email-jaysoffian@gmail.com> ``` Jay Soffian writes: > Several places in send-email prompt for input, and will do so forever > when the input is EOF. This is poor behavior when send-email is run > unattended (say from cron). Thanks a lot for the patch, it does fix the problem I reported in http://article.gmane.org/gmane.comp.version-control.git/114577 . Minor problem: I still (harmless) get error messages in my log: print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 193. readline() on closed filehandle FIN at /usr/share/perl/5.8/Term/ReadLine.pm line 395. print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 203. But I can very well live with them! -- Matthieu ``` ## Jay Soffian, 2009-03-30 14:17 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com> URL: https://gitlist.dev/e/76718490903300717x5cc5e33ax1cf91471efd242bd%40mail.gmail.com In-Reply-To: ``` On Mon, Mar 30, 2009 at 7:29 AM, Matthieu Moy wrote: > Minor problem: I still (harmless) get error messages in my log: > >  print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 193. >  readline() on closed filehandle FIN at /usr/share/perl/5.8/Term/ReadLine.pm line 395. >  print() on closed filehandle FOUT at /usr/share/perl/5.8/Term/ReadLine.pm line 203. > > But I can very well live with them! perl send-email ... 2>/dev/null :-) Seriously though, I am unable to reproduce the messages you are getting from Term::ReadLine, and I tried really hard. What does: $ perl -e 'use Term::ReadLine; print "$Term::ReadLine::VERSION\n"' tell you? Also, thank you for confirming the fix. j. ``` ## Jay Soffian, 2009-03-30 14:18 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903300718m7d172a12y4aab63f0ff75f286@mail.gmail.com> URL: https://gitlist.dev/e/76718490903300718m7d172a12y4aab63f0ff75f286%40mail.gmail.com In-Reply-To: <76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com> ``` On Mon, Mar 30, 2009 at 10:17 AM, Jay Soffian wrote: > perl send-email ... 2>/dev/null :-) Hmfph, perl on the mind. That should of course be "git send-email ... 2>/dev/null" but it was only a half-serious suggestion anyway. j. ``` ## Matthieu Moy, 2009-03-30 14:40 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: URL: https://gitlist.dev/e/vpqwsa7gjlh.fsf%40bauges.imag.fr In-Reply-To: <76718490903300717x5cc5e33ax1cf91471efd242bd@mail.gmail.com> ``` Jay Soffian writes: > On Mon, Mar 30, 2009 at 7:29 AM, Matthieu Moy wrote: > > perl send-email ... 2>/dev/null :-) > > Seriously though, I am unable to reproduce the messages you are > getting from Term::ReadLine, and I tried really hard. Anyway, as I said, the warnings do not harm. They just appear messy in my log files, but it's really a matter of polishing the UI. > What does: > > $ perl -e 'use Term::ReadLine; print "$Term::ReadLine::VERSION\n"' $ perl -e 'use Term::ReadLine; print "$Term::ReadLine::VERSION\n"' 1.02 $ That's Debian oldstable (etch), so perhaps newer versions fixed the problem. Thanks again, -- Matthieu ``` ## Jay Soffian, 2009-03-30 15:45 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903300845x14897599jc73f50a16b21605d@mail.gmail.com> URL: https://gitlist.dev/e/76718490903300845x14897599jc73f50a16b21605d%40mail.gmail.com In-Reply-To: ``` On Mon, Mar 30, 2009 at 10:40 AM, Matthieu Moy wrote: > Jay Soffian writes: >> Seriously though, I am unable to reproduce the messages you are >> getting from Term::ReadLine, and I tried really hard. > > Anyway, as I said, the warnings do not harm. They just appear messy in > my log files, but it's really a matter of polishing the UI. Okay, well, I figured out how to work the polish. Term::ReadLine is attempting to use /dev/tty for input/output, which is closed. And because send-email enables warnings, its attempt to do so emits the messages you are seeing. Can you confirm that this patch squelches the warnings? diff --git a/git-send-email.perl b/git-send-email.perl index f0405c8..1b17728 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -612,6 +612,8 @@ sub ask { my $default = $arg{default}; my $resp; my $i = 0; + return undef unless defined fileno($term->IN) and + defined fileno($term->OUT); while ($i++ < 10) { $resp = $term->readline($prompt); if (!defined $resp) { # EOF Thanks, j. ``` ## Jay Soffian, 2009-03-30 16:04 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com> URL: https://gitlist.dev/e/76718490903300904k7e8c1054m99edb92d8d582387%40mail.gmail.com In-Reply-To: <76718490903300845x14897599jc73f50a16b21605d@mail.gmail.com> ``` On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian wrote: > Okay, well, I figured out how to work the polish. Term::ReadLine is > attempting to use /dev/tty for input/output, which is closed. And > because send-email enables warnings, its attempt to do so emits the > messages you are seeing. Can you confirm that this patch squelches the > warnings? Ugh, not that. This: diff --git a/git-send-email.perl b/git-send-email.perl index f0405c8..81240ef 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -612,6 +612,9 @@ sub ask { my $default = $arg{default}; my $resp; my $i = 0; + return defined $default ? $default : undef + unless defined $term->IN and defined fileno($term->IN) and + defined $term->OUT and defined fileno($term->OUT); while ($i++ < 10) { $resp = $term->readline($prompt); if (!defined $resp) { # EOF j. ``` ## Matthieu Moy, 2009-03-31 09:32 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: URL: https://gitlist.dev/e/vpqvdpq9gww.fsf%40bauges.imag.fr In-Reply-To: <76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com> ``` Jay Soffian writes: > On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian wrote: >> Okay, well, I figured out how to work the polish. Term::ReadLine is >> attempting to use /dev/tty for input/output, which is closed. And >> because send-email enables warnings, its attempt to do so emits the >> messages you are seeing. Can you confirm that this patch squelches the >> warnings? > > Ugh, not that. This: > > diff --git a/git-send-email.perl b/git-send-email.perl > index f0405c8..81240ef 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -612,6 +612,9 @@ sub ask { > my $default = $arg{default}; > my $resp; > my $i = 0; > + return defined $default ? $default : undef > + unless defined $term->IN and defined fileno($term->IN) and > + defined $term->OUT and defined fileno($term->OUT); > while ($i++ < 10) { > $resp = $term->readline($prompt); > if (!defined $resp) { # EOF Surprising. I did the test with 3 commits to send (titled "hello", "hello-again" and "hello-oncemore"). the log says this Send this email reply required at /home/moy/local/usr/libexec/git-core//git-send-email line 866. OK. Log says: Sendmail: /usr/sbin/sendmail -i Matthieu.Moy@imag.fr From: Matthieu.Moy@imag.fr To: Matthieu.Moy@imag.fr Subject: [PATCH 1/3] hello Date: Tue, 31 Mar 2009 10:47:59 +0200 Message-Id: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr> X-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty Result: OK (mbox) Adding cc: moy from line 'From: moy ' From: Matthieu.Moy@imag.fr To: Matthieu.Moy@imag.fr Subject: [PATCH 2/3] hello-again Date: Tue, 31 Mar 2009 10:48:00 +0200 Message-Id: <1238489281-5518-2-git-send-email-Matthieu.Moy@imag.fr> X-Mailer: git-send-email 1.6.2.1.413.gf2ad.dirty In-Reply-To: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr> References: <1238489281-5518-1-git-send-email-Matthieu.Moy@imag.fr> (line 866 is this: die "Send this email reply required" unless defined $_;) And I received only patch 1/3. Actually, this seems to be a totally separate issue. What happens is that for the first email, the script executes : if ($needs_confirm eq "inform") { $confirm_unconfigured = 0; # squelch this message for the rest of this run $ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation ... (show message, ...) } $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ", valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i, default => $ask_default); and the second time, since $confirm_unconfigured = 0 has been set, the message is not shown again, _and_ $ask_default not set (since $needs_confirm is set according to $confirm_unconfigured). I think the solution is to use a variable different from $confirm_unconfigured to say whether the message has already been displayed, along the lines of: diff --git a/git-send-email.perl b/git-send-email.perl index 5916c86..9e36c35 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -357,6 +357,7 @@ if ($confirm_unconfigured) { }; die "Unknown --confirm setting: '$confirm'\n" unless $confirm =~ /^(?:auto|cc|compose|always|never)/; +my $confirm_msg_shown = undef; # Debugging, print out the suppressions. if (0) { @@ -844,17 +849,21 @@ X-Mailer: git-send-email $gitversion print "\n$header\n"; my $ask_default; if ($needs_confirm eq "inform") { - $confirm_unconfigured = 0; # squelch this message for the rest of this run $ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation - print " The Cc list above has been expanded by additional\n"; - print " addresses found in the patch commit message. By default\n"; - print " send-email prompts before sending whenever this occurs.\n"; - print " This behavior is controlled by the sendemail.confirm\n"; - print " configuration setting.\n"; - print "\n"; - print " For additional information, run 'git send-email --help'.\n"; - print " To retain the current behavior, but squelch this message,\n"; - print " run 'git config --global sendemail.confirm auto'.\n\n"; + if (!defined $confirm_msg_shown) { + $confirm_msg_shown = 0; # squelch this message for the rest of this run + print " The Cc list above has been expanded by additional\n"; + print " addresses found in the patch commit message. By default\n"; + print " send-email prompts before sending whenever this occurs.\n"; + print " This behavior is controlled by the sendemail.confirm\n"; + print " configuration setting.\n"; + print "\n"; + print " For additional information, run 'git send-email --help'.\n"; + print " To retain the current behavior, but squelch this message,\n"; + print " run 'git config --global sendemail.confirm auto'.\n\n"; + } } $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ", valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i, (worksforme at least) -- Matthieu ``` ## Björn Steinbrink, 2009-03-31 10:33 Subject: Re: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <20090331103303.GD3307@atjola.homenet> URL: https://gitlist.dev/e/20090331103303.GD3307%40atjola.homenet In-Reply-To: <1238290751-57461-2-git-send-email-jaysoffian@gmail.com> ``` On 2009.03.28 21:39:11 -0400, Jay Soffian wrote: > +test_expect_success 'confirm detects EOF (auto causes failure)' ' > + CONFIRM=$(git config --get sendemail.confirm) && > + git config sendemail.confirm auto && > + GIT_SEND_EMAIL_NOTTY=1 \ > + test_must_fail git send-email \ > + --from="Example " \ > + --to=nobody@example.com \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + $patches < /dev/null > + ret="$?" > + git config sendemail.confirm ${CONFIRM:-never} > + test $ret = "0" > +' > + > +test_expect_success 'confirm doesnt loop forever' ' > + CONFIRM=$(git config --get sendemail.confirm) && > + git config sendemail.confirm auto && > + yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \ > + test_must_fail git send-email \ > + --from="Example " \ > + --to=nobody@example.com \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + $patches > + ret="$?" > + git config sendemail.confirm ${CONFIRM:-never} > + test $ret = "0" These two cause interactive prompts for me. Björn ``` ## Jay Soffian, 2009-03-31 13:58 Subject: Re: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <76718490903310658w2dffdfa4y4f40e0757cac8034@mail.gmail.com> URL: https://gitlist.dev/e/76718490903310658w2dffdfa4y4f40e0757cac8034%40mail.gmail.com In-Reply-To: <20090331103303.GD3307@atjola.homenet> ``` 2009/3/31 Björn Steinbrink : >> +     GIT_SEND_EMAIL_NOTTY=1 \ >> +             test_must_fail git send-email \ >> [...] >> +     yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \ >> +             test_must_fail git send-email \ > > These two cause interactive prompts for me. Hmpfh. I think the only way that's possible is if GIT_SEND_EMAIL_NOTTY isn't being set by the shell. But I'm not sure why that's unique to these tests. Thanks for the report. j. ``` ## Jay Soffian, 2009-03-31 14:07 Subject: Re: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <76718490903310707s75e24e39l63f0d396f60a0583@mail.gmail.com> URL: https://gitlist.dev/e/76718490903310707s75e24e39l63f0d396f60a0583%40mail.gmail.com In-Reply-To: <20090331103303.GD3307@atjola.homenet> ``` 2009/3/31 Björn Steinbrink : > These two cause interactive prompts for me. Ah, it's because of going through test_must_fail. Grrr, a test farm for git would be nice. :-) This should fix it up: diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index b4de98c..cd34525 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -476,7 +476,8 @@ test_expect_success 'confirm detects EOF (inform assumes y)' ' test_expect_success 'confirm detects EOF (auto causes failure)' ' CONFIRM=$(git config --get sendemail.confirm) && git config sendemail.confirm auto && - GIT_SEND_EMAIL_NOTTY=1 \ + GIT_SEND_EMAIL_NOTTY=1 && + export GIT_SEND_EMAIL_NOTTY && test_must_fail git send-email \ --from="Example " \ --to=nobody@example.com \ @@ -490,8 +491,9 @@ test_expect_success 'confirm detects EOF (auto causes failure)' ' test_expect_success 'confirm doesnt loop forever' ' CONFIRM=$(git config --get sendemail.confirm) && git config sendemail.confirm auto && - yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \ - test_must_fail git send-email \ + GIT_SEND_EMAIL_NOTTY=1 && + export GIT_SEND_EMAIL_NOTTY && + yes "bogus" | test_must_fail git send-email \ --from="Example " \ --to=nobody@example.com \ --smtp-server="$(pwd)/fake.sendmail" \ What's your OS and test shell btw? j. ``` ## Jay Soffian, 2009-03-31 14:12 Subject: Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever Message-ID: <76718490903310712w540205b4n3826d2d9277853fa@mail.gmail.com> URL: https://gitlist.dev/e/76718490903310712w540205b4n3826d2d9277853fa%40mail.gmail.com In-Reply-To: ``` On Tue, Mar 31, 2009 at 5:32 AM, Matthieu Moy wrote: > Actually, this seems to be a totally separate issue. Argh, I need more sleep these days. j. ``` ## Björn Steinbrink, 2009-03-31 14:19 Subject: Re: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <20090331141937.GC14611@atjola.homenet> URL: https://gitlist.dev/e/20090331141937.GC14611%40atjola.homenet In-Reply-To: <76718490903310707s75e24e39l63f0d396f60a0583@mail.gmail.com> ``` On 2009.03.31 10:07:25 -0400, Jay Soffian wrote: > 2009/3/31 Björn Steinbrink : > > These two cause interactive prompts for me. > > Ah, it's because of going through test_must_fail. Grrr, a test farm > for git would be nice. :-) > > This should fix it up: > > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index b4de98c..cd34525 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -476,7 +476,8 @@ test_expect_success 'confirm detects EOF (inform > assumes y)' ' > test_expect_success 'confirm detects EOF (auto causes failure)' ' > CONFIRM=$(git config --get sendemail.confirm) && > git config sendemail.confirm auto && > - GIT_SEND_EMAIL_NOTTY=1 \ > + GIT_SEND_EMAIL_NOTTY=1 && > + export GIT_SEND_EMAIL_NOTTY && > test_must_fail git send-email \ > --from="Example " \ > --to=nobody@example.com \ > @@ -490,8 +491,9 @@ test_expect_success 'confirm detects EOF (auto > causes failure)' ' > test_expect_success 'confirm doesnt loop forever' ' > CONFIRM=$(git config --get sendemail.confirm) && > git config sendemail.confirm auto && > - yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \ > - test_must_fail git send-email \ > + GIT_SEND_EMAIL_NOTTY=1 && > + export GIT_SEND_EMAIL_NOTTY && > + yes "bogus" | test_must_fail git send-email \ > --from="Example " \ > --to=nobody@example.com \ > --smtp-server="$(pwd)/fake.sendmail" \ Yep, after fixing the linewrapping and restoring the tabs, this works. > What's your OS and test shell btw? Debian sid, kernel 2.6.29, dash as /bin/sh Thanks, Björn ``` ## Jay Soffian, 2009-03-31 14:36 Subject: Re: [PATCH 2/2] send-email: add tests for refactored prompting Message-ID: <76718490903310736r41acc92cg9b09aff902f9850e@mail.gmail.com> URL: https://gitlist.dev/e/76718490903310736r41acc92cg9b09aff902f9850e%40mail.gmail.com In-Reply-To: <20090331141937.GC14611@atjola.homenet> ``` 2009/3/31 Björn Steinbrink : > Yep, after fixing the linewrapping and restoring the tabs, this works. Thanks (sorry about the formatting, gmail...). > Debian sid, kernel 2.6.29, dash as /bin/sh dash seems to be a good lowest common denominator. I'll try testing with it from now on. j. ```