[PATCH 0/3] Range diff with ranges lacking dotdot

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Johannes Schindelin (3):
  range-diff: refactor check for commit range
  range-diff: handle commit ranges other than A..B
  range-diff(docs): explain how to specify commit ranges

 Documentation/git-range-diff.txt | 13 +++++++++++++
 builtin/range-diff.c             | 32 ++++++++++++++++++++++++++++----
 t/t3206-range-diff.sh            |  8 ++++++++
 3 files changed, 49 insertions(+), 4 deletions(-)


base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/841
-- 
gitgitgadget

From: Johannes Schindelin <redacted>

Currently, when called with exactly two arguments, we test for a literal
`..` in each of the two.

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.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/range-diff.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f744..551d3e689cb 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -11,6 +11,11 @@ N_("git range-diff [<options>] <base> <old-tip> <new-tip>"),
 NULL
 };
 
+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;

@@ -46,12 +51,12 @@ 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]))
+			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]))
+			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]);

-- 
gitgitgadget

Hi Dscho

On 21/01/2021 22:20, Johannes Schindelin via GitGitGadget wrote:

If the user wrongly passes two arguments referring to single commits 
with `:/<text>` or `@{/<text>}` where text contains ".." this will give 
a false positive.

Best Wishes

Phillip

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/range-diff.c  | 21 ++++++++++++++++++++-
 t/t3206-range-diff.sh |  8 ++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 551d3e689cb..6097635c432 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -13,7 +13,26 @@ NULL
 
 static int is_range(const char *range)
 {
-	return !!strstr(range, "..");
+	size_t i;
+	char c;
+
+	if (strstr(range, ".."))
+		return 1;
+
+	i = strlen(range);
+	c = i ? range[--i] : 0;
+	if (c == '!')
+		i--; /* might be ...^! or ...^@ */
+	else if (isdigit(c)) {
+		/* handle ...^-<n> */
+		while (i > 2 && isdigit(range[--i]))
+			; /* keep trimming trailing digits */
+		if (i < 2 || range[i--] != '-')
+			return 0;
+	} else
+		return 0;
+
+	return i > 0 && range[i] == '^';
 }
 
 int cmd_range_diff(int argc, const char **argv, const char *prefix)

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be031..e217cecac9e 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

On Thu, Jan 21, 2021 at 5:22 PM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:

Is this something that the --range-diff option of git-format-patch
will want to do, as well? At present,
builtin/log.c:infer_range_diff_ranges() detects a range only by
checking for "..", much like this function did before this patch. If
so, perhaps this function can be part of the public range-diff API
(or, indeed, part of some other more general API if it's not really
specific to range-diff).

Hi Eric,

On Thu, 21 Jan 2021, Eric Sunshine wrote:

On Thu, Jan 21, 2021 at 5:22 PM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:

quoted

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them.

Signed-off-by: Johannes Schindelin <redacted>
---

diff --git a/builtin/range-diff.c b/builtin/range-diff.c

@@ -13,7 +13,26 @@ NULL
 static int is_range(const char *range)
 {
+       if (strstr(range, ".."))
+               return 1;
+
+       i = strlen(range);
+       c = i ? range[--i] : 0;
+       if (c == '!')
+               i--; /* might be ...^! or ...^@ */
+       else if (isdigit(c)) {
+               /* handle ...^-<n> */
+               while (i > 2 && isdigit(range[--i]))
+                       ; /* keep trimming trailing digits */
+               if (i < 2 || range[i--] != '-')
+                       return 0;
+       } else
+               return 0;
+
+       return i > 0 && range[i] == '^';
 }

Is this something that the --range-diff option of git-format-patch
will want to do, as well?

Thank you for pointing that out. I should have checked via `git grep
'strstr.*"\.\."'` myself. There are two more instances, one in
`rev-parse.c` and the other in `revision.c`, but both are necessary as-are
because their return value is actually used to further disect a `..`-style
commit range.

Thanks,
Dscho

At present, builtin/log.c:infer_range_diff_ranges() detects a range only
by checking for "..", much like this function did before this patch. If
so, perhaps this function can be part of the public range-diff API (or,
indeed, part of some other more general API if it's not really specific
to range-diff).

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd..76359baf26d 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
+  equivalent to `<base>..<rev1> <base>..<rev2>` where `<base>` is the
+  merge base as obtained via `git merge-base <rev1> <rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

On Thu, Jan 21, 2021 at 10:20:38PM +0000, Johannes Schindelin via GitGitGadget wrote:

git-log takes a range, too. There you can specify a single rev (with the
semantic to list all commits from this rev up (or down?) to the root).
So <rev> means implicitly <rev>^∞..<rev> for git-log.

Does it make sense to implement this here, too? Maybe this even allows
sharing some more code?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Hi Uwe,

On Fri, 22 Jan 2021, Uwe Kleine-König wrote:

On Thu, Jan 21, 2021 at 10:20:38PM +0000, Johannes Schindelin via GitGitGadget wrote:

quoted

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd..76359baf26d 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.

+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
+  equivalent to `<base>..<rev1> <base>..<rev2>` where `<base>` is the
+  merge base as obtained via `git merge-base <rev1> <rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.

