From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 11 Mar 2009 20:48:07 +0000 (-0700)
Subject: Merge branch 'tr/format-patch-thread'
X-Git-Tag: v1.6.3-rc0~174
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/5a5bd23486c5d8721f4654eadec75213d3dc074f?hp=72e3c32bef0574ec6febc5e815b0467f892ef7e2

Merge branch 'tr/format-patch-thread'

* tr/format-patch-thread:
format-patch: support deep threading
format-patch: thread as reply to cover letter even with in-reply-to
format-patch: track several references
format-patch: threading test reactivation

Conflicts:
builtin-log.c
---

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5152c5038..300ab25dcf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -677,6 +677,16 @@ format.pretty::
 	See linkgit:git-log[1], linkgit:git-show[1],
 	linkgit:git-whatchanged[1].
 
+format.thread::
+	The default threading style for 'git-format-patch'.  Can be
+	either a boolean value, `shallow` or `deep`.  'Shallow'
+	threading makes every mail a reply to the head of the series,
+	where the head is chosen from the cover letter, the
+	`\--in-reply-to`, and the first patch mail, in this order.
+	'Deep' threading makes every mail a reply to the previous one.
+	A true boolean value is the same as `shallow`, and a false
+	value disables threading.
+
 gc.aggressiveWindow::
 	The window size parameter used in the delta compression
 	algorithm used by 'git-gc --aggressive'.  This defaults
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e7ae8cf109..c14e3ee395 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -127,10 +127,18 @@ include::diff-options.txt[]
 	which is the commit message and the patch itself in the
 	second part, with "Content-Disposition: inline".
 
---thread::
+--thread[=<style>]::
 	Add In-Reply-To and References headers to make the second and
 	subsequent mails appear as replies to the first.  Also generates
 	the Message-Id header to reference.
++
+The optional <style> argument can be either `shallow` or `deep`.
+'Shallow' threading makes every mail a reply to the head of the
+series, where the head is chosen from the cover letter, the
+`\--in-reply-to`, and the first patch mail, in this order.  'Deep'
+threading makes every mail a reply to the previous one.  If not
+specified, defaults to the 'format.thread' configuration, or `shallow`
+if that is not set.
 
 --in-reply-to=Message-Id::
 	Make the first mail (or all the mails with --no-thread) appear as a
diff --git a/builtin-log.c b/builtin-log.c
index 8549028817..8684fcdb67 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -17,6 +17,7 @@
 #include "run-command.h"
 #include "shortlog.h"
 #include "remote.h"
+#include "string-list.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
@@ -461,6 +462,10 @@ static void add_header(const char *value)
 	extra_hdr[extra_hdr_nr++] = xstrndup(value, len);
 }
 
+#define THREAD_SHALLOW 1
+#define THREAD_DEEP 2
+static int thread = 0;
+
 static int git_format_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "format.headers")) {
@@ -497,7 +502,18 @@ static int git_format_config(const char *var, const char *value, void *cb)
 			default_attach = xstrdup(git_version_string);
 		return 0;
 	}
-
+	if (!strcmp(var, "format.thread")) {
+		if (value && !strcasecmp(value, "deep")) {
+			thread = THREAD_DEEP;
+			return 0;
+		}
+		if (value && !strcasecmp(value, "shallow")) {
+			thread = THREAD_SHALLOW;
+			return 0;
+		}
+		thread = git_config_bool(var, value) && THREAD_SHALLOW;
+		return 0;
+	}
 
 	return git_log_config(var, value, cb);
 }
@@ -776,7 +792,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int numbered_files = 0;		/* _just_ numbers */
 	int subject_prefix = 0;
 	int ignore_if_in_upstream = 0;
-	int thread = 0;
 	int cover_letter = 0;
 	int boundary_count = 0;
 	int no_binary_diff = 0;
@@ -878,8 +893,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 		else if (!strcmp(argv[i], "--ignore-if-in-upstream"))
 			ignore_if_in_upstream = 1;
