Merge branch 'js/range-diff-with-pathspec'

Allow passing a pathspec to "git range-diff".

* js/range-diff-with-pathspec:
  range-diff: optionally accept pathspecs
  range-diff: consistently validate the arguments
  range-diff: reorder argument handling

4 files changed, 94 insertions, 26 deletions

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index fe350d7f40..0b393715d7 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 	[--no-dual-color] [--creation-factor=<factor>]
 	[--left-only | --right-only]
 	( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
+	[[--] <path>...]
 
 DESCRIPTION
 -----------
@@ -19,6 +20,9 @@ DESCRIPTION
 This command shows the differences between two versions of a patch
 series, or more generally, two commit ranges (ignoring merge commits).
 
+In the presence of `<path>` arguments, these commit ranges are limited
+accordingly.
+
 To that end, it first finds pairs of commits from both commit ranges
 that correspond with each other. Two commits are said to correspond when
 the diff between their patches (i.e. the author information, the commit
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 50318849d6..e2a74efb42 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -38,8 +38,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 	struct option *options;
-	int res = 0;
+	int i, dash_dash = -1, res = 0;
 	struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;
+	struct object_id oid;
+	const char *three_dots = NULL;
 
 	git_config(git_diff_ui_config, NULL);
 
@@ -47,7 +49,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 
 	options = parse_options_concat(range_diff_options, diffopt.parseopts);
 	argc = parse_options(argc, argv, prefix, options,
-			     builtin_range_diff_usage, 0);
+			     builtin_range_diff_usage, PARSE_OPT_KEEP_DASHDASH);
 
 	diff_setup_done(&diffopt);
 
@@ -55,40 +57,91 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	if (!simple_color)
 		diffopt.use_color = 1;
 
-	if (argc == 2) {
-		if (!is_range_diff_range(argv[0]))
-			die(_("not a commit range: '%s'"), argv[0]);
-		strbuf_addstr(&range1, argv[0]);
+	for (i = 0; i < argc; i++)
+		if (!strcmp(argv[i], "--")) {
+			dash_dash = i;
+			break;
+		}
+
+	if (dash_dash == 3 ||
+	    (dash_dash < 0 && argc > 2 &&
+	     !get_oid_committish(argv[0], &oid) &&
+	     !get_oid_committish(argv[1], &oid) &&
+	     !get_oid_committish(argv[2], &oid))) {
+		if (dash_dash < 0)
+			; /* already validated arguments */
+		else if (get_oid_committish(argv[0], &oid))
+			usage_msg_optf(_("not a revision: '%s'"),
+				       builtin_range_diff_usage, options,
+				       argv[0]);
+		else if (get_oid_committish(argv[1], &oid))
+			usage_msg_optf(_("not a revision: '%s'"),
+				       builtin_range_diff_usage, options,
+				       argv[1]);
+		else if (get_oid_committish(argv[2], &oid))
+			usage_msg_optf(_("not a revision: '%s'"),
+				       builtin_range_diff_usage, options,
+				       argv[2]);
 
-		if (!is_range_diff_range(argv[1]))
-			die(_("not a commit range: '%s'"), argv[1]);
-		strbuf_addstr(&range2, argv[1]);
-	} else if (argc == 3) {
 		strbuf_addf(&range1, "%s..%s", argv[0], argv[1]);
 		strbuf_addf(&range2, "%s..%s", argv[0], argv[2]);
-	} else if (argc == 1) {
-		const char *b = strstr(argv[0], "..."), *a = argv[0];
+
+		strvec_pushv(&other_arg, argv +
+			     (dash_dash < 0 ? 3 : dash_dash));
+	} else if (dash_dash == 2 ||
+		   (dash_dash < 0 && argc > 1 &&
+		    is_range_diff_range(argv[0]) &&
+		    is_range_diff_range(argv[1]))) {
+		if (dash_dash < 0)
+			; /* already validated arguments */
+		else if (!is_range_diff_range(argv[0]))
+			usage_msg_optf(_("not a commit range: '%s'"),
+				       builtin_range_diff_usage, options,
+				       argv[0]);
+		else if (!is_range_diff_range(argv[1]))
+			usage_msg_optf(_("not a commit range: '%s'"),
+				       builtin_range_diff_usage, options,
+				       argv[1]);
+
+		strbuf_addstr(&range1, argv[0]);
+		strbuf_addstr(&range2, argv[1]);
+
+		strvec_pushv(&other_arg, argv +
+			     (dash_dash < 0 ? 2 : dash_dash));
+	} else if (dash_dash == 1 ||
+		   (dash_dash < 0 && argc > 0 &&
+		    (three_dots = strstr(argv[0], "...")))) {
+		const char *a, *b;
 		int a_len;
 
-		if (!b) {
-			error(_("single arg format must be symmetric range"));
-			usage_with_options(builtin_range_diff_usage, options);
-		}
+		if (dash_dash < 0)
+			; /* already validated arguments */
+		else if (!(three_dots = strstr(argv[0], "...")))
+			usage_msg_optf(_("not a symmetric range: '%s'"),
+					 builtin_range_diff_usage, options,
+					 argv[0]);
 
-		a_len = (int)(b - a);
-		if (!a_len) {
+		if (three_dots == argv[0]) {
 			a = "HEAD";
 			a_len = strlen(a);
+		} else {
+			a = argv[0];
+			a_len = (int)(three_dots - a);
 		}
-		b += 3;
-		if (!*b)
+
+		if (three_dots[3])
+			b = three_dots + 3;
+		else
 			b = "HEAD";
+
 		strbuf_addf(&range1, "%s..%.*s", b, a_len, a);
 		strbuf_addf(&range2, "%.*s..%s", a_len, a, b);
-	} else {
-		error(_("need two commit ranges"));
-		usage_with_options(builtin_range_diff_usage, options);
-	}
+
+		strvec_pushv(&other_arg, argv +
+			     (dash_dash < 0 ? 1 : dash_dash));
+	} else
+		usage_msg_opt(_("need two commit ranges"),
+			      builtin_range_diff_usage, options);
 	FREE_AND_NULL(options);
 
 	range_diff_opts.dual_color = simple_color < 1;
diff --git a/range-diff.c b/range-diff.c
index f63b3ffc20..124dd678c3 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -57,9 +57,9 @@ static int read_patches(const char *range, struct string_list *list,
 		     "--pretty=medium",
 		     "--notes",
 		     NULL);
+	strvec_push(&cp.args, range);
 	if (other_arg)
 		strvec_pushv(&cp.args, other_arg->v);
-	strvec_push(&cp.args, range);
 	cp.out = -1;
 	cp.no_stdin = 1;
 	cp.git_cmd = 1;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index d12e4e4cc6..459beaf7d9 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -162,7 +162,7 @@ test_expect_success 'A^! and A^-<n> (unmodified)' '
 '
 
 test_expect_success 'A^{/..} is not mistaken for a range' '
-	test_must_fail git range-diff topic^.. topic^{/..} 2>error &&
+	test_must_fail git range-diff topic^.. topic^{/..} -- 2>error &&
 	test_i18ngrep "not a commit range" error
 '
 
@@ -772,6 +772,17 @@ test_expect_success '--left-only/--right-only' '
 	test_cmp expect actual
 '
 
+test_expect_success 'ranges with pathspecs' '
+	git range-diff topic...mode-only-change -- other-file >actual &&
+	test_line_count = 2 actual &&
+	topic_oid=$(git rev-parse --short topic) &&
+	mode_change_oid=$(git rev-parse --short mode-only-change^) &&
+	file_change_oid=$(git rev-parse --short mode-only-change) &&
+	grep "$mode_change_oid" actual &&
+	! grep "$file_change_oid" actual &&
+	! grep "$topic_oid" actual
+'
+
 test_expect_success 'submodule changes are shown irrespective of diff.submodule' '
 	git init sub-repo &&
 	test_commit -C sub-repo sub-first &&