git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jay Soffian <jaysoffian@gmail.com>
To: git@vger.kernel.org
Cc: Jay Soffian <jaysoffian@gmail.com>,
	Bruce Stephens <bruce.stephens@isode.com>,
	Dan McGee <dpmcgee@gmail.com>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] send-email: fix nasty bug in ask() function
Date: Sat,  4 Apr 2009 23:23:21 -0400	[thread overview]
Message-ID: <1238901801-47109-1-git-send-email-jaysoffian@gmail.com> (raw)
In-Reply-To: <449c10960904041002s22124b74k8440af216b1de9ee@mail.gmail.com>

Commit 6e18251 (send-email: refactor and ensure prompting doesn't loop
forever) introduced an ask function, which unfortunately had a nasty
bug. This caused it not to accept anything but the default reply to the
"Who should the emails appear to be from?" prompt, and nothing but
ctrl-d to the "Who should the emails be sent to?" and "Message-ID to be
used as In-Reply-To for the first email?" prompts.

This commit corrects the issues and adds a test to confirm the fix.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Sat, Apr 4, 2009 at 1:02 PM, Dan McGee <dpmcgee@gmail.com> wrote:
> I'm guessing this is related to commit
> 6e1825186bd052fc1f77b7c8c9a31fbb9a67d90c but I haven't bisected yet.
> Having to hit enter 10 times ad the Message-ID prompt seemed a bit odd
> to me. Has anyone else seen this behavior?
>
> dmcgee@galway ~/projects/git (working)
> $ git send-email 000*
> 0001-git-repack-use-non-dashed-update-server-info.patch
> 0002-pack-objects-report-actual-number-of-threads-to-be.patch
> Who should the emails appear to be from? [Dan McGee <dpmcgee@gmail.com>]
> Emails will be sent from: Dan McGee <dpmcgee@gmail.com>
> Message-ID to be used as In-Reply-To for the first email?
> Message-ID to be used as In-Reply-To for the first email?

I really apologize for this breakage. This patch should fix the issue.
I'm quite surprised that there wasn't already a test for the prompting,
but shame on me for not double-checking before refactoring.

I'm also super confused why the issue is occuring. You can see from the
patch below that by default the ask() function attempted to match the
user's input against the empty regex //, which should match anything:

$ perl -e 'use strict; my $resp="something"; my $re=""; print "true\n" if $resp =~ /$re/'
true
$ perl -e 'use strict; my $resp=""; my $re=""; print "true\n" if $resp =~ /$re/'
true

Any yet while my demonstration above works as I expect, for some reason
what is basically the same code (AFAICT) in send-email does not match. I
thought I knew my perl fairly well, but maybe some perl guru can see
what's going wrong.

 git-send-email.perl   |    4 ++--
 t/t9001-send-email.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 6bbdfec..172b53c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -608,7 +608,7 @@ EOT
 
 sub ask {
 	my ($prompt, %arg) = @_;
-	my $valid_re = $arg{valid_re} || ""; # "" matches anything
+	my $valid_re = $arg{valid_re};
 	my $default = $arg{default};
 	my $resp;
 	my $i = 0;
@@ -624,7 +624,7 @@ sub ask {
 		if ($resp eq '' and defined $default) {
 			return $default;
 		}
-		if ($resp =~ /$valid_re/) {
+		if (!defined $valid_re or $resp =~ /$valid_re/) {
 			return $resp;
 		}
 	}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 192b97b..3c90c4f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -130,6 +130,19 @@ test_expect_success 'Show all headers' '
 	test_cmp expected-show-all-headers actual-show-all-headers
 '
 
+test_expect_success 'Prompting works' '
+	clean_fake_sendmail &&
+	(echo "Example <from@example.com>"
+	 echo "to@example.com"
+	 echo ""
+	) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches \
+		2>errors &&
+		grep "^From: Example <from@example.com>$" msgtxt1 &&
+		grep "^To: to@example.com$" msgtxt1
+'
+
 z8=zzzzzzzz
 z64=$z8$z8$z8$z8$z8$z8$z8$z8
 z512=$z64$z64$z64$z64$z64$z64$z64$z64
-- 
1.6.2.2.405.g6d8cc4

  parent reply	other threads:[~2009-04-05  3:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-04 17:02 git send-email prompting too much Dan McGee
2009-04-04 17:12 ` Bruce Stephens
2009-04-05  2:13   ` Jay Soffian
2009-04-05  2:49     ` Jay Soffian
2009-04-05  9:45       ` Junio C Hamano
2009-04-05  3:23 ` Jay Soffian [this message]
2009-04-05  3:31   ` [PATCH] send-email: fix nasty bug in ask() function Jay Soffian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1238901801-47109-1-git-send-email-jaysoffian@gmail.com \
    --to=jaysoffian@gmail.com \
    --cc=bruce.stephens@isode.com \
    --cc=dpmcgee@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).