threads / patch / 19727

patch, 6 partssend-email: fix a typo in a comment

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

## tl;dr

38 messages between Jun 7, 2009 and Jun 12, 2009. Diffs are folded; open one to read it.

replies: 37people: 2as markdown or json

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · lore

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

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(-)

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

Show changes to git-send-email.perl +1 −1

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

Junio C Hamano· Jun 8, 2009, 21:44 UTC · re: Markus Heidelberg · lore

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

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

Markus Heidelberg· Jun 9, 2009, 07:05 UTC · re: Junio C Hamano · lore

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

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

Markus Heidelberg· Jun 9, 2009, 07:11 UTC · re: Markus Heidelberg · lore

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

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

Show changes to t/t9001-send-email.sh +10 −0

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

Markus Heidelberg· Jun 9, 2009, 07:11 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +5 −2

git-send-email.perl, t/t9001-send-email.sh

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

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

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

Show changes to t/t9001-send-email.sh +11 −0

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

Junio C Hamano· Jun 7, 2009, 23:28 UTC · re: Markus Heidelberg · lore

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

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

Show 10 quoted lines

> +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.

Markus Heidelberg· Jun 8, 2009, 05:47 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, 08.06.2009:

Show 17 quoted lines

> 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?

Markus Heidelberg· Jun 8, 2009, 06:37 UTC · re: Markus Heidelberg · lore

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

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

Markus Heidelberg, 08.06.2009:

Show 23 quoted lines

> 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(-)

Show changes to t/t9001-send-email.sh +11 −0

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

Junio C Hamano· Jun 8, 2009, 06:57 UTC · re: Markus Heidelberg · lore

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

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.

Show 27 quoted lines

>
>
>  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

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +2 −2

git-send-email.perl, t/t9001-send-email.sh

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

Junio C Hamano· Jun 10, 2009, 05:51 UTC · re: Markus Heidelberg · lore

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

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

Show 21 quoted lines

> 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.

Junio C Hamano· Jun 10, 2009, 07:15 UTC · re: Junio C Hamano · lore

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

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.

Markus Heidelberg· Jun 11, 2009, 16:49 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, 10.06.2009:

Show 13 quoted lines

> 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"?

Junio C Hamano· Jun 11, 2009, 20:38 UTC · re: Markus Heidelberg · lore

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

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

Show 12 quoted lines

>> [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.

Markus Heidelberg· Jun 11, 2009, 22:49 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, 11.06.2009:

Show 17 quoted lines

> 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.

Junio C Hamano· Jun 11, 2009, 23:21 UTC · re: Markus Heidelberg · lore

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

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 ;-).

Markus Heidelberg· Jun 12, 2009, 10:49 UTC · re: Junio C Hamano · lore

[PATCH 0/6] send-email threading fixes

Junio C Hamano, 12.06.2009:

Show 6 quoted lines

> 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"

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to t/t9001-send-email.sh +10 −0

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

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +3 −2

git-send-email.perl, t/t9001-send-email.sh

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

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to Documentation/git-send-email.txt +3 −1

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

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to t/t9001-send-email.sh +11 −0

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

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +3 −2

git-send-email.perl, t/t9001-send-email.sh

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

Markus Heidelberg· Jun 12, 2009, 10:51 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to git-send-email.perl +1 −1

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

Junio C Hamano· Jun 10, 2009, 07:16 UTC · re: Junio C Hamano · lore

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

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(-)

Show changes to t/t9001-send-email.sh +10 −0

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

Junio C Hamano· Jun 10, 2009, 07:16 UTC · re: Junio C Hamano · lore

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

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

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(-)

Show changes to 2 files +3 −2

git-send-email.perl, t/t9001-send-email.sh

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

Junio C Hamano· Jun 10, 2009, 07:16 UTC · re: Junio C Hamano · lore

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

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(-)

Show changes to Documentation/git-send-email.txt +3 −1

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

Junio C Hamano· Jun 10, 2009, 07:17 UTC · re: Junio C Hamano · lore

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

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

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(-)

Show changes to git-send-email.perl +2 −1

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

Junio C Hamano· Jun 10, 2009, 07:17 UTC · re: Junio C Hamano · lore

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

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(-)

Show changes to t/t9001-send-email.sh +11 −0

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

Junio C Hamano· Jun 10, 2009, 07:17 UTC · re: Junio C Hamano · lore

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

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(-)

Show changes to git-send-email.perl +1 −1

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

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

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

Show changes to t/t9001-send-email.sh +10 −0

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

Markus Heidelberg· Jun 8, 2009, 06:41 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to t/t9001-send-email.sh +10 −0

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

Junio C Hamano· Jun 8, 2009, 07:00 UTC · re: Markus Heidelberg · lore

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

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

Show 8 quoted lines

> 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".

Markus Heidelberg· Jun 8, 2009, 07:13 UTC · re: Junio C Hamano · lore

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

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

Junio C Hamano, 08.06.2009:

Show 5 quoted lines

> 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(-)

Show changes to t/t9001-send-email.sh +10 −0

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

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +5 −2

git-send-email.perl, t/t9001-send-email.sh

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

Markus Heidelberg· Jun 8, 2009, 06:43 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to 2 files +5 −2

git-send-email.perl, t/t9001-send-email.sh

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

Markus Heidelberg· Jun 7, 2009, 21:40 UTC · re: Markus Heidelberg · lore

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

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(-)

Show changes to Documentation/git-send-email.txt +3 −1

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