git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: <git@vger.kernel.org>
Cc: Jeff King <peff@peff.net>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	Derrick Stolee <stolee@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v2] log,diff-tree: add --combined-with-paths options for merges with renames
Date: Fri, 25 Jan 2019 08:54:16 -0800	[thread overview]
Message-ID: <20190125165416.17473-1-newren@gmail.com> (raw)
In-Reply-To: <20190124164654.1923-1-newren@gmail.com>

The raw diff format for merges with -c or --cc will only list one
filename, even if rename detection is active and a rename was detected
for the given path.  Examples:

  ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM	describe.c
  ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM	bar.sh
  ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR	phooey.c

This doesn't let us know what the original name of bar.sh was in the
first parent, and doesn't let us know what either of the original
names of phooey.c were in either of the parents.  In contrast, for
non-merge commits, raw format does provide original filenames (and a
rename score to boot).  In order to also provide original filenames
for merge commits, add a --combined-with-paths option (which is only
useful in conjunction with -c, --raw, and -M and thus implies all
those options) so that we can print tab-separated filenames when
renames are involved.  This transforms the above output to:

  ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM	describe.c
  ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM	foo.sh	bar.sh
  ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR	fooey.c	fuey.c	phooey.c

Signed-off-by: Elijah Newren <newren@gmail.com>
---

Differences since v1: added a testcase, with and without -z

 Documentation/diff-format.txt      | 23 +++++++++++++---
 Documentation/git-diff-tree.txt    |  9 +++++--
 Documentation/rev-list-options.txt |  5 ++++
 combine-diff.c                     | 42 ++++++++++++++++++++++++++----
 diff.h                             |  1 +
 revision.c                         |  7 +++++
 revision.h                         |  1 +
 t/t4038-diff-combined.sh           | 36 +++++++++++++++++++++++++
 8 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index cdcc17f0ad..6e6cfc5d56 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -87,7 +87,7 @@ diff format for merges
 ----------------------
 
 "git-diff-tree", "git-diff-files" and "git-diff --raw"
-can take `-c` or `--cc` option
+can take `-c`, `--cc`, or `--combined-with-paths` option
 to generate diff output also for merge commits.  The output differs
 from the format described above in the following way:
 
@@ -95,12 +95,29 @@ from the format described above in the following way:
 . there are more "src" modes and "src" sha1
 . status is concatenated status characters for each parent
 . no optional "score" number
-. single path, only for "dst"
+. tab-separated pathname(s) of the file
 
-Example:
+For `-c` and `--cc`, only the destination or final path is shown even
+if the file was renamed on any side of history.  With
+`--combined-with-paths`, the number of paths printed will be one more
+than the number of 'R' characters in the concatenated status.  For
+each 'R' in the concatenated status characters, the original pathname
+on that side of history will be shown, and the final path shown on the
+line will be the path used in the merge.
+
+Examples for `-c` and `-cc`:
+------------------------------------------------
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM	describe.c
+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM	bar.sh
+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR	phooey.c
+------------------------------------------------
+
+Examples for `--combined-with-paths`:
 
 ------------------------------------------------
 ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM	describe.c
+::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM	foo.sh	bar.sh
+::100644 100644 100644 e07d6c5 9042e82 ee91881 RR	fooey.c	fuey.c	phooey.c
 ------------------------------------------------
 
 Note that 'combined diff' lists only files which were modified from
diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 2319b2b192..2d9ba382bc 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -10,8 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]
-	      [-t] [-r] [-c | --cc] [--root] [<common diff options>]
-	      <tree-ish> [<tree-ish>] [<path>...]
+	      [-t] [-r] [-c | --cc | --combined-with-paths] [--root]
+	      [<common diff options>] <tree-ish> [<tree-ish>] [<path>...]
 
 DESCRIPTION
 -----------
@@ -108,6 +108,11 @@ include::pretty-options.txt[]
 	itself and the commit log message is not shown, just like in any other
 	"empty diff" case.
 
+--combined-with-paths::
+	This flag is similar to -c, but modifies the raw output format for
+	merges to also show the original paths when renames are found.
+	Implies	-c, -M, and --raw.
+
 --always::
 	Show the commit itself and the commit log message even
 	if the diff itself is empty.
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 98b538bc77..4fdcaef52d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -948,6 +948,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 	the parents have only two variants and the merge result picks
 	one of them without modification.
 
+--combined-with-paths::
+	This flag is similar to -c, but modifies the raw output format for
+	merges to also show the original paths when renames are found.
+	Implies	-c, -M, and --raw.
+
 -m::
 	This flag makes the merge commits show the full diff like
 	regular commits; for each merge parent, a separate log entry
diff --git a/combine-diff.c b/combine-diff.c
index a143c00634..77f7de14c9 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -23,11 +23,15 @@ static int compare_paths(const struct combine_diff_path *one,
 				 two->path, strlen(two->path), two->mode);
 }
 