git-log takes a range, too. There you can specify a single rev (with the
semantic to list all commits from this rev up (or down?) to the root).
So <rev> means implicitly <rev>^∞..<rev> for git-log.

Does it make sense to implement this here, too? Maybe this even allows
sharing some more code?

I don't think that it makes sense to support open-ended ranges. `git
range-diff` is expensive, its runtime is proportional to the number of
patches in the first range times the number of patches in the second
range. Allowing open-ended ranges will simply allow users to be stuck with
a long runtime by mistake, and I do not see any valid use case in return
for that risk.

Ciao,
Johannes

Hello Johannes,

On Thu, Jan 21, 2021 at 10:20:35PM +0000, Johannes Schindelin via GitGitGadget wrote:

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Oh wow, great you picked that up. It still occasionally annoys me that
the range parser of range-diff cannot do $hash^!.

I assume you will fix the stuff Junio pointed out (or argument your
patch 1 is right, I didn't try to check). When this is discussed to an
end or fixed I will happily test your series.

And by the way you also made one of my coworkers happy with your series.

Best regards and thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
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 [off-list ref]
      
       ## 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 [off-list ref]
      
     + ## 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 [off-list ref]
      
       ## 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 [off-list ref]
      
     - ## 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

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them.

Signed-off-by: Johannes Schindelin <redacted>
---
 revision.c            | 22 +++++++++++++++++++++-
 t/t3206-range-diff.sh |  8 ++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 00675f598a3..9ee063a2c03 100644
--- a/revision.c
+++ b/revision.c

@@ -4209,5 +4209,25 @@ void put_revision_mark(const struct rev_info *revs, const struct commit *commit)
 
 int specifies_commit_range(const char *range)
 {
-	return !!strstr(range, "..");
+	size_t i;
+	char c;
+
+	if (strstr(range, ".."))
+		return 1;
+
+	i = strlen(range);
+	c = i > 2 ? range[--i] : 0;
+	if (c == '!')
+		i--; /* might be ...^! */
+	else if (isdigit(c)) {
+		/* handle ...^-<n> */
+		while (i > 2 && isdigit(range[--i]))
+			; /* keep trimming trailing digits */
+		if (i < 2 || range[i--] != '-')
+			return 0;
+	} else
+		return 0;
+
+	/* Before the `!` or the `-<n>`, we expect `<rev>^` */
+	return i > 0 && range[i] == '^';
 }

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be031..e217cecac9e 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd..76359baf26d 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
+  equivalent to `<base>..<rev1> <base>..<rev2>` where `<base>` is the
+  merge base as obtained via `git merge-base <rev1> <rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

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 check into its own function.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/log.c        | 2 +-
 builtin/range-diff.c | 9 +++++----
 revision.c           | 5 +++++
 revision.h           | 7 +++++++
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index bd6ff4f9f95..099abdfb7e6 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1680,7 +1680,7 @@ 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);

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f744..89d54158011 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -3,6 +3,7 @@
 #include "parse-options.h"
 #include "range-diff.h"
 #include "config.h"
+#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),

@@ -46,12 +47,12 @@ 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 (!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 (!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]);

diff --git a/revision.c b/revision.c
index 9dff845bed6..00675f598a3 100644
--- a/revision.c
+++ b/revision.c

@@ -4206,3 +4206,8 @@ 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, "..");
+}

diff --git a/revision.h b/revision.h
index 086ff10280d..66777c8e60f 100644
--- a/revision.h
+++ b/revision.h

@@ -457,4 +457,11 @@ 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

-- 
gitgitgadget

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Changes since v2:

 * Move the helper function from revision.c to range-diff.c and rename it.
 * Use a regex to make it easier to understand what we're trying to match.
 * Fix the documentation that claimed that we used git merge-base internally
   when git range-diff parses ...-style arguments, which is not the case.

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 | 12 ++++++++++++
 builtin/log.c                    |  2 +-
 builtin/range-diff.c             |  9 +++++----
 range-diff.c                     | 17 +++++++++++++++++
 range-diff.h                     |  8 ++++++++
 t/t3206-range-diff.sh            |  8 ++++++++
 6 files changed, 51 insertions(+), 5 deletions(-)


base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/841

