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>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Mike Hommey" <mh@glandium.org>,
	"Uwe Kleine-König" <ukleinek@informatik.uni-freiburg.de>,
	"Len Brown" <lenb@kernel.org>
Subject: [PATCH] send-email: fix In-Reply-To regression
Date: Thu, 21 Feb 2008 19:16:04 -0500	[thread overview]
Message-ID: <1203639364-91817-1-git-send-email-jaysoffian@gmail.com> (raw)
In-Reply-To: <76718490802211351n1f0a6c36nb84902a624ee2120@mail.gmail.com>

Fix a regression introduced by

1ca3d6e (send-email: squelch warning due to comparing undefined $_ to "")

where if the user was prompted for an initial In-Reply-To and didn't
provide one, messages would be sent out with an invalid In-Reply-To of
"<>"

Also add test cases for the regression and the fix. A small modification
was needed to allow send-email to take its replies from stdin if the
environment variable GIT_SEND_EMAIL_NOTTY is set.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---

An issue cropped up with testing the regression: it only occured if
initial_reply_to was "", and the only way to do that is to respond to
the Initial-Reply-To prompt with "\n". But send-email uses readline,
which insists on /dev/tty.

So, I initially simulated the breakage by using --initial-reply-to=" ",
but this isn't quite the same thing. However, it turns out this can also
cause an invalid In-Reply-To, which wasn't fixed by Junio's suggestion
of checking that initial_reply_to ne "". So I did a slightly different
fix that handles initial_reply_to eq  " ".

Anyway, back to testing the original problem. I didn't want to use
expect or Expect.pm, so I made a small change to send-email where if
GIT_SEND_EMAIL_NOTTY is set, it will read from stdin instead of
/dev/tty.

But, I wonder if in the future we'll want to enhance the testing
framework to be able to do expect-type stuff?

j.

 git-send-email.perl   |    9 ++++++---
 t/t9001-send-email.sh |   21 +++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ccb87a2..29b1105 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -170,7 +170,9 @@ my $envelope_sender;
 
 my $repo = Git->repository();
 my $term = eval {
-	new Term::ReadLine 'git-send-email';
+	$ENV{"GIT_SEND_EMAIL_NOTTY"}
+		? new Term::ReadLine 'git-send-email', \*STDIN, \*STDOUT
+		: new Term::ReadLine 'git-send-email';
 };
 if ($@) {
 	$term = new FakeTerm "$@: going non-interactive";
@@ -476,8 +478,9 @@ if ($thread && !defined $initial_reply_to && $prompting) {
 	$initial_reply_to = $_;
 }
 if (defined $initial_reply_to) {
-	$initial_reply_to =~ s/^\s*<?/</;
-	$initial_reply_to =~ s/>?\s*$/>/;
+	$initial_reply_to =~ s/^\s*<?//;
+	$initial_reply_to =~ s/>?\s*$//;
+	$initial_reply_to = "<$initial_reply_to>" if $initial_reply_to ne '';
 }
 
 if (!defined $smtp_server) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 08f7c3d..1422e9f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -108,4 +108,25 @@ test_expect_success 'allow long lines with --no-validate' '
 		2>errors
 '
 
+test_expect_failure 'Invalid In-Reply-To' '
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--in-reply-to=" " \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches
+		2>errors
+	! grep "^In-Reply-To: < *>" msgtxt
+'
+
+test_expect_success 'Valid In-Reply-To when prompting' '
+	(echo "From Example <from@example.com>"
+	 echo "To Example <to@example.com>"
+	 echo ""
+	) | env GIT_SEND_EMAIL_NOTTY=1 git send-email \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches 2>errors &&
+	! grep "^In-Reply-To: < *>" msgtxt
+'
+
 test_done
-- 
1.5.4.2.236.g77b4.dirty

  reply	other threads:[~2008-02-22  0:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-21  9:29 git-send-email getting filtered as spam by vger.kernel.org Len Brown
2008-02-21 12:21 ` Len Brown
2008-02-21 20:51   ` Uwe Kleine-König
2008-02-21 21:14     ` Junio C Hamano
2008-02-21 21:34       ` Mike Hommey
2008-02-21 21:41       ` Jay Soffian
2008-02-21 21:49         ` Mike Hommey
2008-02-21 21:51           ` Jay Soffian
2008-02-22  0:16             ` Jay Soffian [this message]
2008-02-22  1:18               ` [PATCH] send-email: fix In-Reply-To regression Junio C Hamano
2008-02-27 11:13                 ` Uwe Kleine-König
2008-02-27 19:48                   ` Junio C Hamano
2008-02-21 22:05           ` git-send-email getting filtered as spam by vger.kernel.org Junio C Hamano
2008-02-23  6:23   ` Len Brown

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=1203639364-91817-1-git-send-email-jaysoffian@gmail.com \
    --to=jaysoffian@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=lenb@kernel.org \
    --cc=mh@glandium.org \
    --cc=ukleinek@informatik.uni-freiburg.de \
    /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).