Re: [PATCH 1/2] send-email: refactor and ensure prompting doesn't loop forever

From: Matthieu Moy <matthieu.moy@imag.fr>
Date: Mar 31, 2009, 09:32 UTC
Message-ID: <vpqvdpq9gww.fsf@bauges.imag.fr>
In-Reply-To: <76718490903300904k7e8c1054m99edb92d8d582387@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 23 quoted lines

> On Mon, Mar 30, 2009 at 11:45 AM, Jay Soffian <jaysoffian@gmail.com> 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 <moy@973704de-e02c-4a59-87bb-087b52aa604b> from line 'From: moy <moy@973704de-e02c-4a59-87bb-087b52aa604b>'

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