Range-diff vs v2:

 1:  3f21e10f919 ! 1:  b98fa94b870 range-diff/format-patch: refactor check for commit range
     @@ builtin/log.c: static void infer_range_diff_ranges(struct strbuf *r1,
       {
       	const char *head_oid = oid_to_hex(&head->object.oid);
      -	int prev_is_range = !!strstr(prev, "..");
     -+	int prev_is_range = specifies_commit_range(prev);
     ++	int prev_is_range = is_range_diff_range(prev);
       
       	if (prev_is_range)
       		strbuf_addstr(r1, prev);
     @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char
       	if (argc == 2) {
      -		if (!strstr(argv[0], ".."))
      -			die(_("no .. in range: '%s'"), argv[0]);
     -+		if (!specifies_commit_range(argv[0]))
     ++		if (!is_range_diff_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 (!specifies_commit_range(argv[1]))
     ++		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]);
      
     - ## revision.c ##
     -@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)
     - 	fputs(mark, stdout);
     - 	putchar(' ');
     + ## range-diff.c ##
     +@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
     + 
     + 	return res;
       }
      +
     -+int specifies_commit_range(const char *range)
     ++int is_range_diff_range(const char *arg)
      +{
     -+	return !!strstr(range, "..");
     ++	return !!strstr(arg, "..");
      +}
      
     - ## 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);
     + ## range-diff.h ##
     +@@ range-diff.h: int show_range_diff(const char *range1, const char *range2,
     + 		    const struct diff_options *diffopt,
     + 		    const struct strvec *other_arg);
       
      +/*
     -+ * 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.
     ++ * Determine whether the given argument is usable as a range argument of `git
     ++ * range-diff`, 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);
     ++int is_range_diff_range(const char *arg);
      +
       #endif
 2:  2c2744333ec ! 2:  0880ca587e6 range-diff/format-patch: handle commit ranges other than A..B
     @@ Commit message
      
          Signed-off-by: Johannes Schindelin [off-list ref]
      
     - ## revision.c ##
     -@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)
     + ## range-diff.c ##
     +@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
       
     - int specifies_commit_range(const char *range)
     + int is_range_diff_range(const char *arg)
       {
     --	return !!strstr(range, "..");
     -+	size_t i;
     -+	char c;
     +-	return !!strstr(arg, "..");
     ++	static regex_t *regex;
      +
     -+	if (strstr(range, ".."))
     ++	if (strstr(arg, ".."))
      +		return 1;
      +
     -+	i = strlen(range);
     -+	c = i > 2 ? range[--i] : 0;
     -+	if (c == '!')
     -+		i--; /* might be ...^! */
     -+	else if (isdigit(c)) {
     -+		/* handle ...^-<n> */
     -+		while (i > 2 && isdigit(range[--i]))
     -+			; /* keep trimming trailing digits */
     -+		if (i < 2 || range[i--] != '-')
     -+			return 0;
     -+	} else
     -+		return 0;
     ++	/* match `<rev>^!` and `<rev>^-<n>` */
     ++	if (!regex) {
     ++		regex = xmalloc(sizeof(*regex));
     ++		if (regcomp(regex, "\\^(!|-[0-9]*)$", REG_EXTENDED) < 0)
     ++			BUG("could not compile range-diff regex");
     ++	}
      +
     -+	/* Before the `!` or the `-<n>`, we expect `<rev>^` */
     -+	return i > 0 && range[i] == '^';
     ++	return !regexec(regex, arg, 0, NULL, 0);
       }
      
       ## t/t3206-range-diff.sh ##
 3:  4f5e5acd954 ! 3:  5ab9321a34c range-diff(docs): explain how to specify commit ranges
     @@ Documentation/git-range-diff.txt: Finally, the list of matching commits is shown
      +
      +- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
      +  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
     -+  equivalent to `<base>..<rev1> <base>..<rev2>` where `<base>` is the
     -+  merge base as obtained via `git merge-base <rev1> <rev2>`.
     ++  equivalent to `<rev2>..<rev1> <rev1>..<rev2>`.
      +
      +- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
      +  <base>..<rev2>`.

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd..14bffb272a0 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
+  equivalent to `<rev2>..<rev1> <rev1>..<rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them.

Signed-off-by: Johannes Schindelin <redacted>
---
 range-diff.c          | 14 +++++++++++++-
 t/t3206-range-diff.sh |  8 ++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/range-diff.c b/range-diff.c
