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
next prev 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] builtin/diff: learn --merge-base 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
* 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 \
/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.
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 NNTP newsgroup(s).