[PATCH 0/6] fixes for git-send-email

[PATCH 0/6] fixes for git-send-email

[Markus Heidelberg]

Markus Heidelberg (6):
  send-email: fix a typo in a comment
  add a test for git-send-email for threaded mails without
    chain-reply-to
  send-email: fix threaded mails without chain-reply-to
  add a test for git-send-email for non-threaded mails
  send-email: fix non-threaded mails
  doc/send-email: clarify the behavior of --in-reply-to with
    --no-thread

 Documentation/git-send-email.txt |    5 +++--
 git-send-email.perl              |    7 +++++--
 t/t9001-send-email.sh            |   21 +++++++++++++++++++++
 3 files changed, 29 insertions(+), 4 deletions(-)

[PATCH 1/6] send-email: fix a typo in a comment

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3d6a982..5b7ab4e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -804,7 +804,7 @@ sub sanitize_address
 }
 
 # Returns 1 if the message was sent, and 0 otherwise.
-# In actuality, the whole program dies when a there
+# In actuality, the whole program dies when there
 # is an error sending a message.
 
 sub send_message
-- 
1.6.3.2.221.g0ff2f

[PATCH 2/6] add a test for git-send-email for threaded mails without chain-reply-to

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 t/t9001-send-email.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ce26ea4..4ccca44 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'threading but no chain-reply-to' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--thread \
+		--nochain-reply-to \
+		$patches $patches |
+	grep "In-Reply-To: "
+'
+
 test_done
-- 
1.6.3.2.221.g0ff2f

[PATCH 3/6] send-email: fix threaded mails without chain-reply-to

[Markus Heidelberg]

These commands didn't send threaded mails anymore:

    $ git format-patch <revision range>
    $ git send-email --thread --no-chain-reply-to <files>