index 9b93e08e840..0c6ac4f954d 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -567,5 +567,17 @@ int show_range_diff(const char *range1, const char *range2,
 
 int is_range_diff_range(const char *arg)
 {
-	return !!strstr(arg, "..");
+	static regex_t *regex;
+
+	if (strstr(arg, ".."))
+		return 1;
+
+	/* match `<rev>^!` and `<rev>^-<n>` */
+	if (!regex) {
+		regex = xmalloc(sizeof(*regex));
+		if (regcomp(regex, "\\^(!|-[0-9]*)$", REG_EXTENDED) < 0)
+			BUG("could not compile range-diff regex");
+	}
+
+	return !regexec(regex, arg, 0, NULL, 0);
 }

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be031..e217cecac9e 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

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 check into its own function.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/log.c        | 2 +-
 builtin/range-diff.c | 9 +++++----
 range-diff.c         | 5 +++++
 range-diff.h         | 8 ++++++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index bd6ff4f9f95..aeece57e86a 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1680,7 +1680,7 @@ 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 = is_range_diff_range(prev);
 
 	if (prev_is_range)
 		strbuf_addstr(r1, prev);

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f744..5b1f6326322 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -3,6 +3,7 @@
 #include "parse-options.h"
 #include "range-diff.h"
 #include "config.h"
+#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),

@@ -46,12 +47,12 @@ 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_diff_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_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]);

diff --git a/range-diff.c b/range-diff.c
index b9950f10c8c..9b93e08e840 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,
 
 	return res;
 }
+
+int is_range_diff_range(const char *arg)
+{
+	return !!strstr(arg, "..");
+}

diff --git a/range-diff.h b/range-diff.h
index 583ced2e8e7..c17dbc2e75a 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,
 		    const struct diff_options *diffopt,
 		    const struct strvec *other_arg);
 
+/*
+ * Determine whether the given argument is usable as a range argument of `git
+ * range-diff`, 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 is_range_diff_range(const char *arg);
+
 #endif

-- 
gitgitgadget

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Changes since v3:

 * The revision machinery is now used directly to validate the commit
   ranges.

Changes since v2:

 * Move the helper function from revision.c to range-diff.c and rename it.
 * Use a regex to make it easier to understand what we're trying to match.
 * Fix the documentation that claimed that we used git merge-base internally
   when git range-diff parses ...-style arguments, which is not the case.

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 | 12 ++++++++++++
 builtin/log.c                    |  2 +-
 builtin/range-diff.c             |  9 +++++----
 range-diff.c                     | 22 ++++++++++++++++++++++
 range-diff.h                     |  8 ++++++++
 t/t3206-range-diff.sh            |  8 ++++++++
 6 files changed, 56 insertions(+), 5 deletions(-)


base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/841

Range-diff vs v3:

 1:  b98fa94b8703 = 1:  b98fa94b8703 range-diff/format-patch: refactor check for commit range
 2:  0880ca587e63 ! 2:  448e6a64fa15 range-diff/format-patch: handle commit ranges other than A..B
     @@ Commit message
          described to specify commit ranges that `range-diff` does not yet
          accept: "<commit>^!" and "<commit>^-<n>".
      
     -    Let's accept them.
     +    Let's accept them, by parsing them via the revision machinery and
     +    looking for at least one interesting and one uninteresting revision in
     +    the resulting `pending` array.
      
          Signed-off-by: Johannes Schindelin [off-list ref]
      
       ## range-diff.c ##
     +@@
     + #include "pretty.h"
     + #include "userdiff.h"
     + #include "apply.h"
     ++#include "revision.h"
     + 
     + struct patch_util {
     + 	/* For the search for an exact match */
      @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
       
       int is_range_diff_range(const char *arg)
       {
      -	return !!strstr(arg, "..");
     -+	static regex_t *regex;
     -+
     -+	if (strstr(arg, ".."))
     -+		return 1;
     ++	char *copy = xstrdup(arg); /* setup_revisions() modifies it */
     ++	const char *argv[] = { "", copy, "--", NULL };
     ++	int i, positive = 0, negative = 0;
     ++	struct rev_info revs;
      +
     -+	/* match `<rev>^!` and `<rev>^-<n>` */
     -+	if (!regex) {
     -+		regex = xmalloc(sizeof(*regex));
     -+		if (regcomp(regex, "\\^(!|-[0-9]*)$", REG_EXTENDED) < 0)
     -+			BUG("could not compile range-diff regex");
     ++	init_revisions(&revs, NULL);
     ++	if (setup_revisions(3, argv, &revs, 0) == 1) {
     ++		for (i = 0; i < revs.pending.nr; i++)
     ++			if (revs.pending.objects[i].item->flags & UNINTERESTING)
     ++				negative++;
     ++			else
     ++				positive++;
      +	}
      +
     -+	return !regexec(regex, arg, 0, NULL, 0);
     ++	free(copy);
     ++	object_array_clear(&revs.pending);
     ++	return negative > 0 && positive > 0;
       }
      
       ## t/t3206-range-diff.sh ##
 3:  5ab9321a34ca = 3:  295fdc1cd32c range-diff(docs): explain how to specify commit ranges

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

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 check into its own function.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/log.c        | 2 +-
 builtin/range-diff.c | 9 +++++----
 range-diff.c         | 5 +++++
 range-diff.h         | 8 ++++++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index bd6ff4f9f956..aeece57e86a2 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1680,7 +1680,7 @@ 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 = is_range_diff_range(prev);
 
 	if (prev_is_range)
 		strbuf_addstr(r1, prev);

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f7446..5b1f6326322f 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -3,6 +3,7 @@
 #include "parse-options.h"
 #include "range-diff.h"
 #include "config.h"
+#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),

@@ -46,12 +47,12 @@ 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_diff_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_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]);

