[v2,0/3] Range diff with ranges lacking dotdot

Message

Jean-Noël Avila via GitGitGadget Jan. 22, 2021, 6:16 p.m. UTC

In
https://lore.kernel.org/git/[e-mail]/,
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 <[e-mail]>
      
       ## 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 <[e-mail]>
      
     + ## 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 <[e-mail]>
      
       ## 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 <[e-mail]>
      
     - ## 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