git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 09/10] builtin/diff-tree: learn --merge-base
Date: Thu, 17 Sep 2020 00:44:13 -0700	[thread overview]
Message-ID: <c0d27b125e969e13c52b0fa806a8e3caa8c20ac6.1600328336.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1600328335.git.liu.denton@gmail.com>

In order to get the diff between a commit and its merge base, the
currently preferred method is to use `git diff A...B`. However, the
range-notation with diff has, time and time again, been noted as a point
of confusion and thus, it should be avoided. Although we have a
substitute for the double-dot notation, we don't have any replacement
for the triple-dot notation.

Introduce the --merge-base flag as a replacement for triple-dot
notation. Thus, we would be able to write the above as
`git diff --merge-base A B`, allowing us to gently deprecate
range-notation completely.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-diff-tree.txt      |  7 ++++-
 Documentation/git-diff.txt           |  9 ++++---
 builtin/diff-tree.c                  | 18 +++++++++++++
 builtin/diff.c                       | 40 +++++++++++++++++++---------
 t/t4068-diff-symmetric-merge-base.sh | 34 +++++++++++++++++++++++
 5 files changed, 91 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 5c8a2a5e97..2fc24c542f 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git diff-tree' [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty]
-	      [-t] [-r] [-c | --cc] [--combined-all-paths] [--root]
+	      [-t] [-r] [-c | --cc] [--combined-all-paths] [--root] [--merge-base]
 	      [<common diff options>] <tree-ish> [<tree-ish>] [<path>...]
 
 DESCRIPTION
@@ -43,6 +43,11 @@ include::diff-options.txt[]
 	When `--root` is specified the initial commit will be shown as a big
 	creation event. This is equivalent to a diff against the NULL tree.
 
+--merge-base::
+	Instead of comparing the <tree-ish>s directly, use the merge
+	base between the two <tree-ish>s as the "before" side.  There
+	must be two <tree-ish>s given and they must both be commits.
+
 --stdin::
 	When `--stdin` is specified, the command does not take
 	<tree-ish> arguments from the command line.  Instead, it
diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 762ee6d074..d3b526e00a 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -11,8 +11,7 @@ SYNOPSIS
 [verse]
 'git diff' [<options>] [<commit>] [--] [<path>...]
 'git diff' [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]
-'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...]
-'git diff' [<options>] <commit>...<commit> [--] [<path>...]
+'git diff' [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]
 'git diff' [<options>] <blob> <blob>
 'git diff' [<options>] --no-index [--] <path> <path>
 
@@ -62,10 +61,14 @@ of <commit> and HEAD.  `git diff --merge-base A` is equivalent to
 	branch name to compare with the tip of a different
 	branch.
 
-'git diff' [<options>] <commit> <commit> [--] [<path>...]::
+'git diff' [<options>] [--merge-base] <commit> <commit> [--] [<path>...]::
 
 	This is to view the changes between two arbitrary
 	<commit>.
++
+If --merge-base is given, use the merge base of the two commits for the
+"before" side.  `git diff --merge-base A B` is equivalent to
+`git diff $(git merge-base A B) B`.
 
 'git diff' [<options>] <commit> <commit>... <commit> [--] [<path>...]::
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 802363d0a2..823d6678e5 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -111,6 +111,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	struct setup_revision_opt s_r_opt;
 	struct userformat_want w;
 	int read_stdin = 0;
+	int merge_base = 0;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(diff_tree_usage);
@@ -143,9 +144,26 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 			read_stdin = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--merge-base")) {
+			merge_base = 1;
+			continue;
+		}
 		usage(diff_tree_usage);
 	}
 