diff --git a/range-diff.c b/range-diff.c
index b9950f10c8c4..9b93e08e8407 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,
 
 	return res;
 }
+
+int is_range_diff_range(const char *arg)
+{
+	return !!strstr(arg, "..");
+}

diff --git a/range-diff.h b/range-diff.h
index 583ced2e8e74..c17dbc2e75a8 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,
 		    const struct diff_options *diffopt,
 		    const struct strvec *other_arg);
 
+/*
+ * Determine whether the given argument is usable as a range argument of `git
+ * range-diff`, 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 is_range_diff_range(const char *arg);
+
 #endif

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them, by parsing them via the revision machinery and
looking for at least one interesting and one uninteresting revision in
the resulting `pending` array.

Signed-off-by: Johannes Schindelin <redacted>
---
 range-diff.c          | 19 ++++++++++++++++++-
 t/t3206-range-diff.sh |  8 ++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/range-diff.c b/range-diff.c
index 9b93e08e8407..07e212d5bb8c 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -11,6 +11,7 @@
 #include "pretty.h"
 #include "userdiff.h"
 #include "apply.h"
+#include "revision.h"
 
 struct patch_util {
 	/* For the search for an exact match */

@@ -567,5 +568,21 @@ int show_range_diff(const char *range1, const char *range2,
 
 int is_range_diff_range(const char *arg)
 {
-	return !!strstr(arg, "..");
+	char *copy = xstrdup(arg); /* setup_revisions() modifies it */
+	const char *argv[] = { "", copy, "--", NULL };
+	int i, positive = 0, negative = 0;
+	struct rev_info revs;
+
+	init_revisions(&revs, NULL);
+	if (setup_revisions(3, argv, &revs, 0) == 1) {
+		for (i = 0; i < revs.pending.nr; i++)
+			if (revs.pending.objects[i].item->flags & UNINTERESTING)
+				negative++;
+			else
+				positive++;
+	}
+
+	free(copy);
+	object_array_clear(&revs.pending);
+	return negative > 0 && positive > 0;
 }

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be0312..e217cecac9ed 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd5..14bffb272a06 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
+  equivalent to `<rev2>..<rev1> <rev1>..<rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Changes since v4:

 * The commit marks are now cleared in is_range_diff_range().
 * A regression test now verifies that HEAD^{/something..or other} isn't
   mistaken for a commit range.
 * The manual page no longer mentions "symmetric range", to avoid
   contentious language.

Changes since v3:

 * The revision machinery is now used directly to validate the commit
   ranges.

Changes since v2:

 * Move the helper function from revision.c to range-diff.c and rename it.
 * Use a regex to make it easier to understand what we're trying to match.
 * Fix the documentation that claimed that we used git merge-base internally
   when git range-diff parses ...-style arguments, which is not the case.

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 | 11 +++++++++++
 builtin/log.c                    |  2 +-
 builtin/range-diff.c             |  9 +++++----
 range-diff.c                     | 27 +++++++++++++++++++++++++++
 range-diff.h                     |  6 ++++++
 t/t3206-range-diff.sh            | 13 +++++++++++++
 6 files changed, 63 insertions(+), 5 deletions(-)


base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/841

Range-diff vs v4:

 1:  b98fa94b8703 = 1:  b98fa94b8703 range-diff/format-patch: refactor check for commit range
 2:  448e6a64fa15 ! 2:  04b5d75adbc3 range-diff/format-patch: handle commit ranges other than A..B
     @@ Commit message
          looking for at least one interesting and one uninteresting revision in
          the resulting `pending` array.
      
     +    This also finally lets us reject arguments that _do_ contain `..` but
     +    are not actually ranges, e.g. `HEAD^{/do.. match this}`.
     +
          Signed-off-by: Johannes Schindelin [off-list ref]
      
       ## range-diff.c ##
     @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
      +	struct rev_info revs;
      +
      +	init_revisions(&revs, NULL);
     -+	if (setup_revisions(3, argv, &revs, 0) == 1) {
     -+		for (i = 0; i < revs.pending.nr; i++)
     -+			if (revs.pending.objects[i].item->flags & UNINTERESTING)
     ++	if (setup_revisions(3, argv, &revs, 0) == 1)
     ++		for (i = 0; i < revs.pending.nr; i++) {
     ++			struct object *obj = revs.pending.objects[i].item;
     ++
     ++			if (obj->flags & UNINTERESTING)
      +				negative++;
      +			else
      +				positive++;
     -+	}
     ++			if (obj->type == OBJ_COMMIT)
     ++				clear_commit_marks((struct commit *)obj,
     ++						   ALL_REV_FLAGS);
     ++		}
      +
      +	free(copy);
      +	object_array_clear(&revs.pending);
      +	return negative > 0 && positive > 0;
       }
      
     + ## range-diff.h ##
     +@@ range-diff.h: int show_range_diff(const char *range1, const char *range2,
     + 
     + /*
     +  * Determine whether the given argument is usable as a range argument of `git
     +- * range-diff`, 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.
     ++ * range-diff`, e.g. A..B.
     +  */
     + int is_range_diff_range(const char *arg);
     + 
     +
       ## t/t3206-range-diff.sh ##
      @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '
       	test_cmp expect actual
     @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '
      +	EOF
      +	test_cmp expect actual
      +'
     ++
     ++test_expect_success 'A^{/..} is not mistaken for a range' '
     ++	test_must_fail git range-diff topic^.. topic^{/..} 2>error &&
     ++	test_i18ngrep "not a commit rang" error
     ++'
      +
       test_expect_success 'trivial reordering' '
       	git range-diff --no-color master topic reordered >actual &&
 3:  295fdc1cd32c ! 3:  bc5de807735d range-diff(docs): explain how to specify commit ranges
     @@ Documentation/git-range-diff.txt: Finally, the list of matching commits is shown
      +  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
      +  in linkgit:gitrevisions[7] for more details.
      +
     -+- `<rev1>...<rev2>`. This resembles the symmetric ranges mentioned in
     -+  the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is
     -+  equivalent to `<rev2>..<rev1> <rev1>..<rev2>`.
     ++- `<rev1>...<rev2>`. This is equivalent to
     ++  `<rev2>..<rev1> <rev1>..<rev2>`.
      +
      +- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
      +  <base>..<rev2>`.

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd5..a968d5237dae 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,17 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This is equivalent to
+  `<rev2>..<rev1> <rev1>..<rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

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 check into its own function.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/log.c        | 2 +-
 builtin/range-diff.c | 9 +++++----
 range-diff.c         | 5 +++++
 range-diff.h         | 8 ++++++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index bd6ff4f9f956..aeece57e86a2 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1680,7 +1680,7 @@ 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 = is_range_diff_range(prev);
 
 	if (prev_is_range)
 		strbuf_addstr(r1, prev);

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f7446..5b1f6326322f 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -3,6 +3,7 @@
 #include "parse-options.h"
 #include "range-diff.h"
 #include "config.h"
+#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),

@@ -46,12 +47,12 @@ 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_diff_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_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]);

