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: Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH 4/4] builtin/diff: learn --merge-base
Date: Sat,  5 Sep 2020 12:08:21 -0700	[thread overview]
Message-ID: <231ba3f661cc4aa7a55c44e339e187c6d70c5507.1599332861.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1599332861.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.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-diff.txt | 24 ++++++++++--
 builtin/diff.c             | 59 ++++++++++++++++++++++++++++
 t/t4068-diff-symmetric.sh  | 79 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 8f7b4ed3ca..0031729794 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git diff' [<options>] [<commit>] [--] [<path>...]
 'git diff' [<options>] --cached [<commit>] [--] [<path>...]
 'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...]
-'git diff' [<options>] <commit>...<commit> [--] [<path>...]
+'git diff' [<options>] --merge-base [--cached] [<commit> [<commit>]] [--] [<path>...]
 'git diff' [<options>] <blob> <blob>
 'git diff' [<options>] --no-index [--] <path> <path>
 
@@ -63,6 +63,24 @@ files on disk.
 	This is to view the changes between two arbitrary
 	<commit>.
 
+'git diff' [<options>] --merge-base [--cached] [<commit> [<commit>]] [--] [<path>...]::
+
+	In this form, the "before" side will be the merge base of the
+	two given commits.  If either commit is omitted, it will default
+	to HEAD.
++
+In the case where two commits are given, a diff is displayed between the
+merge base and the second commit.  `git diff --merge-base A B` is
+equivalent to `git diff $(git merge-base A B) B`.
++
+In the case where one commit is given, a diff is displayed between the
+merge base and the working tree or the index if `--cached` is given.
+`git diff --merge-base A` is equivalent to `git diff $(git merge-base A
+HEAD)`.
++
+In the case where no commits are given, this form behaves identically to
+as if no `--merge-base` were supplied.
+
 'git diff' [<options>] <commit> <commit>... <commit> [--] [<path>...]::
 
 	This form is to view the results of a merge commit.  The first
@@ -89,8 +107,8 @@ files on disk.
 
 Just in case you are doing something exotic, it should be
 noted that all of the <commit> in the above description, except
-in the last two forms that use `..` notations, can be any
-<tree>.
+in the `--merge-base` case and the last two forms that use `..`
+notations, can be any <tree>.
 
 For a more complete list of ways to spell <commit>, see
 "SPECIFYING REVISIONS" section in linkgit:gitrevisions[7].
diff --git a/builtin/diff.c b/builtin/diff.c
index 0e086ed7c4..0af5a6c8c9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -19,6 +19,7 @@
 #include "builtin.h"
 #include "submodule.h"
 #include "oid-array.h"
+#include "commit-reach.h"
 
 #define DIFF_NO_INDEX_EXPLICIT 1
 #define DIFF_NO_INDEX_IMPLICIT 2
@@ -371,6 +372,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	int blobs = 0, paths = 0;
 	struct object_array_entry *blob[2];
 	int nongit = 0, no_index = 0;
+	int merge_base = 0;
 	int result = 0;
 	struct symdiff sdiff;
 	struct option options[] = {
@@ -378,6 +380,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 			   N_("compare the given paths on the filesystem"),
 			   DIFF_NO_INDEX_EXPLICIT,
 			   PARSE_OPT_NONEG),
+		OPT_BOOL(0, "merge-base", &merge_base,
+			 N_("use the merge base between the two commits as the diff base")),
 		OPT_END(),
 	};
 
@@ -457,6 +461,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	rev.diffopt.flags.allow_external = 1;
 	rev.diffopt.flags.allow_textconv = 1;
 
