I just had the need to find out upstream commits corresponding to a handful of backported commits, and most importantly, identify upstream commits touching a given file that had not yet been backported. This new mode helped me identify them. Changes since v1: * The command-line parameter parsing now avoids duplicating code as much as possible. * This also fixes a bug where git range-diff <incorrect-symmetric-range> -- <pathspec> was mistaken for using the three-revision stanza. * Consistent validation of the command-line arguments has been extracted into its own patch. * Sadly, these changes make the overall diff much larger. I hope that the readability is worth that price. Johannes Schindelin (3): range-diff: reorder argument handling range-diff: consistently validate the arguments range-diff: optionally accept pathspecs Documentation/git-range-diff.txt | 4 ++ builtin/range-diff.c | 101 +++++++++++++++++++++++-------- range-diff.c | 2 +- t/t3206-range-diff.sh | 13 +++- 4 files changed, 94 insertions(+), 26 deletions(-) base-commit: 795ea8776befc95ea2becd8020c7a284677b4161 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1335%2Fdscho%2Frange-diff-with-pathspec-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1335/dscho/range-diff-with-pathspec-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1335 Range-diff vs v1: 1: 7fe4f228ae0 ! 1: 150f29a1c48 range-diff: reorder argument handling @@ Commit message In preparation for allowing pathspecs in `git range-diff` invocations, where we no longer have the luxury of using the number of arguments to disambiguate between these three different ways to specify the commit - ranges, we need to order these cases by argument count, in ascending + ranges, we need to order these cases by argument count, in descending order. This patch is best viewed with `--color-moved`. @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char 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]); -- -- if (!is_range_diff_range(argv[1])) -- die(_("not a commit range: '%s'"), argv[1]); -- strbuf_addstr(&range2, argv[1]); ++ if (argc == 3) { ++ strbuf_addf(&range1, "%s..%s", argv[0], argv[1]); ++ strbuf_addf(&range2, "%s..%s", argv[0], argv[2]); ++ } else if (argc == 2) { + if (!is_range_diff_range(argv[0])) + die(_("not a commit range: '%s'"), argv[0]); + strbuf_addstr(&range1, argv[0]); +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) + 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) { -+ if (argc == 1) { + } else if (argc == 1) { const char *b = strstr(argv[0], "..."), *a = argv[0]; int a_len; - -@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) - b = "HEAD"; - strbuf_addf(&range1, "%s..%.*s", b, a_len, a); - strbuf_addf(&range2, "%.*s..%s", a_len, a, b); -+ } else if (argc == 2) { -+ if (!is_range_diff_range(argv[0])) -+ die(_("not a commit range: '%s'"), argv[0]); -+ strbuf_addstr(&range1, argv[0]); -+ -+ 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 { - error(_("need two commit ranges")); - usage_with_options(builtin_range_diff_usage, options); -: ----------- > 2: 4cd7f09557c range-diff: consistently validate the arguments 2: 064b147451b ! 3: a52ad40e015 range-diff: optionally accept a pathspec @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx> ## Commit message ## - range-diff: optionally accept a pathspec + range-diff: optionally accept pathspecs The `git range-diff` command can be quite expensive, which is not a surprise given that the underlying algorithm to match up pairs of @@ Documentation/git-range-diff.txt: DESCRIPTION ## builtin/range-diff.c ## @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) + OPT_END() + }; struct option *options; - int res = 0; +- int res = 0; ++ int i, dash_dash = -1, res = 0; struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT; -+ struct object_id oid; -+ const char *p; + struct object_id oid; ++ const char *three_dots = NULL; git_config(git_diff_ui_config, NULL); @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char diff_setup_done(&diffopt); @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) - b = "HEAD"; - strbuf_addf(&range1, "%s..%.*s", b, a_len, a); - strbuf_addf(&range2, "%.*s..%s", a_len, a, b); -+ } else if (argc > 1 && (p = strstr(argv[0], "..."))) { -+ const char *a = argv[0]; -+ int a_len = (int)(p - a); -+ -+ if (!a_len) { -+ a = "HEAD"; -+ a_len = strlen(a); + if (!simple_color) + diffopt.use_color = 1; + +- if (argc == 3) { +- if (get_oid_committish(argv[0], &oid)) ++ for (i = 0; i < argc; i++) ++ if (!strcmp(argv[i], "--")) { ++ dash_dash = i; ++ break; + } -+ p += 3; -+ if (!*p) -+ p = "HEAD"; -+ strbuf_addf(&range1, "%s..%.*s", p, a_len, a); -+ strbuf_addf(&range2, "%.*s..%s", a_len, a, p); -+ strvec_pushv(&other_arg, argv + 1); - } else if (argc == 2) { - if (!is_range_diff_range(argv[0])) - die(_("not a commit range: '%s'"), argv[0]); ++ ++ 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]); @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) - if (!is_range_diff_range(argv[1])) - die(_("not a commit range: '%s'"), argv[1]); - strbuf_addstr(&range2, argv[1]); -+ } else if (argc > 2 && -+ is_range_diff_range(argv[0]) && is_range_diff_range(argv[1])) { -+ strbuf_addstr(&range1, argv[0]); -+ strbuf_addstr(&range2, argv[1]); -+ strvec_pushv(&other_arg, argv + 2); - } 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 > 3 && -+ get_oid_committish(argv[0], &oid) && -+ get_oid_committish(argv[1], &oid) && -+ get_oid_committish(argv[2], &oid)) { -+ strbuf_addf(&range1, "%s..%s", argv[0], argv[1]); -+ strbuf_addf(&range2, "%s..%s", argv[0], argv[2]); -+ strvec_pushv(&other_arg, argv + 3); - } else { - error(_("need two commit ranges")); - usage_with_options(builtin_range_diff_usage, options); +- } else if (argc == 2) { +- if (!is_range_diff_range(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]); +@@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) + + strbuf_addstr(&range1, argv[0]); + strbuf_addstr(&range2, argv[1]); +- } else if (argc == 1) { +- const char *b = strstr(argv[0], "..."), *a = argv[0]; ++ ++ 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) ++ 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]); ++ 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); ++ ++ strvec_pushv(&other_arg, argv + ++ (dash_dash < 0 ? 1 : dash_dash)); + } else + usage_msg_opt(_("need two commit ranges"), + builtin_range_diff_usage, options); ## range-diff.c ## @@ range-diff.c: static int read_patches(const char *range, struct string_list *list, @@ range-diff.c: static int read_patches(const char *range, struct string_list *lis cp.git_cmd = 1; ## t/t3206-range-diff.sh ## +@@ t/t3206-range-diff.sh: 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 + ' + @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' ' test_cmp expect actual ' -- gitgitgadget