diff --git a/range-diff.c b/range-diff.c
index b9950f10c8c4..9b93e08e8407 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,
 
 	return res;
 }
+
+int is_range_diff_range(const char *arg)
+{
+	return !!strstr(arg, "..");
+}

diff --git a/range-diff.h b/range-diff.h
index 583ced2e8e74..c17dbc2e75a8 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,
 		    const struct diff_options *diffopt,
 		    const struct strvec *other_arg);
 
+/*
+ * Determine whether the given argument is usable as a range argument of `git
+ * range-diff`, 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 is_range_diff_range(const char *arg);
+
 #endif

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them, by parsing them via the revision machinery and
looking for at least one interesting and one uninteresting revision in
the resulting `pending` array.

This also finally lets us reject arguments that _do_ contain `..` but
are not actually ranges, e.g. `HEAD^{/do.. match this}`.

Signed-off-by: Johannes Schindelin <redacted>
---
 range-diff.c          | 24 +++++++++++++++++++++++-
 range-diff.h          |  4 +---
 t/t3206-range-diff.sh | 13 +++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 9b93e08e8407..c307bca9de23 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -11,6 +11,7 @@
 #include "pretty.h"
 #include "userdiff.h"
 #include "apply.h"
+#include "revision.h"
 
 struct patch_util {
 	/* For the search for an exact match */