+	if (no_index && merge_base)
+		die(_("--no-index and --merge-base are mutually exclusive"));
+
 	/* If this is a no-index diff, just run it and exit there. */
 	if (no_index)
 		exit(diff_no_index(&rev, no_index == DIFF_NO_INDEX_IMPLICIT,
@@ -513,6 +520,58 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	}
 
 	symdiff_prepare(&rev, &sdiff);
+
+	if (merge_base && rev.pending.nr) {
+		int i;
+		struct commit *mb_child[2] = {0};
+		struct commit_list *merge_bases;
+		int old_nr;
+
+		for (i = 0; i < rev.pending.nr; i++) {
+			struct object *obj = rev.pending.objects[i].item;
+			if (obj->flags)
+				die(_("--merge-base does not work with ranges"));
+			if (obj->type != OBJ_COMMIT)
+				die(_("--merge-base only works with commits"));
+		}
+
+		/*
+		 * This check must go after the for loop above because A...B
+		 * ranges produce three pending commits, resulting in a
+		 * misleading error message.
+		 */
+		if (rev.pending.nr > ARRAY_SIZE(mb_child))
+			die(_("--merge-base does not work with more than two commits"));
+
+		for (i = 0; i < rev.pending.nr; i++)
+			mb_child[i] = lookup_commit_reference(the_repository, &rev.pending.objects[i].item->oid);
+		if (rev.pending.nr < ARRAY_SIZE(mb_child)) {
+			struct object_id oid;
+
+			if (rev.pending.nr != 1)
+				BUG("unexpected rev.pending.nr: %d", rev.pending.nr);
+
+			if (get_oid("HEAD", &oid))
+				die(_("unable to get HEAD"));
+
+			mb_child[1] = lookup_commit_reference(the_repository, &oid);
+		}
+
+		merge_bases = repo_get_merge_bases(the_repository, mb_child[0], mb_child[1]);
+		if (!merge_bases)
+			die(_("no merge base found"));
+		if (merge_bases->next)
+			die(_("multiple merge bases found"));
+
+		old_nr = rev.pending.nr;
+		rev.pending.nr = 1;
+		object_array_pop(&rev.pending);
+		add_object_array(&merge_bases->item->object, oid_to_hex(&merge_bases->item->object.oid), &rev.pending);
+		rev.pending.nr = old_nr;
+
+		free_commit_list(merge_bases);
+	}
+
 	for (i = 0; i < rev.pending.nr; i++) {
 		struct object_array_entry *entry = &rev.pending.objects[i];
 		struct object *obj = entry->item;
diff --git a/t/t4068-diff-symmetric.sh b/t/t4068-diff-symmetric.sh
index 60c506c2b2..0e43ed7660 100755
--- a/t/t4068-diff-symmetric.sh
+++ b/t/t4068-diff-symmetric.sh
@@ -88,4 +88,83 @@ test_expect_success 'diff with ranges and extra arg' '
 	test_i18ngrep "usage" err
 '
 
+test_expect_success 'diff --merge-base with two commits' '
+	git diff commit-C master >expect &&
+	git diff --merge-base br2 master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff --merge-base with no commits' '
+	git diff --merge-base >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'diff --merge-base with one commit' '
+	git checkout master &&
+	git diff commit-C >expect &&
+	git diff --merge-base br2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff --merge-base with one commit and unstaged changes' '
+	git checkout master &&
+	test_when_finished git reset --hard &&
+	echo unstaged >>c &&
+	git diff commit-C >expect &&
+	git diff --merge-base br2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff --merge-base with one commit and staged and unstaged changes' '
+	git checkout master &&
+	test_when_finished git reset --hard &&
+	echo staged >>c &&
+	git add c &&
+	echo unstaged >>c &&
+	git diff commit-C >expect &&
+	git diff --merge-base br2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff --merge-base --cached with one commit and staged and unstaged changes' '
+	git checkout master &&
+	test_when_finished git reset --hard &&
+	echo staged >>c &&
+	git add c &&
+	echo unstaged >>c &&
+	git diff --cached commit-C >expect &&
+	git diff --cached --merge-base br2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff --merge-base with --no-index' '
+	test_must_fail git diff --merge-base --no-index expect actual 2>err &&
+	test_i18ngrep "fatal: --no-index and --merge-base are mutually exclusive" 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_expect_success 'diff --merge-base non-commit' '
+	test_must_fail git diff --merge-base master^{tree} 2>err &&
+	test_i18ngrep "fatal: --merge-base only works with commits" err
+'
+
+test_expect_success 'diff --merge-base with three commits' '
+	test_must_fail git diff --merge-base br1 br2 master 2>err &&
+	test_i18ngrep "fatal: --merge-base does not work with more than two commits" err
+'
+
+test_expect_success 'diff --merge-base with no merge bases' '
+	test_must_fail git diff --merge-base br2 br3 2>err &&
+	test_i18ngrep "fatal: no merge base found" err
+'
+
+test_expect_success 'diff --merge-base with multiple merge bases' '
+	test_must_fail git diff --merge-base master br1 2>err &&
+	test_i18ngrep "fatal: multiple merge bases found" err
+'
+
 test_done
-- 
2.28.0.rc0.135.gc7877b767d


  parent reply	other threads:[~2020-09-05 19:09 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-05 19:08 [PATCH 0/4] " 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 ` Denton Liu [this message]
2020-09-06 21:58   ` [PATCH 4/4] builtin/diff: learn --merge-base 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     ` [PATCH v3 09/10] builtin/diff-tree: " Denton Liu
2020-09-17 18:23       ` 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=231ba3f661cc4aa7a55c44e339e187c6d70c5507.1599332861.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --subject='Re: [PATCH 4/4] builtin/diff: 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).