-static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
+static struct combine_diff_path *intersect_paths(
+	struct combine_diff_path *curr,
+	int n,
+	int num_parent,
+	int combined_with_paths)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct combine_diff_path *p, **tail = &curr;
-	int i, cmp;
+	int i, j, cmp;
 
 	if (!n) {
 		for (i = 0; i < q->nr; i++) {
@@ -50,6 +54,13 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
 			oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
 			p->parent[n].mode = q->queue[i]->one->mode;
 			p->parent[n].status = q->queue[i]->status;
+
+			if (combined_with_paths &&
+			    p->parent[n].status == 'R') {
+				strbuf_init(&p->parent[n].path, 0);
+				strbuf_addstr(&p->parent[n].path,
+					      q->queue[i]->one->path);
+			}
 			*tail = p;
 			tail = &p->next;
 		}
@@ -68,6 +79,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
 		if (cmp < 0) {
 			/* p->path not in q->queue[]; drop it */
 			*tail = p->next;
+			for (j = 0; j < num_parent; j++)
+				if (combined_with_paths &&
+				    p->parent[j].status == 'R')
+					strbuf_release(&p->parent[j].path);
 			free(p);
 			continue;
 		}
@@ -81,6 +96,10 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
 		oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
 		p->parent[n].mode = q->queue[i]->one->mode;
 		p->parent[n].status = q->queue[i]->status;
+		if (combined_with_paths &&
+		    p->parent[n].status == 'R')
+			strbuf_addstr(&p->parent[n].path,
+				      q->queue[i]->one->path);
 
 		tail = &p->next;
 		i++;
@@ -1227,6 +1246,11 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
 		putchar(inter_name_termination);
 	}
 
+	for (i = 0; i < num_parent; i++)
+		if (rev->combined_with_paths &&
+		    p->parent[i].status == 'R')
+			write_name_quoted(p->parent[i].path.buf, stdout,
+					  inter_name_termination);
 	write_name_quoted(p->path, stdout, line_termination);
 }
 
@@ -1324,7 +1348,9 @@ static const char *path_path(void *obj)
 
 /* find set of paths that every parent touches */
 static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