-		else if (!strcmp(argv[i], "--thread"))
-			thread = 1;
+		else if (!strcmp(argv[i], "--thread")
+			|| !strcmp(argv[i], "--thread=shallow"))
+			thread = THREAD_SHALLOW;
+		else if (!strcmp(argv[i], "--thread=deep"))
+			thread = THREAD_DEEP;
+		else if (!strcmp(argv[i], "--no-thread"))
+			thread = 0;
 		else if (!prefixcmp(argv[i], "--in-reply-to="))
 			in_reply_to = argv[i] + 14;
 		else if (!strcmp(argv[i], "--in-reply-to")) {
@@ -1030,8 +1050,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		numbered = 1;
 	if (numbered)
 		rev.total = total + start_number - 1;
-	if (in_reply_to)
-		rev.ref_message_id = clean_message_id(in_reply_to);
+	if (in_reply_to || thread || cover_letter)
+		rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
+	if (in_reply_to) {
+		const char *msgid = clean_message_id(in_reply_to);
+		string_list_append(msgid, rev.ref_message_ids);
+	}
 	if (cover_letter) {
 		if (thread)
 			gen_message_id(&rev, "cover");
@@ -1050,15 +1074,33 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			/* Have we already had a message ID? */
 			if (rev.message_id) {
 				/*
-				 * If we've got the ID to be a reply
-				 * to, discard the current ID;
-				 * otherwise, make everything a reply
-				 * to that.
+				 * For deep threading: make every mail
+				 * a reply to the previous one, no
+				 * matter what other options are set.
+				 *
+				 * For shallow threading:
+				 *
+				 * Without --cover-letter and
+				 * --in-reply-to, make every mail a
+				 * reply to the one before.
+				 *
+				 * With --in-reply-to but no
+				 * --cover-letter, make every mail a
+				 * reply to the <reply-to>.
+				 *
+				 * With --cover-letter, make every
+				 * mail but the cover letter a reply
+				 * to the cover letter.  The cover
+				 * letter is a reply to the
+				 * --in-reply-to, if specified.
 				 */
-				if (rev.ref_message_id)
+				if (thread == THREAD_SHALLOW
+				    && rev.ref_message_ids->nr > 0
+				    && (!cover_letter || rev.nr > 1))
 					free(rev.message_id);
 				else
-					rev.ref_message_id = rev.message_id;
+					string_list_append(rev.message_id,
+							   rev.ref_message_ids);
 			}
 			gen_message_id(&rev, sha1_to_hex(commit->object.sha1));
 		}
diff --git a/log-tree.c b/log-tree.c
index 63cff74350..9565c184db 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -6,6 +6,7 @@
 #include "log-tree.h"
 #include "reflog-walk.h"
 #include "refs.h"
+#include "string-list.h"
 
 struct decoration name_decoration = { "object names" };
 
@@ -211,9 +212,13 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 		printf("Message-Id: <%s>\n", opt->message_id);
 		graph_show_oneline(opt->graph);
 	}