This regression was introduced in commit 15da108 ("send-email:
'References:' should only reference what is sent", 2009-04-13) by a
hidden code style change:

    ! defined $reply_to || length($reply_to) == 0
was changed to
    not defined $reply_to || length($reply_to) == 0
which is
    not (defined $reply_to || length($reply_to) == 0)
instead of
    (not defined $reply_to) || (length($reply_to) == 0)

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-send-email.perl   |    2 +-
 t/t9001-send-email.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5b7ab4e..c11a245 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1142,7 +1142,7 @@ foreach my $t (@files) {
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent and $chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4ccca44..41af35a 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'threading but no chain-reply-to' '
+test_expect_success 'threading but no chain-reply-to' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.221.g0ff2f

[PATCH 4/6] add a test for git-send-email for non-threaded mails

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 41af35a..9cddd5f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,6 +621,16 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	! git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches |
+	grep "In-Reply-To: "
+'
+
 test_expect_success 'threading but no chain-reply-to' '
 	git send-email \
 		--dry-run \
-- 
1.6.3.2.221.g0ff2f

[PATCH 5/6] send-email: fix non-threaded mails

[Markus Heidelberg]

After commit 3e0c4ff (send-email: respect in-reply-to regardless of
threading, 2009-03-01) the variable $thread was only used for prompting
for an "In-Reply-To", but not for controlling whether the "In-Reply-To"
and "References" fields should be written into the email.

Thus these fields were always used beginning with the second mail and it
was not possible to produce non-threaded mails anymore until commit
15da108 ("send-email: 'References:' should only reference what is sent",
2009-04-13), which introduced a regression with the side effect to make
it possible again when --no-chain-reply-to was set.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-send-email.perl   |    5 ++++-
 t/t9001-send-email.sh |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index c11a245..be63ea7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1142,7 +1142,10 @@ foreach my $t (@files) {
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($message_was_sent and $chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent and $thread and
+			$chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+		# with --chain-reply-to every time
+		# else only after the first mail (and only if --in-reply-to was not specified)
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9cddd5f..12433a7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'no in-reply-to and no threading' '
+test_expect_success 'no in-reply-to and no threading' '
 	! git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.221.g0ff2f

[PATCH 6/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread

[Markus Heidelberg]

Also remove the argument from --[no-]chain-reply-to.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 Documentation/git-send-email.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 7c5ce41..dff90f0 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -161,7 +161,7 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to=<identifier>::
+--[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -210,7 +210,8 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 --[no-]thread::
 	If this is set, the In-Reply-To header will be set on each email sent.
 	If disabled with "--no-thread", no emails will have the In-Reply-To
-	header set. Default is the value of the 'sendemail.thread' configuration
+	header set, unless specified with --in-reply-to.
+	Default is the value of the 'sendemail.thread' configuration
 	value; if that is unspecified, default to --thread.
 
 
-- 
1.6.3.2.221.g0ff2f

Re: [PATCH 2/6] add a test for git-send-email for threaded mails without chain-reply-to

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> +test_expect_failure 'threading but no chain-reply-to' '
> +	git send-email \
> +		--dry-run \
> +		--from="Example <nobody@example.com>" \
> +		--to=nobody@example.com \
> +		--thread \
> +		--nochain-reply-to \
> +		$patches $patches |
> +	grep "In-Reply-To: "
> +'

Thanks, but this is not a very good style, as it won't catch if "git
send-email" dumps core or otherwise fails, exiting with a non-zero status.

Same comments applies to [PATCH 4/6] as well.

Re: [PATCH 2/6] add a test for git-send-email for threaded mails without chain-reply-to

[Markus Heidelberg]

Junio C Hamano, 08.06.2009:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > +test_expect_failure 'threading but no chain-reply-to' '
> > +	git send-email \
> > +		--dry-run \
> > +		--from="Example <nobody@example.com>" \
> > +		--to=nobody@example.com \
> > +		--thread \
> > +		--nochain-reply-to \
> > +		$patches $patches |
> > +	grep "In-Reply-To: "
> > +'
> 
> Thanks, but this is not a very good style, as it won't catch if "git
> send-email" dumps core or otherwise fails, exiting with a non-zero status.
> 
> Same comments applies to [PATCH 4/6] as well.

OK, this was my first attempt in writing a test and I just used the last
test 'in-reply-to but no threading' as template.

What can be done to make the test better?

[PATCH v2 2/6] add a test for git-send-email for threaded mails without chain-reply-to

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

Markus Heidelberg, 08.06.2009:
> Junio C Hamano, 08.06.2009:
> > Markus Heidelberg <markus.heidelberg@web.de> writes:
> > 
> > > +test_expect_failure 'threading but no chain-reply-to' '
> > > +	git send-email \
> > > +		--dry-run \
> > > +		--from="Example <nobody@example.com>" \
> > > +		--to=nobody@example.com \
> > > +		--thread \
> > > +		--nochain-reply-to \
> > > +		$patches $patches |
> > > +	grep "In-Reply-To: "
> > > +'
> > 
> > Thanks, but this is not a very good style, as it won't catch if "git
> > send-email" dumps core or otherwise fails, exiting with a non-zero status.
> > 
> > Same comments applies to [PATCH 4/6] as well.
> 
> OK, this was my first attempt in writing a test and I just used the last
> test 'in-reply-to but no threading' as template.
> 
> What can be done to make the test better?

Is this change sufficient? Dumping the stdout into a file and grepping
this.


 t/t9001-send-email.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ce26ea4..8f810ab 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'threading but no chain-reply-to' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--thread \
+		--nochain-reply-to \
+		$patches $patches >stdout &&
+	grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.220.gb421

Re: [PATCH v2 4/6] add a test for git-send-email for non-threaded mails

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

Differences to v1:
* dump stdout into a file and grep this
* use "test_must_fail grep" instead of "!git send-email"
  I'm not sure about this. When should test_must_fail be used and when
  is !command OK?

 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 990dd1a..4a4fa7f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,6 +621,16 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches >stdout &&
+	test_must_fail grep "In-Reply-To: " stdout
+'
+
 test_expect_success 'threading but no chain-reply-to' '
 	git send-email \
 		--dry-run \
-- 
1.6.3.2.220.gb421

[PATCH v2 5/6] send-email: fix non-threaded mails

[Markus Heidelberg]

After commit 3e0c4ff (send-email: respect in-reply-to regardless of
threading, 2009-03-01) the variable $thread was only used for prompting
for an "In-Reply-To", but not for controlling whether the "In-Reply-To"
and "References" fields should be written into the email.

Thus these fields were always used beginning with the second mail and it
was not possible to produce non-threaded mails anymore until commit
15da108 ("send-email: 'References:' should only reference what is sent",
2009-04-13), which introduced a regression with the side effect to make
it possible again when --no-chain-reply-to was set.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

Differences to v1:
* patch context in the test:
  "git send-email" instead of "!git send-email"

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

diff --git a/git-send-email.perl b/git-send-email.perl
index c11a245..be63ea7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1142,7 +1142,10 @@ foreach my $t (@files) {
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($message_was_sent and $chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent and $thread and
+			$chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+		# with --chain-reply-to every time
+		# else only after the first mail (and only if --in-reply-to was not specified)
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4a4fa7f..87bbbe7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'no in-reply-to and no threading' '
+test_expect_success 'no in-reply-to and no threading' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.220.gb421

Re: [PATCH v2 2/6] add a test for git-send-email for threaded mails without chain-reply-to

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

>> What can be done to make the test better?
>
> Is this change sufficient? Dumping the stdout into a file and grepping
> this.

Yup, that is better for test scripts.

>
>
>  t/t9001-send-email.sh |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index ce26ea4..8f810ab 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' '
>  	grep "In-Reply-To: <in-reply-id@example.com>"
>  '
>  
> +test_expect_failure 'threading but no chain-reply-to' '
> +	git send-email \
> +		--dry-run \
> +		--from="Example <nobody@example.com>" \
> +		--to=nobody@example.com \
> +		--thread \
> +		--nochain-reply-to \
> +		$patches $patches >stdout &&
> +	grep "In-Reply-To: " stdout
> +'
> +
>  test_done
> -- 
> 1.6.3.2.220.gb421

Re: [PATCH v2 4/6] add a test for git-send-email for non-threaded mails

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---
>
> Differences to v1:
> * dump stdout into a file and grep this
> * use "test_must_fail grep" instead of "!git send-email"
>   I'm not sure about this. When should test_must_fail be used and when
>   is !command OK?

Thanks.

Generally, we protect "git" (i.e. what we write ourselves) with test_must_fail
so that we can catch stupid segfaulting crash we introduce ourselves
(unlike "! git foo", "test_must_fail git foo" says "oh, no, it did not
correctly fail" if git segfaults).  We do not expect "grep" to segfault
(iow we are not testing "grep"), so it is customary to say "! grep".

[PATCH v3 4/6] add a test for git-send-email for non-threaded mails

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---

Junio C Hamano, 08.06.2009:
> Generally, we protect "git" (i.e. what we write ourselves) with test_must_fail
> so that we can catch stupid segfaulting crash we introduce ourselves
> (unlike "! git foo", "test_must_fail git foo" says "oh, no, it did not
> correctly fail" if git segfaults).  We do not expect "grep" to segfault
> (iow we are not testing "grep"), so it is customary to say "! grep".

Thanks for the explanation.

Differences to v2:
* "! grep" instead of "test_must_fail grep"


 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 990dd1a..20efdc1 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,6 +621,16 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches >stdout &&
+	! grep "In-Reply-To: " stdout
+'
+
 test_expect_success 'threading but no chain-reply-to' '
 	git send-email \
 		--dry-run \
-- 
1.6.3.2.224.gaa543

Re: [PATCH 1/6] send-email: fix a typo in a comment

[Junio C Hamano]

Thanks; is there any patch among the six that should go to 'maint' (aka
1.6.3.X)?

Re: [PATCH 1/6] send-email: fix a typo in a comment

[Markus Heidelberg]

Junio C Hamano, 08.06.2009:
> Thanks; is there any patch among the six that should go to 'maint' (aka
> 1.6.3.X)?

[PATCH 4/6] add a test for git-send-email for non-threaded mails
[PATCH 5/6] send-email: fix non-threaded mails

fix the regression from commit 3e0c4ff (send-email: respect in-reply-to
regardless of threading, 2009-03-01), which was included before 1.6.3
and so is included in 'maint', so they should go to 'maint'. I'll resend
rebased.

[PATCH 1/6] send-email: fix a typo in a comment
[PATCH 2/6] add a test for git-send-email for threaded mails without chain-reply-to
[PATCH 3/6] send-email: fix threaded mails without chain-reply-to

fix commit 15da108 ("send-email: 'References:' should only reference
what is sent", 2009-04-13), which was included after 1.6.3 directly to
'master', so they don't go to 'maint'.

I don't know how documentation fixes are handled for inclusion to
'maint', but this could apply to 'maint' as well:

[PATCH 6/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread

[PATCH 1/2] add a test for git-send-email for non-threaded mails

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ce26ea4..5bfa36e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,14 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches >stdout &&
+	! grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.8.g055bf

[PATCH 2/2] send-email: fix non-threaded mails

[Markus Heidelberg]

After commit 3e0c4ff (send-email: respect in-reply-to regardless of
threading, 2009-03-01) the variable $thread was only used for prompting
for an "In-Reply-To", but not for controlling whether the "In-Reply-To"
and "References" fields should be written into the email.

Thus these fields were always used beginning with the second mail and it
was not possible to produce non-threaded mails anymore until commit
15da108 ("send-email: 'References:' should only reference what is sent",
2009-04-13), which introduced a regression with the side effect to make
it possible again when --no-chain-reply-to was set.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 git-send-email.perl   |    5 ++++-
 t/t9001-send-email.sh |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index cccbf45..3f31b7b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1137,7 +1137,10 @@ foreach my $t (@files) {
 	send_message();
 
 	# set up for the next message
-	if ($chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($thread and
+			$chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+		# with --chain-reply-to every time
+		# else only after the first mail (and only if --in-reply-to was not specified)
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5bfa36e..8518aca 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'no in-reply-to and no threading' '
+test_expect_success 'no in-reply-to and no threading' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.8.g055bf

Re: [PATCH 3/6] send-email: fix threaded mails without chain-reply-to

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> These commands didn't send threaded mails anymore:
>
>     $ git format-patch <revision range>
>     $ git send-email --thread --no-chain-reply-to <files>
>
> This regression was introduced in commit 15da108 ("send-email:
> 'References:' should only reference what is sent", 2009-04-13) by a
> hidden code style change:
>
>     ! defined $reply_to || length($reply_to) == 0
> was changed to
>     not defined $reply_to || length($reply_to) == 0
> which is
>     not (defined $reply_to || length($reply_to) == 0)
> instead of
>     (not defined $reply_to) || (length($reply_to) == 0)
>
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ...
> -	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
> +	if ($message_was_sent and $chain_reply_to || !defined $reply_to || length($reply_to) == 0) {

You were too polite to say "by a hidden code style change", instead of
saying something stronger, like "without understanding the operator
precedence rules", but I actually thing that is what is going on.  Not
just the original author of 15da108 but no reviewer noticed when the patch
was posted on the list.

The problem description strongly suggests to me that we should probably
fix the use of "and" here as well in order to avoid confusing ourselves.

You have another patch that touches this area that adds "and $thread", but
I think it is much less error prone if we stick to && and || with explicit
parentheses to clarify the precedence we want to see.

[RF sanity check] send-email threading fixes (was Re: [PATCH 3/6] send-email: fix threaded mails without chain-reply-to)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> You were too polite to say "by a hidden code style change", instead of
> saying something stronger,...

Erm, I meant "you were very polite and said"...

In any case, here is what I came up with by reshuffling your six patches.

[PATCH 1/6] add a test for git-send-email for non-threaded mails
[PATCH 2/6] send-email: fix non-threaded mails
[PATCH 3/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread
[PATCH 4/6] send-email: fix threaded mails without chain-reply-to
[PATCH 5/6] add a test for git-send-email for threaded mails without chain-reply-to
[PATCH 6/6] send-email: fix a typo in a comment

The first three patches apply to the tip of 'maint', which is currently at
802f9c9 (diff.c: plug a memory leak in an error path, 2009-06-08).  The
first one exposes breakage introduced by 3e0c4ff and then the second one
fixes it.

Patch 4/6 applies on top of 15da108 to fix a breakage introduced by that
commit.  Together with the result of applying the first three patches to
maint, they fix --no-thread and --thread (without --chain-reply-to) cases.
The result can be verified by applying Patch 5/6 on top of them.

They will appear in tonight's 'pu'; the overall tip for the topic would be
at 479ad6b (unless I find breakages during the final testing).

Thanks.

[PATCH 1/6] add a test for git-send-email for non-threaded mails

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Mon, 8 Jun 2009 09:13:16 +0200

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * To be applied on 'maint'.

 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ce26ea4..5bfa36e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,14 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches >stdout &&
+	! grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.214.gf4f78e

[PATCH 2/6] send-email: fix non-threaded mails

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Mon, 8 Jun 2009 08:43:56 +0200

After commit 3e0c4ff (send-email: respect in-reply-to regardless of
threading, 2009-03-01) the variable $thread was only used for prompting
for an "In-Reply-To", but not for controlling whether the "In-Reply-To"
and "References" fields should be written into the email.

Thus these fields were always used beginning with the second mail and it
was not possible to produce non-threaded mails anymore until commit
15da108 ("send-email: 'References:' should only reference what is sent",
2009-04-13), which introduced a regression with the side effect to make
it possible again when --no-chain-reply-to was set.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of [1/6]

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

diff --git a/git-send-email.perl b/git-send-email.perl
index cccbf45..5d51697 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1137,7 +1137,8 @@ foreach my $t (@files) {
 	send_message();
 
 	# set up for the next message
-	if ($chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($thread &&
+		($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5bfa36e..8518aca 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'no in-reply-to and no threading' '
+test_expect_success 'no in-reply-to and no threading' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.214.gf4f78e

[PATCH 3/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Sun, 7 Jun 2009 23:40:57 +0200

Also remove the argument from --[no-]chain-reply-to.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of [2/6]

 Documentation/git-send-email.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 794224b..3c7f896 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -155,7 +155,7 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to=<identifier>::
+--[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -204,7 +204,8 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 --[no-]thread::
 	If this is set, the In-Reply-To header will be set on each email sent.
 	If disabled with "--no-thread", no emails will have the In-Reply-To
-	header set. Default is the value of the 'sendemail.thread' configuration
+	header set, unless specified with --in-reply-to.
+	Default is the value of the 'sendemail.thread' configuration
 	value; if that is unspecified, default to --thread.
 
 
-- 
1.6.3.2.214.gf4f78e

[PATCH 4/6] send-email: fix threaded mails without chain-reply-to

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Sun, 7 Jun 2009 23:40:54 +0200

An earlier commit 15da108 ("send-email: 'References:' should only
reference what is sent", 2009-04-13) broke logic to set up threading
information for the next message by rewriting "!" to "not" without
understanding the precedence rules of the language.

Namely,

    ! defined $reply_to || length($reply_to) == 0

was changed to

    not defined $reply_to || length($reply_to) == 0

which is

    not (defined $reply_to || length($reply_to) == 0)

and different from what was intended, which is

    (not defined $reply_to) || (length($reply_to) == 0)

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of 15da108 to fix the breakage there.

 git-send-email.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 43f956b..c55b045 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1135,7 +1135,8 @@ foreach my $t (@files) {
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent &&
+		($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
-- 
1.6.3.2.214.gf4f78e

[PATCH 5/6] add a test for git-send-email for threaded mails without chain-reply-to

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Mon, 8 Jun 2009 08:37:31 +0200

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * To be applied on top of:
   - applying 1/6, 2/6 and 3/6 to maint
   - applying 4/6 to 15da108; then
   - merging the above two together.
   to demonstrate the fix in 4/6

 t/t9001-send-email.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 2ce24cd..4f67de3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'threading but no chain-reply-to' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--thread \
+		--nochain-reply-to \
+		$patches $patches >stdout &&
+	grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.214.gf4f78e

[PATCH 6/6] send-email: fix a typo in a comment

[Junio C Hamano]

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Sun, 7 Jun 2009 23:40:52 +0200

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is pretty much an independent fix; applicable to 'master'.

 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index be7eb5c..303e03a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -812,7 +812,7 @@ sub sanitize_address
 }
 
 # Returns 1 if the message was sent, and 0 otherwise.
-# In actuality, the whole program dies when a there
+# In actuality, the whole program dies when there
 # is an error sending a message.
 
 sub send_message
-- 
1.6.3.2.214.gf4f78e

Re: [RF sanity check] send-email threading fixes (was Re: [PATCH 3/6] send-email: fix threaded mails without chain-reply-to)

[Markus Heidelberg]

Junio C Hamano, 10.06.2009:
> In any case, here is what I came up with by reshuffling your six patches.
> 
> [PATCH 1/6] add a test for git-send-email for non-threaded mails
> [PATCH 2/6] send-email: fix non-threaded mails
> [PATCH 3/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread
> [PATCH 4/6] send-email: fix threaded mails without chain-reply-to
> [PATCH 5/6] add a test for git-send-email for threaded mails without chain-reply-to
> [PATCH 6/6] send-email: fix a typo in a comment
> 
> Patch 4/6 applies on top of 15da108 to fix a breakage introduced by that
> commit.  Together with the result of applying the first three patches to
> maint, they fix --no-thread and --thread (without --chain-reply-to) cases.
> The result can be verified by applying Patch 5/6 on top of them.

Out of curiosity: why are 4/6 and 5/6 applied on different branches
without the usual commits first "test with test_expect_fail" then "fix +
test changed to test_expect_success"?

Re: [RF sanity check] send-email threading fixes

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

>> [PATCH 4/6] send-email: fix threaded mails without chain-reply-to
>> [PATCH 5/6] add a test for git-send-email for threaded mails without chain-reply-to
>> [PATCH 6/6] send-email: fix a typo in a comment
>> 
>> Patch 4/6 applies on top of 15da108 to fix a breakage introduced by that
>> commit.  Together with the result of applying the first three patches to
>> maint, they fix --no-thread and --thread (without --chain-reply-to) cases.
>> The result can be verified by applying Patch 5/6 on top of them.
>
> Out of curiosity: why are 4/6 and 5/6 applied on different branches
> without the usual commits first "test with test_expect_fail" then "fix +
> test changed to test_expect_success"?

Maybe I misunderstood your problem description, but my impression from it
was that 4/6 is a fix to a latent bug in 15da108 that later affected what
was done by new code that appeard somewhere between 15da108 and 'master',
and the test 5/6 was about that breakage.  IOW, root cause was older than
the symptom.

Re: [RF sanity check] send-email threading fixes

[Markus Heidelberg]

Junio C Hamano, 11.06.2009:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> >> [PATCH 4/6] send-email: fix threaded mails without chain-reply-to
> >> [PATCH 5/6] add a test for git-send-email for threaded mails without chain-reply-to
> >> [PATCH 6/6] send-email: fix a typo in a comment
> >> 
> >> Patch 4/6 applies on top of 15da108 to fix a breakage introduced by that
> >> commit.  Together with the result of applying the first three patches to
> >> maint, they fix --no-thread and --thread (without --chain-reply-to) cases.
> >> The result can be verified by applying Patch 5/6 on top of them.
> >
> > Out of curiosity: why are 4/6 and 5/6 applied on different branches
> > without the usual commits first "test with test_expect_fail" then "fix +
> > test changed to test_expect_success"?
> 
> Maybe I misunderstood your problem description, but my impression from it
> was that 4/6 is a fix to a latent bug in 15da108

Right, ...

> that later affected what

... but the bug was immediately noticable.

> was done by new code that appeard somewhere between 15da108 and 'master',

The other regression from 3e0c4ff appeared before 15da108, not after it.
This regression was immediately noticable as well.

> and the test 5/6 was about that breakage.

No, 5/6 was only about the breakage in 15da108 fixed by 4/6. I noticed
the regression in 3e0c4ff after it.

> IOW, root cause was older than
> the symptom.

Since it's a bit confusing, I'll try to clarify:

* 3e0c4ff (2009-03-01) broke non-threaded
  This sent threaded mails instead of non-threaded:
    $ git send-email --no-thread --no-chain-reply-to
    $ git send-email --no-thread --chain-reply-to
* 1/6 tests this
* 2/6 fixes this

* 15da108 (2009-04-13) broke threaded without chain-reply
  This sent non-threaded mails instead of threaded:
    $ git send-email --thread --no-chain-reply-to
  This correctly sent threaded mails:
    $ git send-email --thread --chain-reply-to
* 4/6 fixes this
* 5/6 tests this
* 15da108 additionally changed another behaviour
  (this is the side effect from the 2/6 description, which is
   anticipating in view of the reordered patch sequence):
  This now sent non-threaded mails again, which has been broken by 3e0c4ff:
    $ git send-email --no-thread --no-chain-reply-to
  This still sent threaded mails instead of non-threaded, it was still
  broken since 3e0c4ff:
    $ git send-email --no-thread --chain-reply-to

HTH and I got everything explained correctly.

Re: [RF sanity check] send-email threading fixes

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> HTH and I got everything explained correctly.

Now, you can summarize the series with a proper [0/6], perhaps modelling
after how I explained the order of application and merge structure ;-).

[PATCH 0/6] send-email threading fixes

[Markus Heidelberg]

Junio C Hamano, 12.06.2009:
> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > HTH and I got everything explained correctly.
> 
> Now, you can summarize the series with a proper [0/6], perhaps modelling
> after how I explained the order of application and merge structure ;-).

OK, I'll try :)
For easier understanding I kept the wording from your reshuffled series.

[PATCH 1/6] add a test for git-send-email for non-threaded mails
[PATCH 2/6] send-email: fix non-threaded mails
[PATCH 3/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread
[PATCH 4/6] add a test for git-send-email for threaded mails without chain-reply-to
[PATCH 5/6] send-email: fix threaded mails without chain-reply-to
[PATCH 6/6] send-email: fix a typo in a comment

The first three patches apply to the tip of 'maint', which is currently at
94af7c3 (Documentation: git-send-mail can take rev-list arg to drive
format-patch, 2009-06-11).  The first one exposes breakage introduced by
3e0c4ff and then the second one fixes it.

The last three patches apply to the tip of 'master'. Patch 4/6 exposes
breakage introduced by 15da108, patch 5/6 fixes it.

Changes compared to 'pu':

 * adjusted the description of 2/6 to hopefully add less confusion
 * changed the order of 4/6 and 5/6 and applied 5/6 directly on top of 4/6,
   since the regression was immediately noticable
 * also change test_expect_failure to test_expect_success in 5/6 then
 * instead of in the former merge commit a6f8abc "Merge branch
   'mh/master-send-email-threaded-fix' into mh/master-send-email"

[PATCH 1/6] add a test for git-send-email for non-threaded mails

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * To be applied on 'maint'.

 t/t9001-send-email.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index ce26ea4..5bfa36e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,14 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'no in-reply-to and no threading' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--nothread \
+		$patches $patches >stdout &&
+	! grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.236.ge505d

[PATCH 2/6] send-email: fix non-threaded mails

[Markus Heidelberg]

After commit 3e0c4ff (send-email: respect in-reply-to regardless of
threading, 2009-03-01) the variable $thread was only used for prompting
for an "In-Reply-To", but not for controlling whether the "In-Reply-To"
and "References" fields should be written into the email.

Thus these fields were always used beginning with the second mail and it
was not possible to produce non-threaded mails anymore.

However, a later commit 15da108 ("send-email: 'References:' should only
reference what is sent", 2009-04-13) introduced a regression with the
side effect to make non-threaded mails possible again, but only when
--no-chain-reply-to was used.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of [1/6]

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

diff --git a/git-send-email.perl b/git-send-email.perl
index cccbf45..5d51697 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1137,7 +1137,8 @@ foreach my $t (@files) {
 	send_message();
 
 	# set up for the next message
-	if ($chain_reply_to || !defined $reply_to || length($reply_to) == 0) {
+	if ($thread &&
+		($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5bfa36e..8518aca 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'no in-reply-to and no threading' '
+test_expect_success 'no in-reply-to and no threading' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.236.ge505d

[PATCH 3/6] doc/send-email: clarify the behavior of --in-reply-to with --no-thread

[Markus Heidelberg]

Also remove the argument from --[no-]chain-reply-to.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of [2/6]

 Documentation/git-send-email.txt |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index a282190..0ec5343 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -159,7 +159,7 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
---[no-]chain-reply-to=<identifier>::
+--[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
 	the first will be sent as replies to the first email sent.  When using
@@ -208,7 +208,8 @@ specified, as well as 'body' if --no-signed-off-cc is specified.
 --[no-]thread::
 	If this is set, the In-Reply-To header will be set on each email sent.
 	If disabled with "--no-thread", no emails will have the In-Reply-To
-	header set. Default is the value of the 'sendemail.thread' configuration
+	header set, unless specified with --in-reply-to.
+	Default is the value of the 'sendemail.thread' configuration
 	value; if that is unspecified, default to --thread.
 
 
-- 
1.6.3.2.236.ge505d

[PATCH 4/6] add a test for git-send-email for threaded mails without chain-reply-to

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * To be applied on 'master'.

 t/t9001-send-email.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 2ce24cd..4f67de3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
+test_expect_failure 'threading but no chain-reply-to' '
+	git send-email \
+		--dry-run \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--thread \
+		--nochain-reply-to \
+		$patches $patches >stdout &&
+	grep "In-Reply-To: " stdout
+'
+
 test_done
-- 
1.6.3.2.236.ge505d

[PATCH 5/6] send-email: fix threaded mails without chain-reply-to

[Markus Heidelberg]

An earlier commit 15da108 ("send-email: 'References:' should only
reference what is sent", 2009-04-13) broke logic to set up threading
information for the next message by rewriting "!" to "not" without
understanding the precedence rules of the language.

Namely,

    ! defined $reply_to || length($reply_to) == 0

was changed to

    not defined $reply_to || length($reply_to) == 0

which is

    not (defined $reply_to || length($reply_to) == 0)

and different from what was intended, which is

    (not defined $reply_to) || (length($reply_to) == 0)

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Applies on top of [4/6]

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

diff --git a/git-send-email.perl b/git-send-email.perl
index 4c795a4..16d12e0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1150,7 +1150,8 @@ foreach my $t (@files) {
 	my $message_was_sent = send_message();
 
 	# set up for the next message
-	if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) {
+	if ($message_was_sent &&
+		($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) {
 		$reply_to = $message_id;
 		if (length $references > 0) {
 			$references .= "\n $message_id";
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 4f67de3..8ab1a78 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' '
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
 
-test_expect_failure 'threading but no chain-reply-to' '
+test_expect_success 'threading but no chain-reply-to' '
 	git send-email \
 		--dry-run \
 		--from="Example <nobody@example.com>" \
-- 
1.6.3.2.236.ge505d

[PATCH 6/6] send-email: fix a typo in a comment

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is pretty much an independent fix; applicable to 'master'.

 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 16d12e0..4a77d44 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -812,7 +812,7 @@ sub sanitize_address
 }
 
 # Returns 1 if the message was sent, and 0 otherwise.
-# In actuality, the whole program dies when a there
+# In actuality, the whole program dies when there
 # is an error sending a message.
 
 sub send_message
-- 
1.6.3.2.236.ge505d