diff: support ^! for merges

revision.c::handle_revision_arg_1() resolves <rev>^! by first adding the
negated parents and then <rev> itself.  builtin_diff_combined() expects
the first tree to be the merge and the remaining ones to be the parents,
though.  This mismatch results in bogus diff output.

Remember the first tree that doesn't belong to a parent and use it
instead of blindly picking the first one.  This makes "git diff <rev>^!"
consistent with "git show <rev>^!".

Reported-by: Tim Jaacks <tim.jaacks@garz-fricke.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 6236c75c9b826a315e553ead169b7fc3fb183c72..44748fa802f8e4b76634d1660e0448d199bc02b9 100644 (file)
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -79,10 +79,10 @@ If --merge-base is given, use the merge base of the two commits for the
 
        This form is to view the results of a merge commit.  The first
        listed <commit> must be the merge itself; the remaining two or
-       more commits should be its parents.  A convenient way to produce
-       the desired set of revisions is to use the `^@` suffix.
-       For instance, if `master` names a merge commit, `git diff master
-       master^@` gives the same combined diff as `git show master`.
+       more commits should be its parents.  Convenient ways to produce
+       the desired set of revisions are to use the suffixes `^@` and
+       `^!`.  If A is a merge commit, then `git diff A A^@`,
+       `git diff A^!` and `git show A` all give the same combined diff.
 
 'git diff' [<options>] <commit>..<commit> [--] [<path>...]::
 

diff --git a/builtin/diff.c b/builtin/diff.c
index 54bb3de964cb5a1e815702a8cbcedab166b96ee3..0e49919735ffda44e5839d730dc389b09ed739e6 100644 (file)
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -209,7 +209,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 static int builtin_diff_combined(struct rev_info *revs,
                                 int argc, const char **argv,
                                 struct object_array_entry *ent,
-                                int ents)
+                                int ents, int first_non_parent)
 {
        struct oid_array parents = OID_ARRAY_INIT;
        int i;
@@ -217,11 +217,18 @@ static int builtin_diff_combined(struct rev_info *revs,
        if (argc > 1)
                usage(builtin_diff_usage);
 
+       if (first_non_parent < 0)
+               die(_("no merge given, only parents."));
+       if (first_non_parent >= ents)
+               BUG("first_non_parent out of range: %d", first_non_parent);
+
        diff_merges_set_dense_combined_if_unset(revs);
 
-       for (i = 1; i < ents; i++)
-               oid_array_append(&parents, &ent[i].item->oid);
-       diff_tree_combined(&ent[0].item->oid, &parents, revs);
+       for (i = 0; i < ents; i++) {
+               if (i != first_non_parent)
+                       oid_array_append(&parents, &ent[i].item->oid);
+       }
+       diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);
        oid_array_clear(&parents);
        return 0;
 }
@@ -385,6 +392,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
        int i;
        struct rev_info rev;
        struct object_array ent = OBJECT_ARRAY_INIT;
+       int first_non_parent = -1;
        int blobs = 0, paths = 0;
        struct object_array_entry *blob[2];
        int nongit = 0, no_index = 0;
@@ -543,6 +551,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
                                continue;
                        obj->flags |= flags;
                        add_object_array(obj, name, &ent);
+                       if (first_non_parent < 0 &&
+                           (i >= rev.cmdline.nr || /* HEAD by hand. */
+                            rev.cmdline.rev[i].whence != REV_CMD_PARENTS_ONLY))
+                               first_non_parent = ent.nr - 1;
                } else if (obj->type == OBJ_BLOB) {
                        if (2 <= blobs)
                                die(_("more than two blobs given: '%s'"), name);
@@ -590,7 +602,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
                                           &ent.objects[0], &ent.objects[1]);
        } else
                result = builtin_diff_combined(&rev, argc, argv,
-                                              ent.objects, ent.nr);
+                                              ent.objects, ent.nr,
+                                              first_non_parent);
        result = diff_result_code(&rev.diffopt, result);
        if (1 < rev.diffopt.skip_stat_unmatch)
                refresh_index_quietly();

diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 9a292bac70c248c9273bf29a94d5ce39f17551df..2ce26e585c98c1a3045dee4545763f160f59aa82 100755 (executable)
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -80,11 +80,21 @@ test_expect_success 'check combined output (1)' '
        verify_helper sidewithone
 '
 
+test_expect_success 'check combined output (1) with git diff <rev>^!' '
+       git diff sidewithone^! -- >sidewithone &&
+       verify_helper sidewithone
+'
+
 test_expect_success 'check combined output (2)' '
        git show sidesansone -- >sidesansone &&
        verify_helper sidesansone
 '
 
+test_expect_success 'check combined output (2) with git diff <rev>^!' '
+       git diff sidesansone^! -- >sidesansone &&
+       verify_helper sidesansone
+'
+
 test_expect_success 'diagnose truncated file' '
        >file &&
        git add file &&