-	if (opt->ref_message_id) {
-		printf("In-Reply-To: <%s>\nReferences: <%s>\n",
-		       opt->ref_message_id, opt->ref_message_id);
+	if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) {
+		int i, n;
+		n = opt->ref_message_ids->nr;
+		printf("In-Reply-To: <%s>\n", opt->ref_message_ids->items[n-1].string);
+		for (i = 0; i < n; i++)
+			printf("%s<%s>\n", (i > 0 ? "\t" : "References: "),
+			       opt->ref_message_ids->items[i].string);
 		graph_show_oneline(opt->graph);
 	}
 	if (opt->mime_boundary) {
diff --git a/revision.h b/revision.h
index 7cf848771b..8c0a41713c 100644
--- a/revision.h
+++ b/revision.h
@@ -89,7 +89,7 @@ struct rev_info {
 	int		nr, total;
 	const char	*mime_boundary;
 	char		*message_id;
-	const char	*ref_message_id;
+	struct string_list *ref_message_ids;
 	const char	*add_signoff;
 	const char	*extra_headers;
 	const char	*log_reencode;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index f045898fe3..ebfc4a6590 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -138,56 +138,243 @@ test_expect_success 'multiple files' '
 	ls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch
 '
 
-test_expect_success 'thread' '
+check_threading () {
+	expect="$1" &&
+	shift &&
+	(git format-patch --stdout "$@"; echo $? > status.out) |
+	# Prints everything between the Message-ID and In-Reply-To,
+	# and replaces all Message-ID-lookalikes by a sequence number
+	perl -ne '
+		if (/^(message-id|references|in-reply-to)/i) {
+			$printing = 1;
+		} elsif (/^\S/) {
+			$printing = 0;
+		}
+		if ($printing) {
+			$h{$1}=$i++ if (/<([^>]+)>/ and !exists $h{$1});
+			for $k (keys %h) {s/$k/$h{$k}/};
+			print;
+		}
+		print "---\n" if /^From /i;
+	' > actual &&
+	test 0 = "$(cat status.out)" &&
+	test_cmp "$expect" actual
+}
+
+cat >> expect.no-threading <<EOF
+---
+---
+---
+EOF
 
-	rm -rf patches/ &&
+test_expect_success 'no threading' '
 	git checkout side &&
-	git format-patch --thread -o patches/ master &&
-	FIRST_MID=$(grep "Message-Id:" patches/0001-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
-	for i in patches/0002-* patches/0003-*
-	do
-	  grep "References: $FIRST_MID" $i &&
-	  grep "In-Reply-To: $FIRST_MID" $i || break
-	done
+	check_threading expect.no-threading master
 '
 
-test_expect_success 'thread in-reply-to' '
+cat > expect.thread <<EOF
+---
+Message-Id: <0>
+---
+Message-Id: <1>
+In-Reply-To: <0>
+References: <0>
+---
+Message-Id: <2>
+In-Reply-To: <0>
+References: <0>
+EOF
 
-	rm -rf patches/ &&
-	git checkout side &&
-	git format-patch --in-reply-to="<test.message>" --thread -o patches/ master &&
-	FIRST_MID="<test.message>" &&
-	for i in patches/*
-	do
-	  grep "References: $FIRST_MID" $i &&
-	  grep "In-Reply-To: $FIRST_MID" $i || break
-	done
+test_expect_success 'thread' '
+	check_threading expect.thread --thread master
 '
 
-test_expect_success 'thread cover-letter' '
+cat > expect.in-reply-to <<EOF
+---
+Message-Id: <0>
+In-Reply-To: <1>
+References: <1>
+---
+Message-Id: <2>
+In-Reply-To: <1>
+References: <1>
+---
+Message-Id: <3>
+In-Reply-To: <1>
+References: <1>
+EOF
 
-	rm -rf patches/ &&
-	git checkout side &&
-	git format-patch --cover-letter --thread -o patches/ master &&
-	FIRST_MID=$(grep "Message-Id:" patches/0000-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
-	for i in patches/0001-* patches/0002-* patches/0003-*
-	do
-	  grep "References: $FIRST_MID" $i &&
-	  grep "In-Reply-To: $FIRST_MID" $i || break
-	done
+test_expect_success 'thread in-reply-to' '
+	check_threading expect.in-reply-to --in-reply-to="<test.message>" \
+		--thread master
 '
 
+cat > expect.cover-letter <<EOF
+---
+Message-Id: <0>
+---
+Message-Id: <1>
+In-Reply-To: <0>
+References: <0>
+---
+Message-Id: <2>
+In-Reply-To: <0>
+References: <0>
+---
+Message-Id: <3>
+In-Reply-To: <0>
+References: <0>
+EOF
+
+test_expect_success 'thread cover-letter' '
+	check_threading expect.cover-letter --cover-letter --thread master
+'
+
+cat > expect.cl-irt <<EOF
+---
+Message-Id: <0>
+In-Reply-To: <1>
+References: <1>
+---
+Message-Id: <2>
+In-Reply-To: <0>
+References: <1>
+	<0>
+---
+Message-Id: <3>
+In-Reply-To: <0>
+References: <1>
+	<0>
+---
+Message-Id: <4>
+In-Reply-To: <0>
+References: <1>
+	<0>
+EOF
+
 test_expect_success 'thread cover-letter in-reply-to' '
+	check_threading expect.cl-irt --cover-letter \
+		--in-reply-to="<test.message>" --thread master
+'
 
-	rm -rf patches/ &&
-	git checkout side &&
-	git format-patch --cover-letter --in-reply-to="<test.message>" --thread -o patches/ master &&
-	FIRST_MID="<test.message>" &&
-	for i in patches/*
-	do
-	  grep "References: $FIRST_MID" $i &&
-	  grep "In-Reply-To: $FIRST_MID" $i || break
-	done
+test_expect_success 'thread explicit shallow' '
+	check_threading expect.cl-irt --cover-letter \
+		--in-reply-to="<test.message>" --thread=shallow master
+'
+
+cat > expect.deep <<EOF
+---
+Message-Id: <0>
+---
+Message-Id: <1>
+In-Reply-To: <0>
+References: <0>
+---
+Message-Id: <2>
+In-Reply-To: <1>
+References: <0>
+	<1>
+EOF
+
+test_expect_success 'thread deep' '
+	check_threading expect.deep --thread=deep master
+'
+
+cat > expect.deep-irt <<EOF
+---
+Message-Id: <0>
+In-Reply-To: <1>
+References: <1>
+---
+Message-Id: <2>
+In-Reply-To: <0>
+References: <1>
+	<0>
+---
+Message-Id: <3>
+In-Reply-To: <2>
+References: <1>
+	<0>
+	<2>
+EOF
+
+test_expect_success 'thread deep in-reply-to' '
+	check_threading expect.deep-irt  --thread=deep \
+		--in-reply-to="<test.message>" master
+'
+
+cat > expect.deep-cl <<EOF
+---
+Message-Id: <0>
+---
+Message-Id: <1>
+In-Reply-To: <0>
+References: <0>
+---
+Message-Id: <2>
+In-Reply-To: <1>
+References: <0>
+	<1>
+---
+Message-Id: <3>
+In-Reply-To: <2>
+References: <0>
+	<1>
+	<2>
+EOF
+
+test_expect_success 'thread deep cover-letter' '
+	check_threading expect.deep-cl --cover-letter --thread=deep master
+'
+
+cat > expect.deep-cl-irt <<EOF
+---
+Message-Id: <0>
+In-Reply-To: <1>
+References: <1>
+---
+Message-Id: <2>
+In-Reply-To: <0>
+References: <1>
+	<0>
+---
+Message-Id: <3>
+In-Reply-To: <2>
+References: <1>
+	<0>
+	<2>
+---
+Message-Id: <4>
+In-Reply-To: <3>
+References: <1>
+	<0>
+	<2>
+	<3>
+EOF
+
+test_expect_success 'thread deep cover-letter in-reply-to' '
+	check_threading expect.deep-cl-irt --cover-letter \
+		--in-reply-to="<test.message>" --thread=deep master
+'
+
+test_expect_success 'thread via config' '
+	git config format.thread true &&
+	check_threading expect.thread master
+'
+
+test_expect_success 'thread deep via config' '
+	git config format.thread deep &&
+	check_threading expect.deep master
+'
+
+test_expect_success 'thread config + override' '
+	git config format.thread deep &&
+	check_threading expect.thread --thread master
+'
+
+test_expect_success 'thread config + --no-thread' '
+	git config format.thread deep &&
+	check_threading expect.no-threading --no-thread master
 '
 
 test_expect_success 'excessive subject' '