-	const struct oid_array *parents, struct diff_options *opt)
+	const struct oid_array *parents,
+	struct diff_options *opt,
+	int combined_with_paths)
 {
 	struct combine_diff_path *paths = NULL;
 	int i, num_parent = parents->nr;
@@ -1350,7 +1376,8 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
 			opt->output_format = DIFF_FORMAT_NO_OUTPUT;
 		diff_tree_oid(&parents->oid[i], oid, "", opt);
 		diffcore_std(opt);
-		paths = intersect_paths(paths, i, num_parent);
+		paths = intersect_paths(paths, i, num_parent,
+					combined_with_paths);
 
 		/* if showing diff, show it in requested order */
 		if (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&
@@ -1460,7 +1487,8 @@ void diff_tree_combined(const struct object_id *oid,
 		 * diff(sha1,parent_i) for all i to do the job, specifically
 		 * for parent0.
 		 */
-		paths = find_paths_generic(oid, parents, &diffopts);
+		paths = find_paths_generic(oid, parents, &diffopts,
+					   rev->combined_with_paths);
 	}
 	else {
 		int stat_opt;
@@ -1535,6 +1563,10 @@ void diff_tree_combined(const struct object_id *oid,
 	while (paths) {
 		struct combine_diff_path *tmp = paths;
 		paths = paths->next;
+		for (i = 0; i < num_parent; i++)
+			if (rev->combined_with_paths &&
+			    tmp->parent[i].status == 'R')
+				strbuf_release(&tmp->parent[i].path);
 		free(tmp);
 	}
 
diff --git a/diff.h b/diff.h
index b512d0477a..90ea0256a5 100644
--- a/diff.h
+++ b/diff.h
@@ -294,6 +294,7 @@ struct combine_diff_path {
 		char status;
 		unsigned int mode;
 		struct object_id oid;
+		struct strbuf path;
 	} parent[FLEX_ARRAY];
 };
 #define combine_diff_path_size(n, l) \
diff --git a/revision.c b/revision.c
index 13cfb59b38..21c818ba11 100644
--- a/revision.c
+++ b/revision.c
@@ -1995,6 +1995,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->diff = 1;
 		revs->dense_combined_merges = 0;
 		revs->combine_merges = 1;
+	} else if (!strcmp(arg, "--combined-with-paths")) {
+		revs->diff = 1;
+		revs->dense_combined_merges = 0;
+		revs->combine_merges = 1;
+		revs->combined_with_paths = 1;
+		revs->diffopt.detect_rename = DIFF_DETECT_RENAME;
+		revs->diffopt.output_format |= DIFF_FORMAT_RAW;
 	} else if (!strcmp(arg, "--cc")) {
 		revs->diff = 1;
 		revs->dense_combined_merges = 1;
diff --git a/revision.h b/revision.h
index 52e5a88ff5..59a15b0ed0 100644
--- a/revision.h
+++ b/revision.h
@@ -171,6 +171,7 @@ struct rev_info {
 			verbose_header:1,
 			ignore_merges:1,
 			combine_merges:1,
+			combined_with_paths:1,
 			dense_combined_merges:1,
 			always_show_header:1;
 
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index e2824d3437..9420c1318a 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -435,4 +435,40 @@ test_expect_success 'combine diff gets tree sorting right' '
 	test_cmp expect actual
 '
 
+test_expect_success 'setup for --combined-with-paths' '
+	git branch side1c &&
+	git branch side2c &&
+	git checkout side1c &&
+	test_seq 1 10 >$(printf "file\twith\ttabs") &&
+	git add file* &&
+	git commit -m with &&
+	git checkout side2c &&
+	test_seq 1 9 >$(printf "i\tam\ttabbed") &&
+	echo ten >>$(printf "i\tam\ttabbed") &&
+	git add *tabbed &&
+	git commit -m iam &&
+	git checkout -b mergery side1c &&
+	git merge --no-commit side2c &&
+	git rm *tabs &&
+	echo eleven >>$(printf "i\tam\ttabbed") &&
+	git mv "$(printf "i\tam\ttabbed")" "$(printf "fickle\tnaming")" &&
+	git add fickle* &&
+	git commit
+'
+
+test_expect_success '--combined-with-paths shows all paths involved' '
+	cat <<-\EOF >expect &&
+	::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR	"file\twith\ttabs"	"i\tam\ttabbed"	"fickle\tnaming"
+	EOF
+	git diff-tree --combined-with-paths HEAD >actual.tmp &&
+	sed 1d <actual.tmp >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--combined-with-paths works with -z as well' '
+	printf "0f9645804ebb04cc3eef91f799eb7fb54d70cefb\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\0file\twith\ttabs\0i\tam\ttabbed\0fickle\tnaming\0" >expect &&
+	git diff-tree --combined-with-paths -z HEAD >actual &&
+	test_cmp -a expect actual
+'
+
 test_done
-- 
2.20.1.310.g29e9ddf532


  parent reply	other threads:[~2019-01-25 17:12 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-24 16:46 [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames Elijah Newren
2019-01-25  2:19 ` brian m. carlson
2019-01-25 16:27   ` Elijah Newren
2019-01-25 14:45 ` Derrick Stolee
2019-01-25 16:30   ` Elijah Newren
2019-01-25 16:54 ` Elijah Newren [this message]
2019-01-25 17:40   ` [PATCH v2] " Derrick Stolee
2019-01-25 17:52     ` Elijah Newren
2019-01-25 19:51       ` Eric Sunshine
2019-01-26 22:07         ` Elijah Newren
2019-01-27  1:52       ` brian m. carlson
2019-01-26 22:18   ` [PATCH v3] log,diff-tree: add --combined-all-names option Elijah Newren
2019-02-04 20:07     ` [PATCH v4] " Elijah Newren
2019-02-04 21:20       ` Junio C Hamano
2019-02-05 15:51         ` Elijah Newren
2019-02-05 20:39           ` Junio C Hamano
2019-02-07 19:50             ` Elijah Newren
2019-02-07 20:25               ` Junio C Hamano
2019-02-07 22:10                 ` Elijah Newren
2019-02-07 23:31                   ` Junio C Hamano
2019-02-07 23:48                     ` Elijah Newren
2019-02-05  9:48       ` Johannes Schindelin
2019-02-05 15:54         ` Elijah Newren
2019-02-05 18:04           ` Junio C Hamano
2019-02-07 22:28       ` Junio C Hamano
2019-02-07 23:48         ` Elijah Newren
2019-02-08  1:12       ` [PATCH v5 0/2] add --combined-all-paths option to log and diff-tree Elijah Newren
2019-02-08  1:12         ` [PATCH v5 1/2] log,diff-tree: add --combined-all-paths option Elijah Newren
2019-02-08  4:00           ` Junio C Hamano
2019-02-08  6:52             ` Elijah Newren
2019-02-08 17:50               ` Junio C Hamano
2019-02-08  1:12         ` [PATCH v5 2/2] squash! " Elijah Newren
2019-02-08  4:14           ` Junio C Hamano
2019-02-08  6:48             ` Elijah Newren
2019-01-25 19:29 ` [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames Junio C Hamano
2019-01-25 20:04   ` Elijah Newren
2019-01-25 22:21     ` Junio C Hamano
2019-01-26 22:12       ` Elijah Newren
2019-01-28  0:19         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190125165416.17473-1-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).