Merge branch 'rs/diff-caret-bang-with-parents'

"git diff rev^!" did not show combined diff to go to the rev from
its parents.

* rs/diff-caret-bang-with-parents:
  diff: support ^! for merges
  revisions.txt: unspecify order of resolved parts of ^!
  revision: use strtol_i() for exclude_parent

5 files changed, 35 insertions, 13 deletions

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 85ae6d6d08..52b679256c 100644
--- 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/Documentation/revisions.txt b/Documentation/revisions.txt
index e3e350126d..0d2e55d781 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -363,7 +363,7 @@ Revision Range Summary
 
 '<rev>{caret}!', e.g. 'HEAD{caret}!'::
   A suffix '{caret}' followed by an exclamation mark is the same
-  as giving commit '<rev>' and then all its parents prefixed with
+  as giving commit '<rev>' and all its parents prefixed with
   '{caret}' to exclude them (and their ancestors).
 
 '<rev>{caret}-<n>', e.g. 'HEAD{caret}-, HEAD{caret}-2'::
diff --git a/builtin/diff.c b/builtin/diff.c
index 54bb3de964..0e49919735 100644
--- 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/revision.c b/revision.c
index 8f2623b3b5..0760e78936 100644
--- a/revision.c
+++ b/revision.c
@@ -2113,9 +2113,8 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl
 		int exclude_parent = 1;
 
 		if (mark[2]) {
-			char *end;
-			exclude_parent = strtoul(mark + 2, &end, 10);
-			if (*end != '\0' || !exclude_parent)
+			if (strtol_i(mark + 2, 10, &exclude_parent) ||
+			    exclude_parent < 1)
 				return -1;
 		}
 
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 9a292bac70..2ce26e585c 100755
--- 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 &&