[PATCHi v4] git-send-email.perl: make initial In-Reply-To apply only to first email

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When an initial In-Reply-To is supplied it should apply only to the
first email, second and subsequent messages should behave just according
to the --[no-]chain-reply-to setting; this is also what the man page
says about the --[no-]chain-reply-to option and this is also how the
correspondent git-format-patch option behaves.

This is the typical behaviour we    |
want when we send a series with     | [PATCH 0/2] Here is what I did...
cover letter in reply to some       |   [PATCH 1/2] Clean up and tests
discussion, the new patch series    |   [PATCH 2/2] Implementation
should appear as a separate subtree |   [PATCH v2 0/3] Here is a reroll
in the discussion, look at the v2   |     [PATCH v2 1/3] Clean up
series in the illustration on the   |     [PATCH v2 2/3] New tests
right to see what the new behaviour |     [PATCH v2 3/3] Implementation
ensures.                            |

Moreover, when $initial_reply_to is asked to the user interactively it
is asked as the "Message-ID to be used as In-Reply-To for the _first_
email", this makes the user think that the second and subsequent patches
are not using it but are considered as replies to the first message or
chained according to the --[no-]chain-reply setting.

Fix also the documentation about --in-reply-to to avoid ambiguities.

NOTE: This patch changes the current behaviour and brings it to be what
I think were the intentions stated in the documentation, also aligning it
to how git-format-patch behaves; in order to achieve the old behaviour
of a flat structure in reply to something the user can always use
"--no-thread --in-reply-to <...>".

Signed-off-by: Antonio Ospite <ospite@xxxxxxxxxxxxxxxxx>
---

Hoping we are there. Patch is on top of origin/next.

Changes since v3:
 - Change the test about 'In-Reply-To without --chain-reply-to' instead of
   providing  a new one.
 - Illustrate an actual use case when describing the new behaviour, both in
   the commit message and in the documentation.

It is cool to see how such a small change in the code requires quite some
"communication overhead", not a big surprise in general but in this case I
wonder if I've been overly verbose.

Junio, if you feel that the documentation and the commit message can be slimmed
down feel free to do it when committing the patch.

Matthieu, maybe we can have a Tested-by: you, can't we?

Thanks a lot,
   Antonio

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

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 05904e0..ebc024a 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -82,11 +82,26 @@ See the CONFIGURATION section for 'sendemail.multiedit'.
 	set, as returned by "git var -l".
 
 --in-reply-to=<identifier>::
-	Specify the contents of the first In-Reply-To header.
-	Subsequent emails will refer to the previous email
-	instead of this if --chain-reply-to is set.
-	Only necessary if --compose is also set.  If --compose
-	is not set, this will be prompted for.
+	Make the first mail (or all the mails with `--no-thread`) appear as a
+	reply to the given Message-Id, which avoids breaking threads to
+	provide a new patch series.
+	The second and subsequent emails will be sent as replies according to
+	the `--[no]-chain-reply-to` setting.
++
+So for example when `--thread` and `--no-chain-reply-to` are specified, the
+second and subsequent patches will be replies to the first one like in the
+illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:
++
+  [PATCH 0/2] Here is what I did...
+    [PATCH 1/2] Clean up and tests
+    [PATCH 2/2] Implementation
+    [PATCH v2 0/3] Here is a reroll
+      [PATCH v2 1/3] Clean up
+      [PATCH v2 2/3] New tests
+      [PATCH v2 3/3] Implementation
++
+Only necessary if --compose is also set.  If --compose
+is not set, this will be prompted for.
 
 --subject=<string>::
 	Specify the initial subject of the email thread.
diff --git a/git-send-email.perl b/git-send-email.perl
index f68ed5a..fe6b848 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1319,7 +1319,8 @@ foreach my $t (@files) {
 
 	# set up for the next message
 	if ($thread && $message_was_sent &&
-		(chain_reply_to() || !defined $reply_to || length($reply_to) == 0)) {
+		(chain_reply_to() || !defined $reply_to || length($reply_to) == 0 ||
+		$message_num == 1)) {
 		$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 26c2e93..5e48318 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -324,9 +324,11 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' '
 		--smtp-server="$(pwd)/fake.sendmail" \
 		$patches $patches $patches \
 		2>errors &&
-	# All the messages are replies to --in-reply-to
+	# The first message is a reply to --in-reply-to
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual &&
 	test_cmp expect actual &&
+	# Second and subsequent messages are replies to the first one
+	sed -n -e "s/^Message-Id: *\(.*\)/\1/p" msgtxt1 >expect &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt2 >actual &&
 	test_cmp expect actual &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt3 >actual &&
-- 
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]