+	if (read_stdin && merge_base)
+		die(_("--stdin and --merge-base are mutually exclusive"));
+
+	if (merge_base) {
+		struct object_id oid;
+
+		if (opt->pending.nr != 2)
+			die(_("--merge-base only works with two commits"));
+
+		diff_get_merge_base(opt, &oid);
+		opt->pending.objects[0].item = lookup_object(the_repository, &oid);
+	}
+
 	/*
 	 * NOTE!  We expect "a..b" to expand to "^a b" but it is
 	 * perfectly valid for revision range parser to yield "b ^a",
diff --git a/builtin/diff.c b/builtin/diff.c
index 1baea18ae0..ad78bc89b3 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -26,8 +26,7 @@
 static const char builtin_diff_usage[] =
 "git diff [<options>] [<commit>] [--] [<path>...]\n"
 "   or: git diff [<options>] --cached [<commit>] [--] [<path>...]\n"
-"   or: git diff [<options>] <commit> [<commit>...] <commit> [--] [<path>...]\n"
-"   or: git diff [<options>] <commit>...<commit>] [--] [<path>...]\n"
+"   or: git diff [<options>] <commit> [--merge-base] [<commit>...] <commit> [--] [<path>...]\n"
 "   or: git diff [<options>] <blob> <blob>]\n"
 "   or: git diff [<options>] --no-index [--] <path> <path>]\n"
 COMMON_DIFF_OPTIONS_HELP;
@@ -172,19 +171,34 @@ static int builtin_diff_tree(struct rev_info *revs,
 			     struct object_array_entry *ent1)
 {
 	const struct object_id *(oid[2]);
-	int swap = 0;
+	struct object_id mb_oid;
+	int merge_base = 0;
 
-	if (argc > 1)
-		usage(builtin_diff_usage);
+	while (1 < argc) {
+		const char *arg = argv[1];
+		if (!strcmp(arg, "--merge-base"))
+			merge_base = 1;
+		else
+			usage(builtin_diff_usage);
+		argv++; argc--;
+	}
 
-	/*
-	 * We saw two trees, ent0 and ent1.  If ent1 is uninteresting,
-	 * swap them.
-	 */
-	if (ent1->item->flags & UNINTERESTING)
-		swap = 1;
-	oid[swap] = &ent0->item->oid;
-	oid[1 - swap] = &ent1->item->oid;
+	if (merge_base) {
+		diff_get_merge_base(revs, &mb_oid);
+		oid[0] = &mb_oid;
+		oid[1] = &revs->pending.objects[1].item->oid;
+	} else {
+		int swap = 0;
+
+		/*
+		 * We saw two trees, ent0 and ent1.  If ent1 is uninteresting,
+		 * swap them.
+		 */
+		if (ent1->item->flags & UNINTERESTING)
+			swap = 1;
+		oid[swap] = &ent0->item->oid;
+		oid[1 - swap] = &ent1->item->oid;
+	}
 	diff_tree_oid(oid[0], oid[1], "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
diff --git a/t/t4068-diff-symmetric-merge-base.sh b/t/t4068-diff-symmetric-merge-base.sh
index 49432379cb..03487cc945 100755
--- a/t/t4068-diff-symmetric-merge-base.sh
+++ b/t/t4068-diff-symmetric-merge-base.sh
@@ -156,4 +156,38 @@ do
 	'
 done
 
+for cmd in diff-tree diff
+do
+	test_expect_success "$cmd --merge-base with two commits" '
+		git $cmd commit-C master >expect &&
+		git $cmd --merge-base br2 master >actual &&
+		test_cmp expect actual
+	'
+
+	test_expect_success "$cmd --merge-base commit and non-commit" '
+		test_must_fail git $cmd --merge-base br2 master^{tree} 2>err &&
+		test_i18ngrep "fatal: --merge-base only works with commits" err
+	'
+
+	test_expect_success "$cmd --merge-base with no merge bases and two commits" '
+		test_must_fail git $cmd --merge-base br2 br3 2>err &&
+		test_i18ngrep "fatal: no merge base found" err
+	'
+
+	test_expect_success "$cmd --merge-base with multiple merge bases and two commits" '
+		test_must_fail git $cmd --merge-base master br1 2>err &&
+		test_i18ngrep "fatal: multiple merge bases found" err
+	'
+done
+
+test_expect_success 'diff-tree --merge-base with one commit' '
+	test_must_fail git diff-tree --merge-base master 2>err &&
+	test_i18ngrep "fatal: --merge-base only works with two commits" err
+'
+
+test_expect_success 'diff --merge-base with range' '
+	test_must_fail git diff --merge-base br2..br3 2>err &&
+	test_i18ngrep "fatal: --merge-base does not work with ranges" err
+'
+
 test_done
-- 
2.28.0.618.gf4bc123cb7


  parent reply	other threads:[~2020-09-17  7:45 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-05 19:08 [PATCH 0/4] builtin/diff: " Denton Liu
2020-09-05 19:08 ` [PATCH 1/4] t4068: remove unnecessary >tmp Denton Liu
2020-09-05 19:08 ` [PATCH 2/4] git-diff.txt: backtick quote command text Denton Liu
2020-09-05 19:08 ` [PATCH 3/4] builtin/diff: parse --no-index using parse_options() Denton Liu
2020-09-05 19:08 ` [PATCH 4/4] builtin/diff: learn --merge-base Denton Liu
2020-09-06 21:58   ` Junio C Hamano
2020-09-10  7:32 ` [PATCH v2 0/4] " Denton Liu
2020-09-10  7:32   ` [PATCH v2 1/4] t4068: remove unnecessary >tmp Denton Liu
2020-09-10  7:32   ` [PATCH v2 2/4] git-diff.txt: backtick quote command text Denton Liu
2020-09-10  7:32   ` [PATCH v2 3/4] builtin/diff: parse --no-index using parse_options() Denton Liu
2020-09-10 18:35     ` Junio C Hamano
2020-09-13  8:31       ` Denton Liu
2020-09-13 21:45         ` Junio C Hamano
2020-09-10  7:32   ` [PATCH v2 4/4] builtin/diff: learn --merge-base Denton Liu
2020-09-17  7:44   ` [PATCH v3 00/10] " Denton Liu
2020-09-17  7:44     ` [PATCH v3 01/10] t4068: remove unnecessary >tmp Denton Liu
2020-09-17  7:44     ` [PATCH v3 02/10] git-diff-index.txt: make --cached description a proper sentence Denton Liu
2020-09-17  7:44     ` [PATCH v3 03/10] git-diff.txt: backtick quote command text Denton Liu
2020-09-17  7:44     ` [PATCH v3 04/10] contrib/completion: extract common diff/difftool options Denton Liu
2020-09-17  7:44     ` [PATCH v3 05/10] diff-lib: accept option flags in run_diff_index() Denton Liu
2020-09-17 17:00       ` Junio C Hamano
2020-09-17  7:44     ` [PATCH v3 06/10] diff-lib: define diff_get_merge_base() Denton Liu
2020-09-17 17:16       ` Junio C Hamano
2020-09-18 10:34         ` Denton Liu
2020-09-19  0:33           ` Junio C Hamano
2020-09-17  7:44     ` [PATCH v3 07/10] t4068: add --merge-base tests Denton Liu
2020-09-17  7:44     ` [PATCH v3 08/10] builtin/diff-index: learn --merge-base Denton Liu
2020-09-17 17:28       ` Junio C Hamano
2020-09-17 18:13         ` Jeff King
2020-09-18  5:11           ` Junio C Hamano
2020-09-18 18:12             ` Jeff King
2020-09-17  7:44     ` Denton Liu [this message]
2020-09-17 18:23       ` [PATCH v3 09/10] builtin/diff-tree: " Junio C Hamano
2020-09-18 10:48         ` Denton Liu
2020-09-18 16:52           ` Junio C Hamano
2020-09-20 11:01             ` Denton Liu
2020-09-21 16:05               ` Junio C Hamano
2020-09-21 17:27                 ` Denton Liu
2020-09-21 21:09                   ` Junio C Hamano
2020-09-21 21:19                     ` Junio C Hamano
2020-09-21 21:54                     ` Denton Liu
2020-09-21 22:18                       ` Junio C Hamano
2020-09-23  9:47                         ` Denton Liu
2020-09-25 21:02                           ` Junio C Hamano
2020-09-26  1:52                             ` Denton Liu
2020-09-17  7:44     ` [PATCH v3 10/10] contrib/completion: complete `git diff --merge-base` Denton Liu
2020-09-20 11:22     ` [PATCH v4 00/10] builtin/diff: learn --merge-base Denton Liu
2020-09-20 11:22       ` [PATCH v4 01/10] t4068: remove unnecessary >tmp Denton Liu
2020-09-20 11:22       ` [PATCH v4 02/10] git-diff-index.txt: make --cached description a proper sentence Denton Liu
2020-09-20 11:22       ` [PATCH v4 03/10] git-diff.txt: backtick quote command text Denton Liu
2020-09-20 11:22       ` [PATCH v4 04/10] contrib/completion: extract common diff/difftool options Denton Liu
2020-09-20 11:22       ` [PATCH v4 05/10] diff-lib: accept option flags in run_diff_index() Denton Liu
2020-09-20 11:22       ` [PATCH v4 06/10] diff-lib: define diff_get_merge_base() Denton Liu
2020-09-20 11:22       ` [PATCH v4 07/10] t4068: add --merge-base tests Denton Liu
2020-09-20 11:22       ` [PATCH v4 08/10] builtin/diff-index: learn --merge-base Denton Liu
2020-09-29 19:53         ` Martin Ågren
2020-09-20 11:22       ` [PATCH v4 09/10] builtin/diff-tree: " Denton Liu
2020-09-20 11:22       ` [PATCH v4 10/10] contrib/completion: complete `git diff --merge-base` Denton Liu

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=c0d27b125e969e13c52b0fa806a8e3caa8c20ac6.1600328336.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH v3 09/10] builtin/diff-tree: learn --merge-base' \
    /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

Code repositories for project(s) associated with this 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).