@@ -567,5 +568,26 @@ int show_range_diff(const char *range1, const char *range2,
 
 int is_range_diff_range(const char *arg)
 {
-	return !!strstr(arg, "..");
+	char *copy = xstrdup(arg); /* setup_revisions() modifies it */
+	const char *argv[] = { "", copy, "--", NULL };
+	int i, positive = 0, negative = 0;
+	struct rev_info revs;
+
+	init_revisions(&revs, NULL);
+	if (setup_revisions(3, argv, &revs, 0) == 1)
+		for (i = 0; i < revs.pending.nr; i++) {
+			struct object *obj = revs.pending.objects[i].item;
+
+			if (obj->flags & UNINTERESTING)
+				negative++;
+			else
+				positive++;
+			if (obj->type == OBJ_COMMIT)
+				clear_commit_marks((struct commit *)obj,
+						   ALL_REV_FLAGS);
+		}
+
+	free(copy);
+	object_array_clear(&revs.pending);
+	return negative > 0 && positive > 0;
 }

diff --git a/range-diff.h b/range-diff.h
index c17dbc2e75a8..4abd70c40fed 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -18,9 +18,7 @@ int show_range_diff(const char *range1, const char *range2,
 
 /*
  * Determine whether the given argument is usable as a range argument of `git
- * range-diff`, 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.
+ * range-diff`, e.g. A..B.
  */
 int is_range_diff_range(const char *arg);
 

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be0312..45f21ee215d7 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,19 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'A^{/..} is not mistaken for a range' '
+	test_must_fail git range-diff topic^.. topic^{/..} 2>error &&
+	test_i18ngrep "not a commit rang" error
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

In
https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,
it was reported that git range-diff does not handle commit ranges like
rev^!. This patch series fixes that.

Changes since v5:

 * The commit marks are now cleared in a separate loop.
 * The regression test no longer looks only for "rang" but for "range" in
   the error message.
 * We now pass NULL as opt parameter to setup_revisions(), not 0.

Changes since v4:

 * The commit marks are now cleared in is_range_diff_range().
 * A regression test now verifies that HEAD^{/something..or other} isn't
   mistaken for a commit range.
 * The manual page no longer mentions "symmetric range", to avoid
   contentious language.

Changes since v3:

 * The revision machinery is now used directly to validate the commit
   ranges.

Changes since v2:

 * Move the helper function from revision.c to range-diff.c and rename it.
 * Use a regex to make it easier to understand what we're trying to match.
 * Fix the documentation that claimed that we used git merge-base internally
   when git range-diff parses ...-style arguments, which is not the case.

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 | 11 +++++++++++
 builtin/log.c                    |  2 +-
 builtin/range-diff.c             |  9 +++++----
 range-diff.c                     | 29 +++++++++++++++++++++++++++++
 range-diff.h                     |  6 ++++++
 t/t3206-range-diff.sh            | 13 +++++++++++++
 6 files changed, 65 insertions(+), 5 deletions(-)


base-commit: 71ca53e8125e36efbda17293c50027d31681a41f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/841

Range-diff vs v5:

 1:  b98fa94b8703 = 1:  b98fa94b8703 range-diff/format-patch: refactor check for commit range
 2:  04b5d75adbc3 ! 2:  f8e6a1ad9d3d range-diff/format-patch: handle commit ranges other than A..B
     @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,
      +	struct rev_info revs;
      +
      +	init_revisions(&revs, NULL);
     -+	if (setup_revisions(3, argv, &revs, 0) == 1)
     -+		for (i = 0; i < revs.pending.nr; i++) {
     -+			struct object *obj = revs.pending.objects[i].item;
     -+
     -+			if (obj->flags & UNINTERESTING)
     ++	if (setup_revisions(3, argv, &revs, NULL) == 1) {
     ++		for (i = 0; i < revs.pending.nr; i++)
     ++			if (revs.pending.objects[i].item->flags & UNINTERESTING)
      +				negative++;
      +			else
      +				positive++;
     ++		for (i = 0; i < revs.pending.nr; i++) {
     ++			struct object *obj = revs.pending.objects[i].item;
     ++
      +			if (obj->type == OBJ_COMMIT)
      +				clear_commit_marks((struct commit *)obj,
      +						   ALL_REV_FLAGS);
      +		}
     ++	}
      +
      +	free(copy);
      +	object_array_clear(&revs.pending);
     @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '
      +
      +test_expect_success 'A^{/..} is not mistaken for a range' '
      +	test_must_fail git range-diff topic^.. topic^{/..} 2>error &&
     -+	test_i18ngrep "not a commit rang" error
     ++	test_i18ngrep "not a commit range" error
      +'
      +
       test_expect_success 'trivial reordering' '
 3:  bc5de807735d = 3:  08c5f8732747 range-diff(docs): explain how to specify commit ranges

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
described to specify commit ranges that `range-diff` does not yet
accept: "<commit>^!" and "<commit>^-<n>".

Let's accept them, by parsing them via the revision machinery and
looking for at least one interesting and one uninteresting revision in
the resulting `pending` array.

This also finally lets us reject arguments that _do_ contain `..` but
are not actually ranges, e.g. `HEAD^{/do.. match this}`.

Signed-off-by: Johannes Schindelin <redacted>
---
 range-diff.c          | 26 +++++++++++++++++++++++++-
 range-diff.h          |  4 +---
 t/t3206-range-diff.sh | 13 +++++++++++++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 9b93e08e8407..a88612cb8923 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -11,6 +11,7 @@
 #include "pretty.h"
 #include "userdiff.h"
 #include "apply.h"
+#include "revision.h"
 
 struct patch_util {
 	/* For the search for an exact match */

@@ -567,5 +568,28 @@ int show_range_diff(const char *range1, const char *range2,
 
 int is_range_diff_range(const char *arg)
 {
-	return !!strstr(arg, "..");
+	char *copy = xstrdup(arg); /* setup_revisions() modifies it */
+	const char *argv[] = { "", copy, "--", NULL };
+	int i, positive = 0, negative = 0;
+	struct rev_info revs;
+
+	init_revisions(&revs, NULL);
+	if (setup_revisions(3, argv, &revs, NULL) == 1) {
+		for (i = 0; i < revs.pending.nr; i++)
+			if (revs.pending.objects[i].item->flags & UNINTERESTING)
+				negative++;
+			else
+				positive++;
+		for (i = 0; i < revs.pending.nr; i++) {
+			struct object *obj = revs.pending.objects[i].item;
+
+			if (obj->type == OBJ_COMMIT)
+				clear_commit_marks((struct commit *)obj,
+						   ALL_REV_FLAGS);
+		}
+	}
+
+	free(copy);
+	object_array_clear(&revs.pending);
+	return negative > 0 && positive > 0;
 }

diff --git a/range-diff.h b/range-diff.h
index c17dbc2e75a8..4abd70c40fed 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -18,9 +18,7 @@ int show_range_diff(const char *range1, const char *range2,
 
 /*
  * Determine whether the given argument is usable as a range argument of `git
- * range-diff`, 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.
+ * range-diff`, e.g. A..B.
  */
 int is_range_diff_range(const char *arg);
 

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6eb344be0312..2b518378d4a0 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh

@@ -150,6 +150,19 @@ test_expect_success 'simple A B C (unmodified)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'A^! and A^-<n> (unmodified)' '
+	git range-diff --no-color topic^! unmodified^-1 >actual &&
+	cat >expect <<-EOF &&
+	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'A^{/..} is not mistaken for a range' '
+	test_must_fail git range-diff topic^.. topic^{/..} 2>error &&
+	test_i18ngrep "not a commit range" error
+'
+
 test_expect_success 'trivial reordering' '
 	git range-diff --no-color master topic reordered >actual &&
 	cat >expect <<-EOF &&

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

There are three forms, depending whether the user specifies one, two or
three non-option arguments. We've never actually explained how this
works in the manual, so let's explain it.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-range-diff.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt
index 9701c1e5fdd5..a968d5237dae 100644
--- a/Documentation/git-range-diff.txt
+++ b/Documentation/git-range-diff.txt

@@ -28,6 +28,17 @@ Finally, the list of matching commits is shown in the order of the
 second commit range, with unmatched commits being inserted just after
 all of their ancestors have been shown.
 
+There are three ways to specify the commit ranges:
+
+- `<range1> <range2>`: Either commit range can be of the form
+  `<base>..<rev>`, `<rev>^!` or `<rev>^-<n>`. See `SPECIFYING RANGES`
+  in linkgit:gitrevisions[7] for more details.
+
+- `<rev1>...<rev2>`. This is equivalent to
+  `<rev2>..<rev1> <rev1>..<rev2>`.
+
+- `<base> <rev1> <rev2>`: This is equivalent to `<base>..<rev1>
+  <base>..<rev2>`.
 
 OPTIONS
 -------

-- 
gitgitgadget

From: Johannes Schindelin <redacted>

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 check into its own function.

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/log.c        | 2 +-
 builtin/range-diff.c | 9 +++++----
 range-diff.c         | 5 +++++
 range-diff.h         | 8 ++++++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index bd6ff4f9f956..aeece57e86a2 100644
--- a/builtin/log.c
+++ b/builtin/log.c

@@ -1680,7 +1680,7 @@ 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 = is_range_diff_range(prev);
 
 	if (prev_is_range)
 		strbuf_addstr(r1, prev);

diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 24c4162f7446..5b1f6326322f 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c

@@ -3,6 +3,7 @@
 #include "parse-options.h"
 #include "range-diff.h"
 #include "config.h"
+#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),

@@ -46,12 +47,12 @@ 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_diff_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_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]);

diff --git a/range-diff.c b/range-diff.c
index b9950f10c8c4..9b93e08e8407 100644
--- a/range-diff.c
+++ b/range-diff.c

@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,
 
 	return res;
 }
+
+int is_range_diff_range(const char *arg)
+{
+	return !!strstr(arg, "..");
+}

diff --git a/range-diff.h b/range-diff.h
index 583ced2e8e74..c17dbc2e75a8 100644
--- a/range-diff.h
+++ b/range-diff.h

@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,
 		    const struct diff_options *diffopt,
 		    const struct strvec *other_arg);
 
+/*
+ * Determine whether the given argument is usable as a range argument of `git
+ * range-diff`, 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 is_range_diff_range(const char *arg);
+
 #endif

-- 
gitgitgadget