In https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@xxxxxxxxxxxxxx/, it was reported that git range-diff does not handle commit ranges like rev^!. This patch series fixes that. Changes since v1: * In addition to git range-diff, git format-patch --range-diff gets the same improvement. * The comment talking about ^@ was removed. * The parsing was made a bit safer (e.g. catching ! by its own as an invalid range). Johannes Schindelin (3): range-diff/format-patch: refactor check for commit range range-diff/format-patch: handle commit ranges other than A..B range-diff(docs): explain how to specify commit ranges Documentation/git-range-diff.txt | 13 +++++++++++++ builtin/log.c | 2 +- builtin/range-diff.c | 9 +++++---- revision.c | 25 +++++++++++++++++++++++++ revision.h | 7 +++++++ t/t3206-range-diff.sh | 8 ++++++++ 6 files changed, 59 insertions(+), 5 deletions(-) base-commit: 71ca53e8125e36efbda17293c50027d31681a41f Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/841 Range-diff vs v1: 1: 5839ba4f761 ! 1: 3f21e10f919 range-diff: refactor check for commit range @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx> ## Commit message ## - range-diff: refactor check for commit range + range-diff/format-patch: refactor check for commit range - Currently, when called with exactly two arguments, we test for a literal - `..` in each of the two. + Currently, when called with exactly two arguments, `git range-diff` + tests for a literal `..` in each of the two. Likewise, the argument + provided via `--range-diff` to `git format-patch` is checked in the same + manner. However, `<commit>^!` is a perfectly valid commit range, equivalent to `<commit>^..<commit>` according to the `SPECIFYING RANGES` section of gitrevisions[7]. In preparation for allowing more sophisticated ways to specify commit - ranges, let's refactor the conditional into its own function. + ranges, let's refactor the check into its own function. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> + ## builtin/log.c ## +@@ builtin/log.c: static void infer_range_diff_ranges(struct strbuf *r1, + struct commit *head) + { + const char *head_oid = oid_to_hex(&head->object.oid); +- int prev_is_range = !!strstr(prev, ".."); ++ int prev_is_range = specifies_commit_range(prev); + + if (prev_is_range) + strbuf_addstr(r1, prev); + ## builtin/range-diff.c ## -@@ builtin/range-diff.c: N_("git range-diff [<options>] <base> <old-tip> <new-tip>"), - NULL - }; +@@ + #include "parse-options.h" + #include "range-diff.h" + #include "config.h" ++#include "revision.h" -+static int is_range(const char *range) -+{ -+ return !!strstr(range, ".."); -+} -+ - int cmd_range_diff(int argc, const char **argv, const char *prefix) - { - int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT; + static const char * const builtin_range_diff_usage[] = { + N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"), @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix) diffopt.use_color = 1; if (argc == 2) { - if (!strstr(argv[0], "..")) - die(_("no .. in range: '%s'"), argv[0]); -+ if (!is_range(argv[0])) ++ if (!specifies_commit_range(argv[0])) + die(_("not a commit range: '%s'"), argv[0]); strbuf_addstr(&range1, argv[0]); - if (!strstr(argv[1], "..")) - die(_("no .. in range: '%s'"), argv[1]); -+ if (!is_range(argv[1])) ++ if (!specifies_commit_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]); + + ## revision.c ## +@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit) + fputs(mark, stdout); + putchar(' '); + } ++ ++int specifies_commit_range(const char *range) ++{ ++ return !!strstr(range, ".."); ++} + + ## revision.h ## +@@ revision.h: int rewrite_parents(struct rev_info *revs, + */ + struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit); + ++/* ++ * Determine whether the given argument defines a commit range, e.g. A..B. ++ * Note that this only validates the format but does _not_ parse it, i.e. ++ * it does _not_ look up the specified commits in the local repository. ++ */ ++int specifies_commit_range(const char *range); ++ + #endif 2: 88c15617b4b ! 2: 2c2744333ec range-diff: handle commit ranges other than A..B @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx> ## Commit message ## - range-diff: handle commit ranges other than A..B + range-diff/format-patch: handle commit ranges other than A..B In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are described to specify commit ranges that `range-diff` does not yet @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> - ## builtin/range-diff.c ## -@@ builtin/range-diff.c: NULL + ## revision.c ## +@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit) - static int is_range(const char *range) + int specifies_commit_range(const char *range) { - return !!strstr(range, ".."); + size_t i; @@ builtin/range-diff.c: NULL + return 1; + + i = strlen(range); -+ c = i ? range[--i] : 0; ++ c = i > 2 ? range[--i] : 0; + if (c == '!') -+ i--; /* might be ...^! or ...^@ */ ++ i--; /* might be ...^! */ + else if (isdigit(c)) { + /* handle ...^-<n> */ + while (i > 2 && isdigit(range[--i])) @@ builtin/range-diff.c: NULL + } else + return 0; + ++ /* Before the `!` or the `-<n>`, we expect `<rev>^` */ + return i > 0 && range[i] == '^'; } - - int cmd_range_diff(int argc, const char **argv, const char *prefix) ## t/t3206-range-diff.sh ## @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' ' 3: 041456b6e73 = 3: 4f5e5acd954 range-diff(docs): explain how to specify commit ranges -- gitgitgadget
