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

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

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;

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;

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 &&