{"thread":{"id":"55025","subject":"[PATCH 1/3] range-diff: refactor check for commit range","startedAt":"2021-01-21T22:22:17Z","lastAt":"2021-02-06T03:19:42Z","messageCount":63,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Eric Sunshine","Uwe Kleine-König","Johannes Schindelin","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"414929","messageId":"5839ba4f7615819ed6f9a0fcb6de1855cd2e89e1.1611267638.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.git.1611267638.gitgitgadget@gmail.com","subject":"[PATCH 1/3] range-diff: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-21T22:20:36Z","receivedAt":"2021-01-21T22:22:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, we test for a literal\n`..` in each of the two.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the conditional into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/range-diff.c | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f744..551d3e689cb 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -11,6 +11,11 @@ N_(\"git range-diff [] \"),\n NULL\n };\n \n+static int is_range(const char *range)\n+{\n+\treturn !!strstr(range, \"..\");\n+}\n+\n int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n@@ -46,12 +51,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!is_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!is_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\n-- \ngitgitgadget\n\n"},{"id":"414930","messageId":"88c15617b4ba8ae3211b1a01861eb4165f3eda38.1611267638.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.git.1611267638.gitgitgadget@gmail.com","subject":"[PATCH 2/3] range-diff: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-21T22:20:37Z","receivedAt":"2021-01-21T22:22:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/range-diff.c | 21 ++++++++++++++++++++-\n t/t3206-range-diff.sh | 8 ++++++++\n 2 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 551d3e689cb..6097635c432 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -13,7 +13,26 @@ NULL\n \n static int is_range(const char *range)\n {\n-\treturn !!strstr(range, \"..\");\n+\tsize_t i;\n+\tchar c;\n+\n+\tif (strstr(range, \"..\"))\n+\t\treturn 1;\n+\n+\ti = strlen(range);\n+\tc = i ? range[--i] : 0;\n+\tif (c == '!')\n+\t\ti--; /* might be ...^! or ...^@ */\n+\telse if (isdigit(c)) {\n+\t\t/* handle ...^- */\n+\t\twhile (i > 2 && isdigit(range[--i]))\n+\t\t\t; /* keep trimming trailing digits */\n+\t\tif (i < 2 || range[i--] != '-')\n+\t\t\treturn 0;\n+\t} else\n+\t\treturn 0;\n+\n+\treturn i > 0 && range[i] == '^';\n }\n \n int cmd_range_diff(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be031..e217cecac9e 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"414931","messageId":"041456b6e73b3a88097d0cc06056eb43d35d42c6.1611267638.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.git.1611267638.gitgitgadget@gmail.com","subject":"[PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-21T22:20:38Z","receivedAt":"2021-01-21T22:22:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd..76359baf26d 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This resembles the symmetric ranges mentioned in\n+ the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n+ equivalent to `.. ..` where `` is the\n+ merge base as obtained via `git merge-base `.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"414932","messageId":"pull.841.git.1611267638.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":null,"subject":"[PATCH 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-21T22:20:35Z","receivedAt":"2021-01-21T22:22:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nJohannes Schindelin (3):\n range-diff: refactor check for commit range\n range-diff: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 13 +++++++++++++\n builtin/range-diff.c | 32 ++++++++++++++++++++++++++++----\n t/t3206-range-diff.sh | 8 ++++++++\n 3 files changed, 49 insertions(+), 4 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n-- \ngitgitgadget\n"},{"id":"414938","messageId":"xmqq7do5c1lx.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"5839ba4f7615819ed6f9a0fcb6de1855cd2e89e1.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] range-diff: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T23:27:22Z","receivedAt":"2021-01-21T23:28:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Currently, when called with exactly two arguments, we test for a literal\n> `..` in each of the two.\n>\n> However, `^!` is a perfectly valid commit range, equivalent to\n> `^..` according to the `SPECIFYING RANGES` section of\n> gitrevisions[7].\n>\n> In preparation for allowing more sophisticated ways to specify commit\n> ranges, let's refactor the conditional into its own function.\n\nMakes sense.\n"},{"id":"414939","messageId":"CAPig+cTMqy=57hGksH6GaCB7i=eJHTCWVzwED=PPZF1hNp_1nQ@mail.gmail.com","threadId":"55025","inReplyTo":"88c15617b4ba8ae3211b1a01861eb4165f3eda38.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-01-21T23:37:07Z","receivedAt":"2021-01-21T23:38:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Jan 21, 2021 at 5:22 PM Johannes Schindelin via GitGitGadget\n wrote:\n> In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\n> described to specify commit ranges that `range-diff` does not yet\n> accept: \"^!\" and \"^-\".\n>\n> Let's accept them.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> @@ -13,7 +13,26 @@ NULL\n> static int is_range(const char *range)\n> {\n> + if (strstr(range, \"..\"))\n> + return 1;\n> +\n> + i = strlen(range);\n> + c = i ? range[--i] : 0;\n> + if (c == '!')\n> + i--; /* might be ...^! or ...^@ */\n> + else if (isdigit(c)) {\n> + /* handle ...^- */\n> + while (i > 2 && isdigit(range[--i]))\n> + ; /* keep trimming trailing digits */\n> + if (i < 2 || range[i--] != '-')\n> + return 0;\n> + } else\n> + return 0;\n> +\n> + return i > 0 && range[i] == '^';\n> }\n\nIs this something that the --range-diff option of git-format-patch\nwill want to do, as well? At present,\nbuiltin/log.c:infer_range_diff_ranges() detects a range only by\nchecking for \"..\", much like this function did before this patch. If\nso, perhaps this function can be part of the public range-diff API\n(or, indeed, part of some other more general API if it's not really\nspecific to range-diff).\n"},{"id":"414940","messageId":"xmqq1redc0x9.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"88c15617b4ba8ae3211b1a01861eb4165f3eda38.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T23:42:10Z","receivedAt":"2021-01-21T23:43:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\n> described to specify commit ranges that `range-diff` does not yet\n> accept: \"^!\" and \"^-\".\n>\n> Let's accept them.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/range-diff.c | 21 ++++++++++++++++++++-\n> t/t3206-range-diff.sh | 8 ++++++++\n> 2 files changed, 28 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> index 551d3e689cb..6097635c432 100644\n> --- a/builtin/range-diff.c\n> +++ b/builtin/range-diff.c\n> @@ -13,7 +13,26 @@ NULL\n> \n> static int is_range(const char *range)\n> {\n> -\treturn !!strstr(range, \"..\");\n> +\tsize_t i;\n> +\tchar c;\n> +\n> +\tif (strstr(range, \"..\"))\n> +\t\treturn 1;\n> +\n> +\ti = strlen(range);\n> +\tc = i ? range[--i] : 0;\n> +\tif (c == '!')\n> +\t\ti--; /* might be ...^! or ...^@ */\n\nI am confused. If it ends with '!', I do not see how it can end\nwith \"^@\".\n\nIf the input were \"!\", i gets strlen(\"!\") which is 1, c gets '!'\nwhile predecrementing i down to 0, and we notice c is '!' and\ndecrement i again to make it (size_t)(-1) which is a fairly large\nnumber.\n\nThen we skip all the else/if cascade, ensure that i is positive, and\nhappily access range[i], which likely is way out of bounds (but it\nprobably is almost one turn around the earth out of bounds, it may\naccess just a single byte before the array).\n\nAm I reading the code right?\n\nIOW, \"git range-diff \\! A..B\" would do something strange, I would\nguess.\n\n> +\telse if (isdigit(c)) {\n> +\t\t/* handle ...^- */\n> +\t\twhile (i > 2 && isdigit(range[--i]))\n> +\t\t\t; /* keep trimming trailing digits */\n> +\t\tif (i < 2 || range[i--] != '-')\n> +\t\t\treturn 0;\n> +\t} else\n> +\t\treturn 0;\n> +\n> +\treturn i > 0 && range[i] == '^';\n> }\n> \n> int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index 6eb344be031..e217cecac9e 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'A^! and A^- (unmodified)' '\n> +\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success 'trivial reordering' '\n> \tgit range-diff --no-color master topic reordered >actual &&\n> \tcat >expect <<-EOF &&\n"},{"id":"414942","messageId":"xmqqwnw5am64.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"041456b6e73b3a88097d0cc06056eb43d35d42c6.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-21T23:46:11Z","receivedAt":"2021-01-21T23:47:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> There are three forms, depending whether the user specifies one, two or\n> three non-option arguments. We've never actually explained how this\n> works in the manual, so let's explain it.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/git-range-diff.txt | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n>\n> diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> index 9701c1e5fdd..76359baf26d 100644\n> --- a/Documentation/git-range-diff.txt\n> +++ b/Documentation/git-range-diff.txt\n> @@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n> second commit range, with unmatched commits being inserted just after\n> all of their ancestors have been shown.\n> \n> +There are three ways to specify the commit ranges:\n> +\n> +- ` `: Either commit range can be of the form\n> + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> + in linkgit:gitrevisions[7] for more details.\n\nGood.\n\n> +- `...`. This resembles the symmetric ranges mentioned in\n> + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> + equivalent to `.. ..` where `` is the\n> + merge base as obtained via `git merge-base `.\n\nDoes this merely resemble? Isn't it exactly what a symmetric range is?\n\n> +- ` `: This is equivalent to `..\n> + ..`.\n\nNice to see this documented.\n\nThanks.\n"},{"id":"414953","messageId":"20210122073148.e3vfiq56zivdygep@pengutronix.de","threadId":"55025","inReplyTo":"pull.841.git.1611267638.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Range diff with ranges lacking dotdot","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2021-01-22T07:31:48Z","receivedAt":"2021-01-22T07:32:39Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Johannes,\n\nOn Thu, Jan 21, 2021 at 10:20:35PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> In\n> https://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\n> it was reported that git range-diff does not handle commit ranges like\n> rev^!. This patch series fixes that.\n\nOh wow, great you picked that up. It still occasionally annoys me that\nthe range parser of range-diff cannot do $hash^!.\n\nI assume you will fix the stuff Junio pointed out (or argument your\npatch 1 is right, I didn't try to check). When this is discussed to an\nend or fixed I will happily test your series.\n\nAnd by the way you also made one of my coworkers happy with your series.\n\nBest regards and thanks\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | https://www.pengutronix.de/ |\n"},{"id":"414972","messageId":"nycvar.QRO.7.76.6.2101221713230.52@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqq1redc0x9.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-22T16:20:08Z","receivedAt":"2021-01-22T16:22:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Jan 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\n> > described to specify commit ranges that `range-diff` does not yet\n> > accept: \"^!\" and \"^-\".\n> >\n> > Let's accept them.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > builtin/range-diff.c | 21 ++++++++++++++++++++-\n> > t/t3206-range-diff.sh | 8 ++++++++\n> > 2 files changed, 28 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> > index 551d3e689cb..6097635c432 100644\n> > --- a/builtin/range-diff.c\n> > +++ b/builtin/range-diff.c\n> > @@ -13,7 +13,26 @@ NULL\n> >\n> > static int is_range(const char *range)\n> > {\n> > -\treturn !!strstr(range, \"..\");\n> > +\tsize_t i;\n> > +\tchar c;\n> > +\n> > +\tif (strstr(range, \"..\"))\n> > +\t\treturn 1;\n> > +\n> > +\ti = strlen(range);\n> > +\tc = i ? range[--i] : 0;\n> > +\tif (c == '!')\n> > +\t\ti--; /* might be ...^! or ...^@ */\n>\n> I am confused. If it ends with '!', I do not see how it can end\n> with \"^@\".\n\nBah. This is a left-over from an earlier version. I tried to hide the fact\nthat I had misunderstood `^@` to specify a commit range. Oh well.\n\n> If the input were \"!\", i gets strlen(\"!\") which is 1, c gets '!'\n> while predecrementing i down to 0, and we notice c is '!' and\n> decrement i again to make it (size_t)(-1) which is a fairly large\n> number.\n\nRight, guarding that `range[--i]` only by `i` is not enough. My idea was\nto exit early if the string is too short, anyway, i.e. if `i < 3`.\n\n> Then we skip all the else/if cascade, ensure that i is positive, and\n> happily access range[i], which likely is way out of bounds (but it\n> probably is almost one turn around the earth out of bounds, it may\n> access just a single byte before the array).\n>\n> Am I reading the code right?\n>\n> IOW, \"git range-diff \\! A..B\" would do something strange, I would\n> guess.\n\nRight. Will be fixed in the next iteration.\n\nThanks,\nDscho\n\n"},{"id":"414973","messageId":"nycvar.QRO.7.76.6.2101221720200.52@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqwnw5am64.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-22T16:21:37Z","receivedAt":"2021-01-22T16:46:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Jan 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > There are three forms, depending whether the user specifies one, two or\n> > three non-option arguments. We've never actually explained how this\n> > works in the manual, so let's explain it.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > Documentation/git-range-diff.txt | 13 +++++++++++++\n> > 1 file changed, 13 insertions(+)\n> >\n> > diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> > index 9701c1e5fdd..76359baf26d 100644\n> > --- a/Documentation/git-range-diff.txt\n> > +++ b/Documentation/git-range-diff.txt\n> > @@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n> > second commit range, with unmatched commits being inserted just after\n> > all of their ancestors have been shown.\n> >\n> > +There are three ways to specify the commit ranges:\n> > +\n> > +- ` `: Either commit range can be of the form\n> > + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> > + in linkgit:gitrevisions[7] for more details.\n>\n> Good.\n>\n> > +- `...`. This resembles the symmetric ranges mentioned in\n> > + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> > + equivalent to `.. ..` where `` is the\n> > + merge base as obtained via `git merge-base `.\n>\n> Does this merely resemble? Isn't it exactly what a symmetric range is?\n\nNo, it is not exactly what a symmetric range is because `range-diff`\ntreats both arms of the symmetric range independently, as two distinct\nnon-symmetric ranges.\n\n> > +- ` `: This is equivalent to `..\n> > + ..`.\n>\n> Nice to see this documented.\n\nYes. I was rather surprised that I had overlooked that in my original\nsubmission of the `range-diff` command.\n\nCiao,\nDscho\n"},{"id":"414974","messageId":"nycvar.QRO.7.76.6.2101221711140.52@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"CAPig+cTMqy=57hGksH6GaCB7i=eJHTCWVzwED=PPZF1hNp_1nQ@mail.gmail.com","subject":"Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-22T16:12:49Z","receivedAt":"2021-01-22T16:56:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Thu, 21 Jan 2021, Eric Sunshine wrote:\n\n> On Thu, Jan 21, 2021 at 5:22 PM Johannes Schindelin via GitGitGadget\n> wrote:\n> > In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\n> > described to specify commit ranges that `range-diff` does not yet\n> > accept: \"^!\" and \"^-\".\n> >\n> > Let's accept them.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> > @@ -13,7 +13,26 @@ NULL\n> > static int is_range(const char *range)\n> > {\n> > + if (strstr(range, \"..\"))\n> > + return 1;\n> > +\n> > + i = strlen(range);\n> > + c = i ? range[--i] : 0;\n> > + if (c == '!')\n> > + i--; /* might be ...^! or ...^@ */\n> > + else if (isdigit(c)) {\n> > + /* handle ...^- */\n> > + while (i > 2 && isdigit(range[--i]))\n> > + ; /* keep trimming trailing digits */\n> > + if (i < 2 || range[i--] != '-')\n> > + return 0;\n> > + } else\n> > + return 0;\n> > +\n> > + return i > 0 && range[i] == '^';\n> > }\n>\n> Is this something that the --range-diff option of git-format-patch\n> will want to do, as well?\n\nThank you for pointing that out. I should have checked via `git grep\n'strstr.*\"\\.\\.\"'` myself. There are two more instances, one in\n`rev-parse.c` and the other in `revision.c`, but both are necessary as-are\nbecause their return value is actually used to further disect a `..`-style\ncommit range.\n\nThanks,\nDscho\n\n> At present, builtin/log.c:infer_range_diff_ranges() detects a range only\n> by checking for \"..\", much like this function did before this patch. If\n> so, perhaps this function can be part of the public range-diff API (or,\n> indeed, part of some other more general API if it's not really specific\n> to range-diff).\n"},{"id":"414980","messageId":"20210122182050.xyzkvlctb4kiin7a@pengutronix.de","threadId":"55025","inReplyTo":"041456b6e73b3a88097d0cc06056eb43d35d42c6.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2021-01-22T18:20:50Z","receivedAt":"2021-01-22T18:31:55Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"On Thu, Jan 21, 2021 at 10:20:38PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> There are three forms, depending whether the user specifies one, two or\n> three non-option arguments. We've never actually explained how this\n> works in the manual, so let's explain it.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/git-range-diff.txt | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n> \n> diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> index 9701c1e5fdd..76359baf26d 100644\n> --- a/Documentation/git-range-diff.txt\n> +++ b/Documentation/git-range-diff.txt\n> @@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n> second commit range, with unmatched commits being inserted just after\n> all of their ancestors have been shown.\n> \n> +There are three ways to specify the commit ranges:\n> +\n> +- ` `: Either commit range can be of the form\n> + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> + in linkgit:gitrevisions[7] for more details.\n> +\n> +- `...`. This resembles the symmetric ranges mentioned in\n> + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> + equivalent to `.. ..` where `` is the\n> + merge base as obtained via `git merge-base `.\n> +\n> +- ` `: This is equivalent to `..\n> + ..`.\n\ngit-log takes a range, too. There you can specify a single rev (with the\nsemantic to list all commits from this rev up (or down?) to the root).\nSo means implicitly ^∞.. for git-log.\n\nDoes it make sense to implement this here, too? Maybe this even allows\nsharing some more code?\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | https://www.pengutronix.de/ |\n"},{"id":"414981","messageId":"2c2744333ecf5662d4198bdddeee80ff4adf6acd.1611339373.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v2.git.1611339373.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-22T18:16:11Z","receivedAt":"2021-01-22T18:41:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them.\n\nSigned-off-by: Johannes Schindelin \n---\n revision.c | 22 +++++++++++++++++++++-\n t/t3206-range-diff.sh | 8 ++++++++\n 2 files changed, 29 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 00675f598a3..9ee063a2c03 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -4209,5 +4209,25 @@ void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n \n int specifies_commit_range(const char *range)\n {\n-\treturn !!strstr(range, \"..\");\n+\tsize_t i;\n+\tchar c;\n+\n+\tif (strstr(range, \"..\"))\n+\t\treturn 1;\n+\n+\ti = strlen(range);\n+\tc = i > 2 ? range[--i] : 0;\n+\tif (c == '!')\n+\t\ti--; /* might be ...^! */\n+\telse if (isdigit(c)) {\n+\t\t/* handle ...^- */\n+\t\twhile (i > 2 && isdigit(range[--i]))\n+\t\t\t; /* keep trimming trailing digits */\n+\t\tif (i < 2 || range[i--] != '-')\n+\t\t\treturn 0;\n+\t} else\n+\t\treturn 0;\n+\n+\t/* Before the `!` or the `-`, we expect `^` */\n+\treturn i > 0 && range[i] == '^';\n }\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be031..e217cecac9e 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"414982","messageId":"4f5e5acd954d933df741b57f3877954d02199fe8.1611339373.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v2.git.1611339373.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-22T18:16:12Z","receivedAt":"2021-01-22T18:41:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd..76359baf26d 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This resembles the symmetric ranges mentioned in\n+ the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n+ equivalent to `.. ..` where `` is the\n+ merge base as obtained via `git merge-base `.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"414983","messageId":"xmqq1recal32.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"nycvar.QRO.7.76.6.2101221720200.52@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-22T18:21:53Z","receivedAt":"2021-01-22T18:41:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > +- `...`. This resembles the symmetric ranges mentioned in\n>> > + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n>> > + equivalent to `.. ..` where `` is the\n>> > + merge base as obtained via `git merge-base `.\n\nThe above paragraph says A...B is turned into $(git merge-base A\nB)..A and $(git merge-base A B)..B, but I wonder if we should be\nrewriting it into A..B and B..A instead; that would make it\nunnecessary to explain what should happen when there are more than\none merge bases.\n\n>> Does this merely resemble? Isn't it exactly what a symmetric range is?\n>\n> No, it is not exactly what a symmetric range is because `range-diff`\n> treats both arms of the symmetric range independently, as two distinct\n> non-symmetric ranges.\n\nThis however is an end-user documentation, isn't it?\n\nThe fact that range-diff internally translates A...B into something\nelse is an implementation detail of achieving something, and from\nthe end-users' point of view, isn't it \"take A...B from the\nend-user, and show the difference between the left and right\nbranches of the symmetric range\" that range-diff achieves to do?\n\nSo it processes exactly the same two sets of commits as what is\ngiven by \"--left-right A...B\", no?\n"},{"id":"414986","messageId":"pull.841.v2.git.1611339373.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.git.1611267638.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-22T18:16:09Z","receivedAt":"2021-01-22T19:17:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nChanges since v1:\n\n * In addition to git range-diff, git format-patch --range-diff gets the\n same improvement.\n * The comment talking about ^@ was removed.\n * The parsing was made a bit safer (e.g. catching ! by its own as an\n invalid range).\n\nJohannes Schindelin (3):\n range-diff/format-patch: refactor check for commit range\n range-diff/format-patch: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 13 +++++++++++++\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n revision.c | 25 +++++++++++++++++++++++++\n revision.h | 7 +++++++\n t/t3206-range-diff.sh | 8 ++++++++\n 6 files changed, 59 insertions(+), 5 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n\nRange-diff vs v1:\n\n 1: 5839ba4f761 ! 1: 3f21e10f919 range-diff: refactor check for commit range\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - range-diff: refactor check for commit range\n + range-diff/format-patch: refactor check for commit range\n \n - Currently, when called with exactly two arguments, we test for a literal\n - `..` in each of the two.\n + Currently, when called with exactly two arguments, `git range-diff`\n + tests for a literal `..` in each of the two. Likewise, the argument\n + provided via `--range-diff` to `git format-patch` is checked in the same\n + manner.\n \n However, `^!` is a perfectly valid commit range, equivalent to\n `^..` according to the `SPECIFYING RANGES` section of\n gitrevisions[7].\n \n In preparation for allowing more sophisticated ways to specify commit\n - ranges, let's refactor the conditional into its own function.\n + ranges, let's refactor the check into its own function.\n \n Signed-off-by: Johannes Schindelin \n \n + ## builtin/log.c ##\n +@@ builtin/log.c: static void infer_range_diff_ranges(struct strbuf *r1,\n + \t\t\t\t struct commit *head)\n + {\n + \tconst char *head_oid = oid_to_hex(&head->object.oid);\n +-\tint prev_is_range = !!strstr(prev, \"..\");\n ++\tint prev_is_range = specifies_commit_range(prev);\n + \n + \tif (prev_is_range)\n + \t\tstrbuf_addstr(r1, prev);\n +\n ## builtin/range-diff.c ##\n -@@ builtin/range-diff.c: N_(\"git range-diff [] \"),\n - NULL\n - };\n +@@\n + #include \"parse-options.h\"\n + #include \"range-diff.h\"\n + #include \"config.h\"\n ++#include \"revision.h\"\n \n -+static int is_range(const char *range)\n -+{\n -+\treturn !!strstr(range, \"..\");\n -+}\n -+\n - int cmd_range_diff(int argc, const char **argv, const char *prefix)\n - {\n - \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n + static const char * const builtin_range_diff_usage[] = {\n + N_(\"git range-diff [] .. ..\"),\n @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n -\t\tif (!strstr(argv[0], \"..\"))\n -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n -+\t\tif (!is_range(argv[0]))\n ++\t\tif (!specifies_commit_range(argv[0]))\n +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n -\t\tif (!strstr(argv[1], \"..\"))\n -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n -+\t\tif (!is_range(argv[1]))\n ++\t\tif (!specifies_commit_range(argv[1]))\n +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\n +\n + ## revision.c ##\n +@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n + \tfputs(mark, stdout);\n + \tputchar(' ');\n + }\n ++\n ++int specifies_commit_range(const char *range)\n ++{\n ++\treturn !!strstr(range, \"..\");\n ++}\n +\n + ## revision.h ##\n +@@ revision.h: int rewrite_parents(struct rev_info *revs,\n + */\n + struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);\n + \n ++/*\n ++ * Determine whether the given argument defines a commit range, e.g. A..B.\n ++ * Note that this only validates the format but does _not_ parse it, i.e.\n ++ * it does _not_ look up the specified commits in the local repository.\n ++ */\n ++int specifies_commit_range(const char *range);\n ++\n + #endif\n 2: 88c15617b4b ! 2: 2c2744333ec range-diff: handle commit ranges other than A..B\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - range-diff: handle commit ranges other than A..B\n + range-diff/format-patch: handle commit ranges other than A..B\n \n In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\n described to specify commit ranges that `range-diff` does not yet\n @@ Commit message\n \n Signed-off-by: Johannes Schindelin \n \n - ## builtin/range-diff.c ##\n -@@ builtin/range-diff.c: NULL\n + ## revision.c ##\n +@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n \n - static int is_range(const char *range)\n + int specifies_commit_range(const char *range)\n {\n -\treturn !!strstr(range, \"..\");\n +\tsize_t i;\n @@ builtin/range-diff.c: NULL\n +\t\treturn 1;\n +\n +\ti = strlen(range);\n -+\tc = i ? range[--i] : 0;\n ++\tc = i > 2 ? range[--i] : 0;\n +\tif (c == '!')\n -+\t\ti--; /* might be ...^! or ...^@ */\n ++\t\ti--; /* might be ...^! */\n +\telse if (isdigit(c)) {\n +\t\t/* handle ...^- */\n +\t\twhile (i > 2 && isdigit(range[--i]))\n @@ builtin/range-diff.c: NULL\n +\t} else\n +\t\treturn 0;\n +\n ++\t/* Before the `!` or the `-`, we expect `^` */\n +\treturn i > 0 && range[i] == '^';\n }\n - \n - int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \n ## t/t3206-range-diff.sh ##\n @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '\n 3: 041456b6e73 = 3: 4f5e5acd954 range-diff(docs): explain how to specify commit ranges\n\n-- \ngitgitgadget\n"},{"id":"414987","messageId":"3f21e10f919eead049dc2440a29bb2bed6c99d0d.1611339373.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v2.git.1611339373.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-22T18:16:10Z","receivedAt":"2021-01-22T19:17:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, `git range-diff`\ntests for a literal `..` in each of the two. Likewise, the argument\nprovided via `--range-diff` to `git format-patch` is checked in the same\nmanner.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the check into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n revision.c | 5 +++++\n revision.h | 7 +++++++\n 4 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bd6ff4f9f95..099abdfb7e6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1680,7 +1680,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n-\tint prev_is_range = !!strstr(prev, \"..\");\n+\tint prev_is_range = specifies_commit_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f744..89d54158011 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -3,6 +3,7 @@\n #include \"parse-options.h\"\n #include \"range-diff.h\"\n #include \"config.h\"\n+#include \"revision.h\"\n \n static const char * const builtin_range_diff_usage[] = {\n N_(\"git range-diff [] .. ..\"),\n@@ -46,12 +47,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!specifies_commit_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!specifies_commit_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\ndiff --git a/revision.c b/revision.c\nindex 9dff845bed6..00675f598a3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -4206,3 +4206,8 @@ void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n \tfputs(mark, stdout);\n \tputchar(' ');\n }\n+\n+int specifies_commit_range(const char *range)\n+{\n+\treturn !!strstr(range, \"..\");\n+}\ndiff --git a/revision.h b/revision.h\nindex 086ff10280d..66777c8e60f 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -457,4 +457,11 @@ int rewrite_parents(struct rev_info *revs,\n */\n struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);\n \n+/*\n+ * Determine whether the given argument defines a commit range, e.g. A..B.\n+ * Note that this only validates the format but does _not_ parse it, i.e.\n+ * it does _not_ look up the specified commits in the local repository.\n+ */\n+int specifies_commit_range(const char *range);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"414989","messageId":"0ce0953a-92b2-2ce6-1e13-6f6240933ce1@gmail.com","threadId":"55025","inReplyTo":"5839ba4f7615819ed6f9a0fcb6de1855cd2e89e1.1611267638.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] range-diff: refactor check for commit range","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-01-22T19:12:54Z","receivedAt":"2021-01-22T19:35:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 21/01/2021 22:20, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Currently, when called with exactly two arguments, we test for a literal\n> `..` in each of the two.\n> \n> However, `^!` is a perfectly valid commit range, equivalent to\n> `^..` according to the `SPECIFYING RANGES` section of\n> gitrevisions[7].\n> \n> In preparation for allowing more sophisticated ways to specify commit\n> ranges, let's refactor the conditional into its own function.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/range-diff.c | 13 +++++++++----\n> 1 file changed, 9 insertions(+), 4 deletions(-)\n> \n> diff --git a/builtin/range-diff.c b/builtin/range-diff.c\n> index 24c4162f744..551d3e689cb 100644\n> --- a/builtin/range-diff.c\n> +++ b/builtin/range-diff.c\n> @@ -11,6 +11,11 @@ N_(\"git range-diff [] \"),\n> NULL\n> };\n> \n> +static int is_range(const char *range)\n> +{\n> +\treturn !!strstr(range, \"..\");\n> +}\n\nIf the user wrongly passes two arguments referring to single commits \nwith `:/` or `@{/}` where text contains \"..\" this will give \na false positive.\n\nBest Wishes\n\nPhillip\n\n> int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> {\n> \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n> @@ -46,12 +51,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n> \t\tdiffopt.use_color = 1;\n> \n> \tif (argc == 2) {\n> -\t\tif (!strstr(argv[0], \"..\"))\n> -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n> +\t\tif (!is_range(argv[0]))\n> +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n> \t\tstrbuf_addstr(&range1, argv[0]);\n> \n> -\t\tif (!strstr(argv[1], \"..\"))\n> -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n> +\t\tif (!is_range(argv[1]))\n> +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n> \t\tstrbuf_addstr(&range2, argv[1]);\n> \t} else if (argc == 3) {\n> \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\n> \n\n"},{"id":"414993","messageId":"xmqqv9bo7lwd.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"2c2744333ecf5662d4198bdddeee80ff4adf6acd.1611339373.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-22T20:32:34Z","receivedAt":"2021-01-22T20:35:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> +\ti = strlen(range);\n> +\tc = i > 2 ? range[--i] : 0;\n> +\tif (c == '!')\n> +\t\ti--; /* might be ...^! */\n> +\telse if (isdigit(c)) {\n> +\t\t/* handle ...^- */\n> +\t\twhile (i > 2 && isdigit(range[--i]))\n> +\t\t\t; /* keep trimming trailing digits */\n> +\t\tif (i < 2 || range[i--] != '-')\n> +\t\t\treturn 0;\n> +\t} else\n> +\t\treturn 0;\n> +\n> +\t/* Before the `!` or the `-`, we expect `^` */\n> +\treturn i > 0 && range[i] == '^';\n\nThis is still way too complex for my liking, but at least I cannot\nimmediately spot a glaring off-by-one like the previous round ;-)\n\nThis is a tangent ([*1*]), but we often equate an omission to\nimplicitly specified HEAD; e.g. \"git log @{u}..\" is what we did\nsince we started building on top of our upstream. I wonder if it\nmakes sense to allow similar short-hand so that ^! alone would mean\nHEAD^!, ^@ alone would mean HEAD^@, and so on.\n\nThanks.\n\n\n[Footnote]\n\n*1* read: this has nothing to do with how ready I think this patch\n is, but the topic reminds me of it strongly enough that I raise\n it here, because I know the opinions on this unrelated thing on\n recipients of this response are worth listening to.\n\n"},{"id":"414995","messageId":"xmqqzh107m5d.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"3f21e10f919eead049dc2440a29bb2bed6c99d0d.1611339373.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-22T20:27:10Z","receivedAt":"2021-01-22T20:49:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Currently, when called with exactly two arguments, `git range-diff`\n> tests for a literal `..` in each of the two. Likewise, the argument\n> provided via `--range-diff` to `git format-patch` is checked in the same\n> manner.\n>\n> However, `^!` is a perfectly valid commit range, equivalent to\n> `^..` according to the `SPECIFYING RANGES` section of\n> gitrevisions[7].\n>\n> In preparation for allowing more sophisticated ways to specify commit\n> ranges, let's refactor the check into its own function.\n\nI think the sharing between the two makes sense, but the helper\nfunction should make it clear in its name that this is \"the kind of\ncommit range range-diff wants to take\". Among the commit range \"git\nlog\" and friends can take, range-diff can take only a subset of it,\nand only a subset of it is meaningful to range-diff (e.g. HEAD^@ is\nstill a commit range you can give to \"git log\", but it would not\nmake much sense to give it to range-diff).\n\nPerhaps\n\n s/specifies_commit_range/is_range_diff_range/\n\nor something.\n\n> diff --git a/revision.h b/revision.h\n\nMove this to range-diff.h, not revision.h which is about true commit\nranges.\n\n> index 086ff10280d..66777c8e60f 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -457,4 +457,11 @@ int rewrite_parents(struct rev_info *revs,\n> */\n> struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);\n> \n> +/*\n> + * Determine whether the given argument defines a commit range, e.g. A..B.\n> + * Note that this only validates the format but does _not_ parse it, i.e.\n> + * it does _not_ look up the specified commits in the local repository.\n> + */\n\nAnd s/defines a commit range/is usable as a range to range-diff/\n\nThanks.\n\n> +int specifies_commit_range(const char *range);\n> +\n> #endif\n"},{"id":"415006","messageId":"xmqqbldg7hvb.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"0ce0953a-92b2-2ce6-1e13-6f6240933ce1@gmail.com","subject":"Re: [PATCH 1/3] range-diff: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-22T21:59:36Z","receivedAt":"2021-01-22T22:01:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n>> +static int is_range(const char *range)\n>> +{\n>> +\treturn !!strstr(range, \"..\");\n>> +}\n>\n> If the user wrongly passes two arguments referring to single commits\n> with `:/` or `@{/}` where text contains \"..\" this will\n> give a false positive.\n\nTrue. I do not think this aims to be complete revision parser in\nthe first place, though.\n\nIt is tempting to at least idly speculate if an approach to run\nsetup_revisions() on argument is_range() takes and checking the\nresult would yield a more practical solution. I would imagine that\nwe would want to see in the resulting revs.pending has at least one\npositive and one negative, and none of them have SYMMETRIC_LEFT set\nin their .flags word.\n\n Side note: Strictly speaking, people could wish \"rev\" to mean\n \"everything reachable from the rev, down to root\", so\n requiring one negative may technically be a wrong\n thing, but in the context of \"range-diff\", I am not\n sure how useful such a behaviour would be.\n"},{"id":"415069","messageId":"a0b100e7-57b8-df46-de2b-d54fdbe6a255@gmail.com","threadId":"55025","inReplyTo":"xmqqbldg7hvb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/3] range-diff: refactor check for commit range","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-01-23T15:59:23Z","receivedAt":"2021-01-23T16:00:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 22/01/2021 21:59, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>>> +static int is_range(const char *range)\n>>> +{\n>>> +\treturn !!strstr(range, \"..\");\n>>> +}\n>>\n>> If the user wrongly passes two arguments referring to single commits\n>> with `:/` or `@{/}` where text contains \"..\" this will\n>> give a false positive.\n> \n> True. I do not think this aims to be complete revision parser in\n> the first place, though.\n\nYes but it affects the error message given to the user. If I run\n\ngit range-diff $(git rev-parse HEAD^{/..q}) $(git rev-parse HEAD^{/..x})\n\nIt fails immediately with\n\nfatal: no .. in range: 'b60863619cf47b2e1e891c2376bd4f6da8111ab1'\n\nThis patch improves the error message to say it is not a range\n\nbut\n\ngit range-diff HEAD^{/..q} HEAD^{/..x}\n\nruns for several minutes without producing any output and eventually \nfails with\n\nfatal: Out of memory, malloc failed (tried to allocate 33846432676 bytes)\n\nSo I think it would be helpful to be more careful here.\n\nBest Wishes\n\nPhillip\n\n\n> It is tempting to at least idly speculate if an approach to run\n> setup_revisions() on argument is_range() takes and checking the\n> result would yield a more practical solution. I would imagine that\n> we would want to see in the resulting revs.pending has at least one\n> positive and one negative, and none of them have SYMMETRIC_LEFT set\n> in their .flags word.\n> \n> Side note: Strictly speaking, people could wish \"rev\" to mean\n> \"everything reachable from the rev, down to root\", so\n> requiring one negative may technically be a wrong\n> thing, but in the context of \"range-diff\", I am not\n> sure how useful such a behaviour would be.\n> \n"},{"id":"415191","messageId":"20210125073508.l7ksohyfhcogch4x@pengutronix.de","threadId":"55025","inReplyTo":"xmqqzh107m5d.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2021-01-25T07:35:08Z","receivedAt":"2021-01-25T07:37:06Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"On Fri, Jan 22, 2021 at 12:27:10PM -0800, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > From: Johannes Schindelin \n> >\n> > Currently, when called with exactly two arguments, `git range-diff`\n> > tests for a literal `..` in each of the two. Likewise, the argument\n> > provided via `--range-diff` to `git format-patch` is checked in the same\n> > manner.\n> >\n> > However, `^!` is a perfectly valid commit range, equivalent to\n> > `^..` according to the `SPECIFYING RANGES` section of\n> > gitrevisions[7].\n> >\n> > In preparation for allowing more sophisticated ways to specify commit\n> > ranges, let's refactor the check into its own function.\n> \n> I think the sharing between the two makes sense, but the helper\n> function should make it clear in its name that this is \"the kind of\n> commit range range-diff wants to take\". Among the commit range \"git\n> log\" and friends can take, range-diff can take only a subset of it,\n> and only a subset of it is meaningful to range-diff (e.g. HEAD^@ is\n> still a commit range you can give to \"git log\", but it would not\n> make much sense to give it to range-diff).\n\nDoes it make so little sense to forbid passing HEAD^@ as a range to\nrange-diff? I can imagine situations where is would make sense, e.g. I\noften create customer patch stacks from a set of topic branches using\noctopus merge. To compare two of these ^@ might be handy.\n\nMy POV is that if it's easy to use the same function (and so the same\nset of range descriptors) for git log and git range-diff then do so.\nThis yields a consistent behaviour which is IMHO better than preventing\npeople to do things that are considered strange today.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | https://www.pengutronix.de/ |\n"},{"id":"415242","messageId":"20210125212525.dpnsj7ejngvpkd5y@pengutronix.de","threadId":"55025","inReplyTo":"xmqqpn1syg3s.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2021-01-25T21:25:25Z","receivedAt":"2021-01-25T21:26:34Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Junio,\n\nOn Mon, Jan 25, 2021 at 11:24:39AM -0800, Junio C Hamano wrote:\n> Uwe Kleine-König writes:\n> \n> >> > In preparation for allowing more sophisticated ways to specify commit\n> >> > ranges, let's refactor the check into its own function.\n> >> \n> >> I think the sharing between the two makes sense, but the helper\n> >> function should make it clear in its name that this is \"the kind of\n> >> commit range range-diff wants to take\". Among the commit range \"git\n> >> log\" and friends can take, range-diff can take only a subset of it,\n> >> and only a subset of it is meaningful to range-diff (e.g. HEAD^@ is\n> >> still a commit range you can give to \"git log\", but it would not\n> >> make much sense to give it to range-diff).\n> >\n> > Does it make so little sense to forbid passing HEAD^@ as a range to\n> > range-diff? I can imagine situations where is would make sense, e.g. I\n> > often create customer patch stacks from a set of topic branches using\n> > octopus merge. To compare two of these ^@ might be handy.\n> \n> You can discuss for each individual syntax of a single-token range\n> and decide which ones are and are not suitable for range-diff, but I\n> suspect the reason behind this business started with dot-dot is to\n> perform a superficial \"sanity check\" at the command line parser\n> level before passing them to the revision machinery, and having to\n> deal with potential errors and/or having to compare unreasonably\n> large segments of history that the user did not intend.\n> \n> Also I first thought that the command changes the behaviour, given\n> two tokens, depending on the shape of these two tokens (i.e. when\n> they satisfy the \"is-range?\" we are discussing, they are taken as\n> two ranges to be compared, and otherwise does something else), but\n> after revisiting the code and \"git help range-diff\", it always does\n> one thing when given \n> \n> (1) one arg: gives a symmetric range and what is to be compared\n> is its left and right half,\n> \n> (2) two args: each is meant to name a set of commits and these two\n> are to be compared) or\n> \n> (3) three args: each is meant to name a commit, and the arg1..arg2\n> and arg1..arg3 are the ranges to be compared.\n> \n> so ...\n> \n> > My POV is that if it's easy to use the same function (and so the same\n> > set of range descriptors) for git log and git range-diff then do so.\n> > This yields a consistent behaviour which is IMHO better than preventing\n> > people to do things that are considered strange today.\n> \n> ... I am OK with that point of view. It certainly is simpler to\n> explain to end users.\n\nIt seems you understood my argument :-)\n\n> Having said that, it would make it much harder to implement\n> efficiently, though. For example, when your user says\n> \n> \tgit range-diff A B\n> \n> to compare \"git log A\" (all the way down to the root) and \"git log\n> B\" (likewise), you'd certainly optimize the older common part of the\n> history out, essentially turning it into\n> \n> \tgit range-diff A..B B..A\n> \n> or its moral equivalent\n> \n> \tgit range-diff A...B\n> \n> But you cannot apply such an optimization blindly. When the user\n> gives A..B and B..A as two args, you somehow need to notice that \n> you shouldn't rewrite it to \"A..B...B..A\", and for that, you'd still\n> need some \"parsing\" of these args.\n\nI agree that for a long history\n\n\tgit range-diff A B\n\nis an expensive request and I wouldn't invest too many cycles optimizing\nit. (And if I'd optimize it, it wouldn't be done using textual\ncombination of the two strings but by checking if the two ranges\nintersect. This way something like\n\n\tgit range-diff v4.0..v4.6-rc1 v4.0..v4.5.6\n\nand maybe even\n\n\tgit range-diff v4.0..v4.6-rc1 v4.0-rc1..v4.5.6\n\nwould benefit, too. But note I'm not (anymore) familiar with the git\nsource code, so I don't know if this is easy/sensible to do and I'm just\nlooking at the problem from an architectural and theoretical POV.)\n\n> So, I dunno. Limiting the second form to only forms that the\n> implementation does not have to do such optimization would certainly\n> make it simpler for Dscho to implement ;-)\n\nI don't want to make it more complicated for Dscho, I'm happy if I can\nin the near future use range-diff with $rev1^! $ref2^! . So feel free to\nignore me.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | https://www.pengutronix.de/ |\n"},{"id":"415287","messageId":"nycvar.QRO.7.76.6.2101261618520.57@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"a0b100e7-57b8-df46-de2b-d54fdbe6a255@gmail.com","subject":"Re: [PATCH 1/3] range-diff: refactor check for commit range","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-26T15:19:34Z","receivedAt":"2021-01-26T15:22:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Sat, 23 Jan 2021, Phillip Wood wrote:\n\n> On 22/01/2021 21:59, Junio C Hamano wrote:\n> > Phillip Wood writes:\n> >\n> > > > +static int is_range(const char *range)\n> > > > +{\n> > > > +\treturn !!strstr(range, \"..\");\n> > > > +}\n> > >\n> > > If the user wrongly passes two arguments referring to single commits\n> > > with `:/` or `@{/}` where text contains \"..\" this will\n> > > give a false positive.\n> >\n> > True. I do not think this aims to be complete revision parser in\n> > the first place, though.\n>\n> Yes but it affects the error message given to the user.\n\nTrue. But my patch series does not try to fix that (it is not an issue\n_introduced_ by this patch series, it was there all along).\n\nCiao,\nDscho\n"},{"id":"415288","messageId":"nycvar.QRO.7.76.6.2101261619570.57@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"20210122182050.xyzkvlctb4kiin7a@pengutronix.de","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-26T15:22:20Z","receivedAt":"2021-01-26T15:25:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Uwe,\n\nOn Fri, 22 Jan 2021, Uwe Kleine-König wrote:\n\n> On Thu, Jan 21, 2021 at 10:20:38PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > There are three forms, depending whether the user specifies one, two or\n> > three non-option arguments. We've never actually explained how this\n> > works in the manual, so let's explain it.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > Documentation/git-range-diff.txt | 13 +++++++++++++\n> > 1 file changed, 13 insertions(+)\n> >\n> > diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> > index 9701c1e5fdd..76359baf26d 100644\n> > --- a/Documentation/git-range-diff.txt\n> > +++ b/Documentation/git-range-diff.txt\n> > @@ -28,6 +28,19 @@ Finally, the list of matching commits is shown in the order of the\n> > second commit range, with unmatched commits being inserted just after\n> > all of their ancestors have been shown.\n> >\n> > +There are three ways to specify the commit ranges:\n> > +\n> > +- ` `: Either commit range can be of the form\n> > + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> > + in linkgit:gitrevisions[7] for more details.\n> > +\n> > +- `...`. This resembles the symmetric ranges mentioned in\n> > + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> > + equivalent to `.. ..` where `` is the\n> > + merge base as obtained via `git merge-base `.\n> > +\n> > +- ` `: This is equivalent to `..\n> > + ..`.\n>\n> git-log takes a range, too. There you can specify a single rev (with the\n> semantic to list all commits from this rev up (or down?) to the root).\n> So means implicitly ^∞.. for git-log.\n>\n> Does it make sense to implement this here, too? Maybe this even allows\n> sharing some more code?\n\nI don't think that it makes sense to support open-ended ranges. `git\nrange-diff` is expensive, its runtime is proportional to the number of\npatches in the first range times the number of patches in the second\nrange. Allowing open-ended ranges will simply allow users to be stuck with\na long runtime by mistake, and I do not see any valid use case in return\nfor that risk.\n\nCiao,\nJohannes\n"},{"id":"415319","messageId":"xmqqpn1syg3s.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"20210125073508.l7ksohyfhcogch4x@pengutronix.de","subject":"Re: [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-25T19:24:39Z","receivedAt":"2021-01-26T20:46:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König writes:\n\n>> > In preparation for allowing more sophisticated ways to specify commit\n>> > ranges, let's refactor the check into its own function.\n>> \n>> I think the sharing between the two makes sense, but the helper\n>> function should make it clear in its name that this is \"the kind of\n>> commit range range-diff wants to take\". Among the commit range \"git\n>> log\" and friends can take, range-diff can take only a subset of it,\n>> and only a subset of it is meaningful to range-diff (e.g. HEAD^@ is\n>> still a commit range you can give to \"git log\", but it would not\n>> make much sense to give it to range-diff).\n>\n> Does it make so little sense to forbid passing HEAD^@ as a range to\n> range-diff? I can imagine situations where is would make sense, e.g. I\n> often create customer patch stacks from a set of topic branches using\n> octopus merge. To compare two of these ^@ might be handy.\n\nYou can discuss for each individual syntax of a single-token range\nand decide which ones are and are not suitable for range-diff, but I\nsuspect the reason behind this business started with dot-dot is to\nperform a superficial \"sanity check\" at the command line parser\nlevel before passing them to the revision machinery, and having to\ndeal with potential errors and/or having to compare unreasonably\nlarge segments of history that the user did not intend.\n\nAlso I first thought that the command changes the behaviour, given\ntwo tokens, depending on the shape of these two tokens (i.e. when\nthey satisfy the \"is-range?\" we are discussing, they are taken as\ntwo ranges to be compared, and otherwise does something else), but\nafter revisiting the code and \"git help range-diff\", it always does\none thing when given \n\n (1) one arg: gives a symmetric range and what is to be compared\n is its left and right half,\n\n (2) two args: each is meant to name a set of commits and these two\n are to be compared) or\n\n (3) three args: each is meant to name a commit, and the arg1..arg2\n and arg1..arg3 are the ranges to be compared.\n\nso ...\n\n> My POV is that if it's easy to use the same function (and so the same\n> set of range descriptors) for git log and git range-diff then do so.\n> This yields a consistent behaviour which is IMHO better than preventing\n> people to do things that are considered strange today.\n\n... I am OK with that point of view. It certainly is simpler to\nexplain to end users.\n\nHaving said that, it would make it much harder to implement\nefficiently, though. For example, when your user says\n\n\tgit range-diff A B\n\nto compare \"git log A\" (all the way down to the root) and \"git log\nB\" (likewise), you'd certainly optimize the older common part of the\nhistory out, essentially turning it into\n\n\tgit range-diff A..B B..A\n\nor its moral equivalent\n\n\tgit range-diff A...B\n\nBut you cannot apply such an optimization blindly. When the user\ngives A..B and B..A as two args, you somehow need to notice that \nyou shouldn't rewrite it to \"A..B...B..A\", and for that, you'd still\nneed some \"parsing\" of these args.\n\nSo, I dunno. Limiting the second form to only forms that the\nimplementation does not have to do such optimization would certainly\nmake it simpler for Dscho to implement ;-)\n"},{"id":"415348","messageId":"xmqqzh0v1ox4.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"20210125212525.dpnsj7ejngvpkd5y@pengutronix.de","subject":"Re: [PATCH v2 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-26T19:25:11Z","receivedAt":"2021-01-26T22:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König writes:\n\n>> > My POV is that if it's easy to use the same function (and so the same\n>> > set of range descriptors) for git log and git range-diff then do so.\n>> > This yields a consistent behaviour which is IMHO better than preventing\n>> > people to do things that are considered strange today.\n>> \n>> ... I am OK with that point of view. It certainly is simpler to\n>> explain to end users.\n>\n> It seems you understood my argument :-)\n\nSo it seems ;-).\n\n>> Having said that, it would make it much harder to implement\n>> efficiently, though. For example, when your user says\n>> \n>> \tgit range-diff A B\n>> \n>> to compare \"git log A\" (all the way down to the root) and \"git log\n>> B\" (likewise), you'd certainly optimize the older common part of the\n>> history out, essentially turning it into\n>> \n>> \tgit range-diff A..B B..A\n>> \n>> or its moral equivalent\n>> \n>> \tgit range-diff A...B\n>> \n>> But you cannot apply such an optimization blindly. When the user\n>> gives A..B and B..A as two args, you somehow need to notice that \n>> you shouldn't rewrite it to \"A..B...B..A\", and for that, you'd still\n>> need some \"parsing\" of these args.\n>\n> I agree that for a long history\n>\n> \tgit range-diff A B\n>\n> is an expensive request and I wouldn't invest too many cycles optimizing\n> it.\n\nWell, your devil's advocate can argue that accepting an input that\ncan easily make the tool unusable would be irresponsible, though.\n\nAnd there are two possible ways out:\n\n (1) declare that optimizing \"A B\" into \"A...B\" is way too difficult\n to do in general, and find a good enough way to see if A and B\n individually gives a \"range\" that should be manageable; or\n\n (2) invest cycles to optimize, so your users do not have to care.\n\nI think the series takes the former approach, and I find it\nunderstandable.\n\nIt is a different matter if the way found and implemented in the\npatch is \"good enough\" to tell if a given argument names a\nmanageable range. You said something about \"rev^{/^here are\ntwo..dots}\" that can be mistaken as a \"good enough\" range, but it\nactually names a revision and all the way down to the root.\n\n> (And if I'd optimize it, it wouldn't be done using textual\n> combination of the two strings but by checking if the two ranges\n> intersect.\n\nYup, that is in line with what I mumbled earlier about\nsetup_revisions() and inspecting the rev_info.pending, I think.\n\n>> So, I dunno. Limiting the second form to only forms that the\n>> implementation does not have to do such optimization would certainly\n>> make it simpler for Dscho to implement ;-)\n>\n> I don't want to make it more complicated for Dscho, I'm happy if I can\n> in the near future use range-diff with $rev1^! $ref2^! . So feel free to\n> ignore me.\n>\n> Best regards\n> Uwe\n\n"},{"id":"415440","messageId":"nycvar.QRO.7.76.6.2101270357480.57@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqq1recal32.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-27T03:01:33Z","receivedAt":"2021-01-27T15:45:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 22 Jan 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> >> > +- `...`. This resembles the symmetric ranges mentioned in\n> >> > + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> >> > + equivalent to `.. ..` where `` is the\n> >> > + merge base as obtained via `git merge-base `.\n>\n> The above paragraph says A...B is turned into $(git merge-base A\n> B)..A and $(git merge-base A B)..B, but I wonder if we should be\n> rewriting it into A..B and B..A instead; that would make it\n> unnecessary to explain what should happen when there are more than\n> one merge bases.\n\nYou know what? I lied. The code already does what you suggested. Look\n[here](https://github.com/git/git/blob/v2.30.0/builtin/range-diff.c#L59-L77):\n\n\t[...]\n } else if (argc == 1) {\n const char *b = strstr(argv[0], \"...\"), *a = argv[0];\n int a_len;\n\n if (!b) {\n error(_(\"single arg format must be symmetric range\"));\n usage_with_options(builtin_range_diff_usage, options);\n }\n\n a_len = (int)(b - a);\n if (!a_len) {\n a = \"HEAD\";\n a_len = strlen(a);\n }\n b += 3;\n if (!*b)\n b = \"HEAD\";\n strbuf_addf(&range1, \"%s..%.*s\", b, a_len, a);\n strbuf_addf(&range2, \"%.*s..%s\", a_len, a, b);\n\t[...]\n\n> >> Does this merely resemble? Isn't it exactly what a symmetric range is?\n> >\n> > No, it is not exactly what a symmetric range is because `range-diff`\n> > treats both arms of the symmetric range independently, as two distinct\n> > non-symmetric ranges.\n>\n> This however is an end-user documentation, isn't it?\n\nYes, and the end user is talking about _two_ commit ranges in the context\nof `git range-diff`, and about _one_ commit range in the context of `git\nlog`.\n\nSo I still think that it really just only resembles the symmetric range.\nIt is fundamentally as different from it as the number 2 is different from\nthe number 1.\n\nCiao,\nDscho\n"},{"id":"415441","messageId":"nycvar.QRO.7.76.6.2101270342140.57@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqv9bo7lwd.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-27T02:57:15Z","receivedAt":"2021-01-27T15:52:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 22 Jan 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > +\ti = strlen(range);\n> > +\tc = i > 2 ? range[--i] : 0;\n> > +\tif (c == '!')\n> > +\t\ti--; /* might be ...^! */\n> > +\telse if (isdigit(c)) {\n> > +\t\t/* handle ...^- */\n> > +\t\twhile (i > 2 && isdigit(range[--i]))\n> > +\t\t\t; /* keep trimming trailing digits */\n> > +\t\tif (i < 2 || range[i--] != '-')\n> > +\t\t\treturn 0;\n> > +\t} else\n> > +\t\treturn 0;\n> > +\n> > +\t/* Before the `!` or the `-`, we expect `^` */\n> > +\treturn i > 0 && range[i] == '^';\n>\n> This is still way too complex for my liking, but at least I cannot\n> immediately spot a glaring off-by-one like the previous round ;-)\n\nMaybe I am too stuck with the idea of avoiding regular expressions after\nthat StackOverflow incident... Maybe\n\n\tstatic regex_t *regex;\n\n\tif (strstr(range, \"..\"))\n\t\treturn 1;\n\n\tif (!regex) {\n\t\tregex = xmalloc(sizeof(*regex));\n\t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED))\n\t\t\tBUG(\"could not compile regex\");\n\t}\n\n\treturn !regexec(regex, range, 0, NULL, 0);\n\nis more readable, and acceptable in this context?\n\n> This is a tangent ([*1*]), but we often equate an omission to\n> implicitly specified HEAD; e.g. \"git log @{u}..\" is what we did\n> since we started building on top of our upstream. I wonder if it\n> makes sense to allow similar short-hand so that ^! alone would mean\n> HEAD^!, ^@ alone would mean HEAD^@, and so on.\n\nI don't know. On one hand, it would make things more consistent. On the\nother hand, it would be relatively hard to explain, I think. And we\nalready have the shortcut `@!^`, which is hard enough to explain as it is.\n\nCiao,\nDscho\n"},{"id":"415447","messageId":"pull.841.v3.git.1611765444.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v2.git.1611339373.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-27T16:37:21Z","receivedAt":"2021-01-27T16:40:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nChanges since v2:\n\n * Move the helper function from revision.c to range-diff.c and rename it.\n * Use a regex to make it easier to understand what we're trying to match.\n * Fix the documentation that claimed that we used git merge-base internally\n when git range-diff parses ...-style arguments, which is not the case.\n\nChanges since v1:\n\n * In addition to git range-diff, git format-patch --range-diff gets the\n same improvement.\n * The comment talking about ^@ was removed.\n * The parsing was made a bit safer (e.g. catching ! by its own as an\n invalid range).\n\nJohannes Schindelin (3):\n range-diff/format-patch: refactor check for commit range\n range-diff/format-patch: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 12 ++++++++++++\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 17 +++++++++++++++++\n range-diff.h | 8 ++++++++\n t/t3206-range-diff.sh | 8 ++++++++\n 6 files changed, 51 insertions(+), 5 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n\nRange-diff vs v2:\n\n 1: 3f21e10f919 ! 1: b98fa94b870 range-diff/format-patch: refactor check for commit range\n @@ builtin/log.c: static void infer_range_diff_ranges(struct strbuf *r1,\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n -\tint prev_is_range = !!strstr(prev, \"..\");\n -+\tint prev_is_range = specifies_commit_range(prev);\n ++\tint prev_is_range = is_range_diff_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\n @@ builtin/range-diff.c: int cmd_range_diff(int argc, const char **argv, const char\n \tif (argc == 2) {\n -\t\tif (!strstr(argv[0], \"..\"))\n -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n -+\t\tif (!specifies_commit_range(argv[0]))\n ++\t\tif (!is_range_diff_range(argv[0]))\n +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n -\t\tif (!strstr(argv[1], \"..\"))\n -\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n -+\t\tif (!specifies_commit_range(argv[1]))\n ++\t\tif (!is_range_diff_range(argv[1]))\n +\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\n \n - ## revision.c ##\n -@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n - \tfputs(mark, stdout);\n - \tputchar(' ');\n + ## range-diff.c ##\n +@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,\n + \n + \treturn res;\n }\n +\n -+int specifies_commit_range(const char *range)\n ++int is_range_diff_range(const char *arg)\n +{\n -+\treturn !!strstr(range, \"..\");\n ++\treturn !!strstr(arg, \"..\");\n +}\n \n - ## revision.h ##\n -@@ revision.h: int rewrite_parents(struct rev_info *revs,\n - */\n - struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);\n + ## range-diff.h ##\n +@@ range-diff.h: int show_range_diff(const char *range1, const char *range2,\n + \t\t const struct diff_options *diffopt,\n + \t\t const struct strvec *other_arg);\n \n +/*\n -+ * Determine whether the given argument defines a commit range, e.g. A..B.\n -+ * Note that this only validates the format but does _not_ parse it, i.e.\n -+ * it does _not_ look up the specified commits in the local repository.\n ++ * Determine whether the given argument is usable as a range argument of `git\n ++ * range-diff`, e.g. A..B. Note that this only validates the format but does\n ++ * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n ++ * local repository.\n + */\n -+int specifies_commit_range(const char *range);\n ++int is_range_diff_range(const char *arg);\n +\n #endif\n 2: 2c2744333ec ! 2: 0880ca587e6 range-diff/format-patch: handle commit ranges other than A..B\n @@ Commit message\n \n Signed-off-by: Johannes Schindelin \n \n - ## revision.c ##\n -@@ revision.c: void put_revision_mark(const struct rev_info *revs, const struct commit *commit)\n + ## range-diff.c ##\n +@@ range-diff.c: int show_range_diff(const char *range1, const char *range2,\n \n - int specifies_commit_range(const char *range)\n + int is_range_diff_range(const char *arg)\n {\n --\treturn !!strstr(range, \"..\");\n -+\tsize_t i;\n -+\tchar c;\n +-\treturn !!strstr(arg, \"..\");\n ++\tstatic regex_t *regex;\n +\n -+\tif (strstr(range, \"..\"))\n ++\tif (strstr(arg, \"..\"))\n +\t\treturn 1;\n +\n -+\ti = strlen(range);\n -+\tc = i > 2 ? range[--i] : 0;\n -+\tif (c == '!')\n -+\t\ti--; /* might be ...^! */\n -+\telse if (isdigit(c)) {\n -+\t\t/* handle ...^- */\n -+\t\twhile (i > 2 && isdigit(range[--i]))\n -+\t\t\t; /* keep trimming trailing digits */\n -+\t\tif (i < 2 || range[i--] != '-')\n -+\t\t\treturn 0;\n -+\t} else\n -+\t\treturn 0;\n ++\t/* match `^!` and `^-` */\n ++\tif (!regex) {\n ++\t\tregex = xmalloc(sizeof(*regex));\n ++\t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED) < 0)\n ++\t\t\tBUG(\"could not compile range-diff regex\");\n ++\t}\n +\n -+\t/* Before the `!` or the `-`, we expect `^` */\n -+\treturn i > 0 && range[i] == '^';\n ++\treturn !regexec(regex, arg, 0, NULL, 0);\n }\n \n ## t/t3206-range-diff.sh ##\n 3: 4f5e5acd954 ! 3: 5ab9321a34c range-diff(docs): explain how to specify commit ranges\n @@ Documentation/git-range-diff.txt: Finally, the list of matching commits is shown\n +\n +- `...`. This resembles the symmetric ranges mentioned in\n + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n -+ equivalent to `.. ..` where `` is the\n -+ merge base as obtained via `git merge-base `.\n ++ equivalent to `.. ..`.\n +\n +- ` `: This is equivalent to `..\n + ..`.\n\n-- \ngitgitgadget\n"},{"id":"415448","messageId":"5ab9321a34ca15b4f532022ad0bab431fb201b07.1611765444.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v3.git.1611765444.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-27T16:37:24Z","receivedAt":"2021-01-27T16:40:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd..14bffb272a0 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This resembles the symmetric ranges mentioned in\n+ the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n+ equivalent to `.. ..`.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"415449","messageId":"0880ca587e63b252dc24d98b2e7590b2c0fcd881.1611765444.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v3.git.1611765444.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-27T16:37:23Z","receivedAt":"2021-01-27T16:40:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them.\n\nSigned-off-by: Johannes Schindelin \n---\n range-diff.c | 14 +++++++++++++-\n t/t3206-range-diff.sh | 8 ++++++++\n 2 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 9b93e08e840..0c6ac4f954d 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -567,5 +567,17 @@ int show_range_diff(const char *range1, const char *range2,\n \n int is_range_diff_range(const char *arg)\n {\n-\treturn !!strstr(arg, \"..\");\n+\tstatic regex_t *regex;\n+\n+\tif (strstr(arg, \"..\"))\n+\t\treturn 1;\n+\n+\t/* match `^!` and `^-` */\n+\tif (!regex) {\n+\t\tregex = xmalloc(sizeof(*regex));\n+\t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED) < 0)\n+\t\t\tBUG(\"could not compile range-diff regex\");\n+\t}\n+\n+\treturn !regexec(regex, arg, 0, NULL, 0);\n }\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be031..e217cecac9e 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"415450","messageId":"b98fa94b87037b811ea973c1aeb7cfe08d7c1bd6.1611765444.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v3.git.1611765444.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-01-27T16:37:22Z","receivedAt":"2021-01-27T16:40:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, `git range-diff`\ntests for a literal `..` in each of the two. Likewise, the argument\nprovided via `--range-diff` to `git format-patch` is checked in the same\nmanner.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the check into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 5 +++++\n range-diff.h | 8 ++++++++\n 4 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bd6ff4f9f95..aeece57e86a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1680,7 +1680,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n-\tint prev_is_range = !!strstr(prev, \"..\");\n+\tint prev_is_range = is_range_diff_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f744..5b1f6326322 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -3,6 +3,7 @@\n #include \"parse-options.h\"\n #include \"range-diff.h\"\n #include \"config.h\"\n+#include \"revision.h\"\n \n static const char * const builtin_range_diff_usage[] = {\n N_(\"git range-diff [] .. ..\"),\n@@ -46,12 +47,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!is_range_diff_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!is_range_diff_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\ndiff --git a/range-diff.c b/range-diff.c\nindex b9950f10c8c..9b93e08e840 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,\n \n \treturn res;\n }\n+\n+int is_range_diff_range(const char *arg)\n+{\n+\treturn !!strstr(arg, \"..\");\n+}\ndiff --git a/range-diff.h b/range-diff.h\nindex 583ced2e8e7..c17dbc2e75a 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n \t\t const struct diff_options *diffopt,\n \t\t const struct strvec *other_arg);\n \n+/*\n+ * Determine whether the given argument is usable as a range argument of `git\n+ * range-diff`, e.g. A..B. Note that this only validates the format but does\n+ * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n+ * local repository.\n+ */\n+int is_range_diff_range(const char *arg);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"415480","messageId":"xmqqim7hy5te.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"nycvar.QRO.7.76.6.2101270357480.57@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-28T05:43:41Z","receivedAt":"2021-01-28T05:44:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The above paragraph says A...B is turned into $(git merge-base A\n>> B)..A and $(git merge-base A B)..B, but I wonder if we should be\n>> rewriting it into A..B and B..A instead; that would make it\n>> unnecessary to explain what should happen when there are more than\n>> one merge bases.\n>\n> You know what? I lied.\n\nI knew. So we need an update to the patch.\n\n>> >> Does this merely resemble? Isn't it exactly what a symmetric range is?\n>> >\n>> > No, it is not exactly what a symmetric range is because `range-diff`\n>> > treats both arms of the symmetric range independently, as two distinct\n>> > non-symmetric ranges.\n>>\n>> This however is an end-user documentation, isn't it?\n>\n> Yes, and the end user is talking about _two_ commit ranges in the context\n> of `git range-diff`, and about _one_ commit range in the context of `git\n> log`.\n\nYou are forgetting that \"log A...B\" shows only one half of what a\nsymmetric range means, and hides which side each commit belongs to.\n\n\"git log --left-right A...B\" shows what a symmetric range really is;\nthere are two sides, left and right, and \"git range-diff A...B\" is\na natural way to compare these two ranges.\n\nI've been quite happy with the way how \"git range-diff @{-1}...\"\nshows the comparison of two sides of the symmetric range, given by\n\"git log --oneline --left-right @{-1}...\"\n\n\n"},{"id":"415482","messageId":"xmqqa6sty55k.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"nycvar.QRO.7.76.6.2101270342140.57@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-28T05:57:59Z","receivedAt":"2021-01-28T05:59:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Maybe I am too stuck with the idea of avoiding regular expressions after\n> that StackOverflow incident... Maybe\n>\n> \tstatic regex_t *regex;\n>\n> \tif (strstr(range, \"..\"))\n> \t\treturn 1;\n>\n> \tif (!regex) {\n> \t\tregex = xmalloc(sizeof(*regex));\n> \t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED))\n> \t\t\tBUG(\"could not compile regex\");\n> \t}\n>\n> \treturn !regexec(regex, range, 0, NULL, 0);\n>\n> is more readable, and acceptable in this context?\n\nReadable, yes, acceptable, I don't know, and I am not even sure if I\nwant to be the one to judge to be honest ;-)\n\nHave you tried the approach to feed the thing to setup_revisions()\nand inspect what objects are in revs.pending? \n\nWhen you got a valid range, you should find one or more positive and\none or more negative commits , and the approach won't be fooled by\nstrings like \"HEAD^{/other than A..B/}\".\n\nOr does the revision parsing machinery too eager to \"die\" when we\nfind a syntax error? If so, scratch that idea, but in the longer\nhaul, fixing these die's would also be something we'd want to do to\nmake more parts of libgit.a callable as a proper library.\n\nThanks.\n"},{"id":"415504","messageId":"nycvar.QRO.7.76.6.2101281636400.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqa6sty55k.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-28T15:38:38Z","receivedAt":"2021-01-28T15:43:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 27 Jan 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > Maybe I am too stuck with the idea of avoiding regular expressions after\n> > that StackOverflow incident... Maybe\n> >\n> > \tstatic regex_t *regex;\n> >\n> > \tif (strstr(range, \"..\"))\n> > \t\treturn 1;\n> >\n> > \tif (!regex) {\n> > \t\tregex = xmalloc(sizeof(*regex));\n> > \t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED))\n> > \t\t\tBUG(\"could not compile regex\");\n> > \t}\n> >\n> > \treturn !regexec(regex, range, 0, NULL, 0);\n> >\n> > is more readable, and acceptable in this context?\n>\n> Readable, yes, acceptable, I don't know, and I am not even sure if I\n> want to be the one to judge to be honest ;-)\n>\n> Have you tried the approach to feed the thing to setup_revisions()\n> and inspect what objects are in revs.pending?\n\nI hadn't thought of that.\n\n> When you got a valid range, you should find one or more positive and\n> one or more negative commits , and the approach won't be fooled by\n> strings like \"HEAD^{/other than A..B/}\".\n>\n> Or does the revision parsing machinery too eager to \"die\" when we\n> find a syntax error? If so, scratch that idea, but in the longer\n> haul, fixing these die's would also be something we'd want to do to\n> make more parts of libgit.a callable as a proper library.\n\nIt should probably be libified anyway if it calles `die()` (I don't know\noff the top of my head).\n\nWorse than the `die()` is probably the allocated memory; IIRC there is no\nway to release the memory allocated by `setup_revisions()` unless one\nperforms a revwalk.\n\nWill try to find some time to play with the `setup_revisions()` idea.\n\nCiao,\nDscho\n"},{"id":"416142","messageId":"pull.841.v4.git.1612431093.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v3.git.1611765444.gitgitgadget@gmail.com","subject":"[PATCH v4 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T09:31:29Z","receivedAt":"2021-02-04T09:32:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nChanges since v3:\n\n * The revision machinery is now used directly to validate the commit\n ranges.\n\nChanges since v2:\n\n * Move the helper function from revision.c to range-diff.c and rename it.\n * Use a regex to make it easier to understand what we're trying to match.\n * Fix the documentation that claimed that we used git merge-base internally\n when git range-diff parses ...-style arguments, which is not the case.\n\nChanges since v1:\n\n * In addition to git range-diff, git format-patch --range-diff gets the\n same improvement.\n * The comment talking about ^@ was removed.\n * The parsing was made a bit safer (e.g. catching ! by its own as an\n invalid range).\n\nJohannes Schindelin (3):\n range-diff/format-patch: refactor check for commit range\n range-diff/format-patch: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 12 ++++++++++++\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 22 ++++++++++++++++++++++\n range-diff.h | 8 ++++++++\n t/t3206-range-diff.sh | 8 ++++++++\n 6 files changed, 56 insertions(+), 5 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n\nRange-diff vs v3:\n\n 1: b98fa94b8703 = 1: b98fa94b8703 range-diff/format-patch: refactor check for commit range\n 2: 0880ca587e63 ! 2: 448e6a64fa15 range-diff/format-patch: handle commit ranges other than A..B\n @@ Commit message\n described to specify commit ranges that `range-diff` does not yet\n accept: \"^!\" and \"^-\".\n \n - Let's accept them.\n + Let's accept them, by parsing them via the revision machinery and\n + looking for at least one interesting and one uninteresting revision in\n + the resulting `pending` array.\n \n Signed-off-by: Johannes Schindelin \n \n ## range-diff.c ##\n +@@\n + #include \"pretty.h\"\n + #include \"userdiff.h\"\n + #include \"apply.h\"\n ++#include \"revision.h\"\n + \n + struct patch_util {\n + \t/* For the search for an exact match */\n @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,\n \n int is_range_diff_range(const char *arg)\n {\n -\treturn !!strstr(arg, \"..\");\n -+\tstatic regex_t *regex;\n -+\n -+\tif (strstr(arg, \"..\"))\n -+\t\treturn 1;\n ++\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n ++\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n ++\tint i, positive = 0, negative = 0;\n ++\tstruct rev_info revs;\n +\n -+\t/* match `^!` and `^-` */\n -+\tif (!regex) {\n -+\t\tregex = xmalloc(sizeof(*regex));\n -+\t\tif (regcomp(regex, \"\\\\^(!|-[0-9]*)$\", REG_EXTENDED) < 0)\n -+\t\t\tBUG(\"could not compile range-diff regex\");\n ++\tinit_revisions(&revs, NULL);\n ++\tif (setup_revisions(3, argv, &revs, 0) == 1) {\n ++\t\tfor (i = 0; i < revs.pending.nr; i++)\n ++\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n ++\t\t\t\tnegative++;\n ++\t\t\telse\n ++\t\t\t\tpositive++;\n +\t}\n +\n -+\treturn !regexec(regex, arg, 0, NULL, 0);\n ++\tfree(copy);\n ++\tobject_array_clear(&revs.pending);\n ++\treturn negative > 0 && positive > 0;\n }\n \n ## t/t3206-range-diff.sh ##\n 3: 5ab9321a34ca = 3: 295fdc1cd32c range-diff(docs): explain how to specify commit ranges\n\n-- \ngitgitgadget\n"},{"id":"416143","messageId":"b98fa94b87037b811ea973c1aeb7cfe08d7c1bd6.1612431093.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v4.git.1612431093.gitgitgadget@gmail.com","subject":"[PATCH v4 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T09:31:30Z","receivedAt":"2021-02-04T09:32:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, `git range-diff`\ntests for a literal `..` in each of the two. Likewise, the argument\nprovided via `--range-diff` to `git format-patch` is checked in the same\nmanner.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the check into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 5 +++++\n range-diff.h | 8 ++++++++\n 4 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bd6ff4f9f956..aeece57e86a2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1680,7 +1680,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n-\tint prev_is_range = !!strstr(prev, \"..\");\n+\tint prev_is_range = is_range_diff_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f7446..5b1f6326322f 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -3,6 +3,7 @@\n #include \"parse-options.h\"\n #include \"range-diff.h\"\n #include \"config.h\"\n+#include \"revision.h\"\n \n static const char * const builtin_range_diff_usage[] = {\n N_(\"git range-diff [] .. ..\"),\n@@ -46,12 +47,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!is_range_diff_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!is_range_diff_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\ndiff --git a/range-diff.c b/range-diff.c\nindex b9950f10c8c4..9b93e08e8407 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,\n \n \treturn res;\n }\n+\n+int is_range_diff_range(const char *arg)\n+{\n+\treturn !!strstr(arg, \"..\");\n+}\ndiff --git a/range-diff.h b/range-diff.h\nindex 583ced2e8e74..c17dbc2e75a8 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n \t\t const struct diff_options *diffopt,\n \t\t const struct strvec *other_arg);\n \n+/*\n+ * Determine whether the given argument is usable as a range argument of `git\n+ * range-diff`, e.g. A..B. Note that this only validates the format but does\n+ * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n+ * local repository.\n+ */\n+int is_range_diff_range(const char *arg);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"416144","messageId":"448e6a64fa157990fcc973ce2fe4a9fc2ba1ab32.1612431093.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v4.git.1612431093.gitgitgadget@gmail.com","subject":"[PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T09:31:31Z","receivedAt":"2021-02-04T09:32:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them, by parsing them via the revision machinery and\nlooking for at least one interesting and one uninteresting revision in\nthe resulting `pending` array.\n\nSigned-off-by: Johannes Schindelin \n---\n range-diff.c | 19 ++++++++++++++++++-\n t/t3206-range-diff.sh | 8 ++++++++\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 9b93e08e8407..07e212d5bb8c 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -11,6 +11,7 @@\n #include \"pretty.h\"\n #include \"userdiff.h\"\n #include \"apply.h\"\n+#include \"revision.h\"\n \n struct patch_util {\n \t/* For the search for an exact match */\n@@ -567,5 +568,21 @@ int show_range_diff(const char *range1, const char *range2,\n \n int is_range_diff_range(const char *arg)\n {\n-\treturn !!strstr(arg, \"..\");\n+\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n+\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n+\tint i, positive = 0, negative = 0;\n+\tstruct rev_info revs;\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(3, argv, &revs, 0) == 1) {\n+\t\tfor (i = 0; i < revs.pending.nr; i++)\n+\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n+\t\t\t\tnegative++;\n+\t\t\telse\n+\t\t\t\tpositive++;\n+\t}\n+\n+\tfree(copy);\n+\tobject_array_clear(&revs.pending);\n+\treturn negative > 0 && positive > 0;\n }\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be0312..e217cecac9ed 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"416145","messageId":"295fdc1cd32cffcef145d9dd0ac76d29580fa6de.1612431093.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v4.git.1612431093.gitgitgadget@gmail.com","subject":"[PATCH v4 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T09:31:32Z","receivedAt":"2021-02-04T09:32:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd5..14bffb272a06 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This resembles the symmetric ranges mentioned in\n+ the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n+ equivalent to `.. ..`.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"416163","messageId":"xmqq7dnn7jjz.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"448e6a64fa157990fcc973ce2fe4a9fc2ba1ab32.1612431093.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T18:51:44Z","receivedAt":"2021-02-04T18:53:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> int is_range_diff_range(const char *arg)\n> {\n> -\treturn !!strstr(arg, \"..\");\n> +\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n> +\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n> +\tint i, positive = 0, negative = 0;\n> +\tstruct rev_info revs;\n> +\n> +\tinit_revisions(&revs, NULL);\n> +\tif (setup_revisions(3, argv, &revs, 0) == 1) {\n> +\t\tfor (i = 0; i < revs.pending.nr; i++)\n> +\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n> +\t\t\t\tnegative++;\n> +\t\t\telse\n> +\t\t\t\tpositive++;\n> +\t}\n> +\n> +\tfree(copy);\n> +\tobject_array_clear(&revs.pending);\n> +\treturn negative > 0 && positive > 0;\n> }\n\nOne thing that worries me with this code is that I do not see\nanybody that clears UNINTERESTING bit in the flags. In-core objects\nare singletons, so if a user fed the command two ranges,\n\n\tgit range-diff A..B C..A\n\nand this code first handled \"A..B\", smudging the in-core instance of\nthe commit object A with UNINTERESTING bit, that in-core instance\nwill be reused when the second range argument \"C..A\" is given to\nthis function again.\n\nAt that point, has anybody cleared the UNINTERESTING bit in the\nflags word for the in-core commit A? I do not see it done in this\nfunction, but perhaps I am missing it done in the init/setup\nfunctions (I somehow doubt it, though)?\n\nShoudn't we be calling clear_commit_marks(ALL_REF_FLAGS) on the\ncommits in revs.pending[] array before we clear it? Depending on\nthe shape of \"arg\" that is end-user supplied, we may have walked the\nhistory in handle_dotdot_1() to parse it (e.g. \"A...B\").\n\nAlso we'd want to see what needs to be cleared in revs.cmdline\nthat would have been populated by calls to add_rev_cmdline().\n\nOther than that, I quite like the way the actual code turned out to\nbe.\n\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index 6eb344be0312..e217cecac9ed 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'A^! and A^- (unmodified)' '\n> +\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success 'trivial reordering' '\n> \tgit range-diff --no-color master topic reordered >actual &&\n> \tcat >expect <<-EOF &&\n"},{"id":"416164","messageId":"xmqq35yb7jhn.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"295fdc1cd32cffcef145d9dd0ac76d29580fa6de.1612431093.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T18:53:08Z","receivedAt":"2021-02-04T18:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> There are three forms, depending whether the user specifies one, two or\n> three non-option arguments. We've never actually explained how this\n> works in the manual, so let's explain it.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/git-range-diff.txt | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> index 9701c1e5fdd5..14bffb272a06 100644\n> --- a/Documentation/git-range-diff.txt\n> +++ b/Documentation/git-range-diff.txt\n> @@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the\n> second commit range, with unmatched commits being inserted just after\n> all of their ancestors have been shown.\n> \n> +There are three ways to specify the commit ranges:\n> +\n> +- ` `: Either commit range can be of the form\n> + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> + in linkgit:gitrevisions[7] for more details.\n> +\n> +- `...`. This resembles the symmetric ranges mentioned in\n> + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> + equivalent to `.. ..`.\n\nAs I said before, this _is_ a symmetric range that has commits\nreachable from rev1 but not from rev2 on the left hand side, and\ncommits reachable from rev2 but not from rev1 on the right hand\nside, not just something else that resembles a symmetric range.\n\n> +- ` `: This is equivalent to `..\n> + ..`.\n> \n> OPTIONS\n> -------\n"},{"id":"416165","messageId":"xmqqy2g364s9.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"b98fa94b87037b811ea973c1aeb7cfe08d7c1bd6.1612431093.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T18:56:06Z","receivedAt":"2021-02-04T18:59:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> diff --git a/range-diff.h b/range-diff.h\n> index 583ced2e8e74..c17dbc2e75a8 100644\n> --- a/range-diff.h\n> +++ b/range-diff.h\n> @@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n> \t\t const struct diff_options *diffopt,\n> \t\t const struct strvec *other_arg);\n> \n> +/*\n> + * Determine whether the given argument is usable as a range argument of `git\n> + * range-diff`, e.g. A..B. Note that this only validates the format but does\n> + * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n> + * local repository.\n> + */\n> +int is_range_diff_range(const char *arg);\n\nIf we were to use [v4 2/3], then we do parse it, even though we do\nuse the parse result to reject some valid ranges (like \"a history\nall the way down to root\" in the implementation). I think just\ndropping everything after \"Note that\" is a sufficient fix.\n\n"},{"id":"416166","messageId":"xmqqtuqr64oa.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"448e6a64fa157990fcc973ce2fe4a9fc2ba1ab32.1612431093.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T18:58:29Z","receivedAt":"2021-02-04T19:00:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> +test_expect_success 'A^! and A^- (unmodified)' '\n> +\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nNow we actually parse the single-token range, instead of relying on\n\"does it have dot-dot\" heuristics, we can make sure that we reject\n\n \"HEAD^{/^string with .. in it}\"\n\nas \"not a range with negative and positive ends\".\n\n"},{"id":"416181","messageId":"nycvar.QRO.7.76.6.2102042026490.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqy2g364s9.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-04T19:27:22Z","receivedAt":"2021-02-04T19:30:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Feb 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > diff --git a/range-diff.h b/range-diff.h\n> > index 583ced2e8e74..c17dbc2e75a8 100644\n> > --- a/range-diff.h\n> > +++ b/range-diff.h\n> > @@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n> > \t\t const struct diff_options *diffopt,\n> > \t\t const struct strvec *other_arg);\n> >\n> > +/*\n> > + * Determine whether the given argument is usable as a range argument of `git\n> > + * range-diff`, e.g. A..B. Note that this only validates the format but does\n> > + * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n> > + * local repository.\n> > + */\n> > +int is_range_diff_range(const char *arg);\n>\n> If we were to use [v4 2/3], then we do parse it, even though we do\n> use the parse result to reject some valid ranges (like \"a history\n> all the way down to root\" in the implementation). I think just\n> dropping everything after \"Note that\" is a sufficient fix.\n\nFair. I will add the sentence in 1/3 (because it is still correct there)\nand remove it as part of 2/3.\n\nCiao,\nDscho\n"},{"id":"416210","messageId":"nycvar.QRO.7.76.6.2102042027570.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqq7dnn7jjz.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-04T21:42:17Z","receivedAt":"2021-02-04T21:44:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Feb 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > int is_range_diff_range(const char *arg)\n> > {\n> > -\treturn !!strstr(arg, \"..\");\n> > +\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n> > +\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n> > +\tint i, positive = 0, negative = 0;\n> > +\tstruct rev_info revs;\n> > +\n> > +\tinit_revisions(&revs, NULL);\n> > +\tif (setup_revisions(3, argv, &revs, 0) == 1) {\n> > +\t\tfor (i = 0; i < revs.pending.nr; i++)\n> > +\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n> > +\t\t\t\tnegative++;\n> > +\t\t\telse\n> > +\t\t\t\tpositive++;\n> > +\t}\n> > +\n> > +\tfree(copy);\n> > +\tobject_array_clear(&revs.pending);\n> > +\treturn negative > 0 && positive > 0;\n> > }\n>\n> One thing that worries me with this code is that I do not see\n> anybody that clears UNINTERESTING bit in the flags. In-core objects\n> are singletons, so if a user fed the command two ranges,\n>\n> \tgit range-diff A..B C..A\n>\n> and this code first handled \"A..B\", smudging the in-core instance of\n> the commit object A with UNINTERESTING bit, that in-core instance\n> will be reused when the second range argument \"C..A\" is given to\n> this function again.\n>\n> At that point, has anybody cleared the UNINTERESTING bit in the\n> flags word for the in-core commit A? I do not see it done in this\n> function, but perhaps I am missing it done in the init/setup\n> functions (I somehow doubt it, though)?\n>\n> Shoudn't we be calling clear_commit_marks(ALL_REF_FLAGS) on the\n> commits in revs.pending[] array before we clear it?\n\nYes, we should ;-)\n\n> Depending on the shape of \"arg\" that is end-user supplied, we may have\n> walked the history in handle_dotdot_1() to parse it (e.g. \"A...B\").\n\nThe code in `handle_dotdot_1()` that handles `...` calls\n`get_merge_bases()`, which eventually calls `get_merge_bases_many_0()`\nwith the `cleanup` parameter set to `1`, i.e. it _does_ clean up the\nflags.\n\nOtherwise, `git log A...B` couldn't work, could it? ;-)\n\n> Also we'd want to see what needs to be cleared in revs.cmdline\n> that would have been populated by calls to add_rev_cmdline().\n\nIs this cleared, like, ever? I don't see any code that handles `cmdline`\nthat way. Seems as we're willfully leaking this.\n\nCiao,\nDscho\n\n> Other than that, I quite like the way the actual code turned out to\n> be.\n>\n> > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> > index 6eb344be0312..e217cecac9ed 100755\n> > --- a/t/t3206-range-diff.sh\n> > +++ b/t/t3206-range-diff.sh\n> > @@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '\n> > \ttest_cmp expect actual\n> > '\n> >\n> > +test_expect_success 'A^! and A^- (unmodified)' '\n> > +\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n> > +\tcat >expect <<-EOF &&\n> > +\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n> > +\tEOF\n> > +\ttest_cmp expect actual\n> > +'\n> > +\n> > test_expect_success 'trivial reordering' '\n> > \tgit range-diff --no-color master topic reordered >actual &&\n> > \tcat >expect <<-EOF &&\n>\n"},{"id":"416211","messageId":"nycvar.QRO.7.76.6.2102042256520.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqtuqr64oa.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-04T21:57:06Z","receivedAt":"2021-02-04T21:58:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Feb 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > +test_expect_success 'A^! and A^- (unmodified)' '\n> > +\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n> > +\tcat >expect <<-EOF &&\n> > +\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n> > +\tEOF\n> > +\ttest_cmp expect actual\n> > +'\n>\n> Now we actually parse the single-token range, instead of relying on\n> \"does it have dot-dot\" heuristics, we can make sure that we reject\n>\n> \"HEAD^{/^string with .. in it}\"\n>\n> as \"not a range with negative and positive ends\".\n\nSure, why not.\n\nCiao,\nDscho\n"},{"id":"416212","messageId":"nycvar.QRO.7.76.6.2102042257130.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqq35yb7jhn.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-04T21:58:21Z","receivedAt":"2021-02-04T22:00:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Feb 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > There are three forms, depending whether the user specifies one, two or\n> > three non-option arguments. We've never actually explained how this\n> > works in the manual, so let's explain it.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > Documentation/git-range-diff.txt | 12 ++++++++++++\n> > 1 file changed, 12 insertions(+)\n> >\n> > diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n> > index 9701c1e5fdd5..14bffb272a06 100644\n> > --- a/Documentation/git-range-diff.txt\n> > +++ b/Documentation/git-range-diff.txt\n> > @@ -28,6 +28,18 @@ Finally, the list of matching commits is shown in the order of the\n> > second commit range, with unmatched commits being inserted just after\n> > all of their ancestors have been shown.\n> >\n> > +There are three ways to specify the commit ranges:\n> > +\n> > +- ` `: Either commit range can be of the form\n> > + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n> > + in linkgit:gitrevisions[7] for more details.\n> > +\n> > +- `...`. This resembles the symmetric ranges mentioned in\n> > + the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n> > + equivalent to `.. ..`.\n>\n> As I said before, this _is_ a symmetric range that has commits\n> reachable from rev1 but not from rev2 on the left hand side, and\n> commits reachable from rev2 but not from rev1 on the right hand\n> side, not just something else that resembles a symmetric range.\n\nIf we were talking about one commit range instead of two, I would agree\nwith you.\n\nTo put an end to this pointless discussion, I struck the sentence from the\ndocumentation. It might even make it easier to read for users. So that's a\nwin.\n\nCiao,\nDscho\n\n> > +- ` `: This is equivalent to `..\n> > + ..`.\n> >\n> > OPTIONS\n> > -------\n>\n"},{"id":"416215","messageId":"xmqqpn1f4fq8.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"nycvar.QRO.7.76.6.2102042257130.54@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v4 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T22:42:39Z","receivedAt":"2021-02-04T22:43:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> As I said before, this _is_ a symmetric range that has commits\n>> reachable from rev1 but not from rev2 on the left hand side, and\n>> commits reachable from rev2 but not from rev1 on the right hand\n>> side, not just something else that resembles a symmetric range.\n>\n> If we were talking about one commit range instead of two, I would agree\n> with you.\n\nIt is one symmetric commit range A...B that has two sides, i.e. B..A\non the left side and A..B on the right side.\n"},{"id":"416218","messageId":"pull.841.v5.git.1612481392.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v4.git.1612431093.gitgitgadget@gmail.com","subject":"[PATCH v5 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T23:29:49Z","receivedAt":"2021-02-04T23:30:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nChanges since v4:\n\n * The commit marks are now cleared in is_range_diff_range().\n * A regression test now verifies that HEAD^{/something..or other} isn't\n mistaken for a commit range.\n * The manual page no longer mentions \"symmetric range\", to avoid\n contentious language.\n\nChanges since v3:\n\n * The revision machinery is now used directly to validate the commit\n ranges.\n\nChanges since v2:\n\n * Move the helper function from revision.c to range-diff.c and rename it.\n * Use a regex to make it easier to understand what we're trying to match.\n * Fix the documentation that claimed that we used git merge-base internally\n when git range-diff parses ...-style arguments, which is not the case.\n\nChanges since v1:\n\n * In addition to git range-diff, git format-patch --range-diff gets the\n same improvement.\n * The comment talking about ^@ was removed.\n * The parsing was made a bit safer (e.g. catching ! by its own as an\n invalid range).\n\nJohannes Schindelin (3):\n range-diff/format-patch: refactor check for commit range\n range-diff/format-patch: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 11 +++++++++++\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 27 +++++++++++++++++++++++++++\n range-diff.h | 6 ++++++\n t/t3206-range-diff.sh | 13 +++++++++++++\n 6 files changed, 63 insertions(+), 5 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n\nRange-diff vs v4:\n\n 1: b98fa94b8703 = 1: b98fa94b8703 range-diff/format-patch: refactor check for commit range\n 2: 448e6a64fa15 ! 2: 04b5d75adbc3 range-diff/format-patch: handle commit ranges other than A..B\n @@ Commit message\n looking for at least one interesting and one uninteresting revision in\n the resulting `pending` array.\n \n + This also finally lets us reject arguments that _do_ contain `..` but\n + are not actually ranges, e.g. `HEAD^{/do.. match this}`.\n +\n Signed-off-by: Johannes Schindelin \n \n ## range-diff.c ##\n @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,\n +\tstruct rev_info revs;\n +\n +\tinit_revisions(&revs, NULL);\n -+\tif (setup_revisions(3, argv, &revs, 0) == 1) {\n -+\t\tfor (i = 0; i < revs.pending.nr; i++)\n -+\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n ++\tif (setup_revisions(3, argv, &revs, 0) == 1)\n ++\t\tfor (i = 0; i < revs.pending.nr; i++) {\n ++\t\t\tstruct object *obj = revs.pending.objects[i].item;\n ++\n ++\t\t\tif (obj->flags & UNINTERESTING)\n +\t\t\t\tnegative++;\n +\t\t\telse\n +\t\t\t\tpositive++;\n -+\t}\n ++\t\t\tif (obj->type == OBJ_COMMIT)\n ++\t\t\t\tclear_commit_marks((struct commit *)obj,\n ++\t\t\t\t\t\t ALL_REV_FLAGS);\n ++\t\t}\n +\n +\tfree(copy);\n +\tobject_array_clear(&revs.pending);\n +\treturn negative > 0 && positive > 0;\n }\n \n + ## range-diff.h ##\n +@@ range-diff.h: int show_range_diff(const char *range1, const char *range2,\n + \n + /*\n + * Determine whether the given argument is usable as a range argument of `git\n +- * range-diff`, e.g. A..B. Note that this only validates the format but does\n +- * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n +- * local repository.\n ++ * range-diff`, e.g. A..B.\n + */\n + int is_range_diff_range(const char *arg);\n + \n +\n ## t/t3206-range-diff.sh ##\n @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '\n +\tEOF\n +\ttest_cmp expect actual\n +'\n ++\n ++test_expect_success 'A^{/..} is not mistaken for a range' '\n ++\ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n ++\ttest_i18ngrep \"not a commit rang\" error\n ++'\n +\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n 3: 295fdc1cd32c ! 3: bc5de807735d range-diff(docs): explain how to specify commit ranges\n @@ Documentation/git-range-diff.txt: Finally, the list of matching commits is shown\n + `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n + in linkgit:gitrevisions[7] for more details.\n +\n -+- `...`. This resembles the symmetric ranges mentioned in\n -+ the `SPECIFYING RANGES` section of linkgit:gitrevisions[7], and is\n -+ equivalent to `.. ..`.\n ++- `...`. This is equivalent to\n ++ `.. ..`.\n +\n +- ` `: This is equivalent to `..\n + ..`.\n\n-- \ngitgitgadget\n"},{"id":"416219","messageId":"bc5de807735d770ca2b785104b9d7667f892825f.1612481392.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v5.git.1612481392.gitgitgadget@gmail.com","subject":"[PATCH v5 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T23:29:52Z","receivedAt":"2021-02-04T23:30:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd5..a968d5237dae 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,17 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This is equivalent to\n+ `.. ..`.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"416220","messageId":"b98fa94b87037b811ea973c1aeb7cfe08d7c1bd6.1612481392.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v5.git.1612481392.gitgitgadget@gmail.com","subject":"[PATCH v5 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T23:29:50Z","receivedAt":"2021-02-04T23:31:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, `git range-diff`\ntests for a literal `..` in each of the two. Likewise, the argument\nprovided via `--range-diff` to `git format-patch` is checked in the same\nmanner.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the check into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 5 +++++\n range-diff.h | 8 ++++++++\n 4 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bd6ff4f9f956..aeece57e86a2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1680,7 +1680,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n-\tint prev_is_range = !!strstr(prev, \"..\");\n+\tint prev_is_range = is_range_diff_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f7446..5b1f6326322f 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -3,6 +3,7 @@\n #include \"parse-options.h\"\n #include \"range-diff.h\"\n #include \"config.h\"\n+#include \"revision.h\"\n \n static const char * const builtin_range_diff_usage[] = {\n N_(\"git range-diff [] .. ..\"),\n@@ -46,12 +47,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!is_range_diff_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!is_range_diff_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\ndiff --git a/range-diff.c b/range-diff.c\nindex b9950f10c8c4..9b93e08e8407 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,\n \n \treturn res;\n }\n+\n+int is_range_diff_range(const char *arg)\n+{\n+\treturn !!strstr(arg, \"..\");\n+}\ndiff --git a/range-diff.h b/range-diff.h\nindex 583ced2e8e74..c17dbc2e75a8 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n \t\t const struct diff_options *diffopt,\n \t\t const struct strvec *other_arg);\n \n+/*\n+ * Determine whether the given argument is usable as a range argument of `git\n+ * range-diff`, e.g. A..B. Note that this only validates the format but does\n+ * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n+ * local repository.\n+ */\n+int is_range_diff_range(const char *arg);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"416221","messageId":"04b5d75adbc3d80e9f9cf9cd380294949e7c68e8.1612481392.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v5.git.1612481392.gitgitgadget@gmail.com","subject":"[PATCH v5 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T23:29:51Z","receivedAt":"2021-02-04T23:31:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them, by parsing them via the revision machinery and\nlooking for at least one interesting and one uninteresting revision in\nthe resulting `pending` array.\n\nThis also finally lets us reject arguments that _do_ contain `..` but\nare not actually ranges, e.g. `HEAD^{/do.. match this}`.\n\nSigned-off-by: Johannes Schindelin \n---\n range-diff.c | 24 +++++++++++++++++++++++-\n range-diff.h | 4 +---\n t/t3206-range-diff.sh | 13 +++++++++++++\n 3 files changed, 37 insertions(+), 4 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 9b93e08e8407..c307bca9de23 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -11,6 +11,7 @@\n #include \"pretty.h\"\n #include \"userdiff.h\"\n #include \"apply.h\"\n+#include \"revision.h\"\n \n struct patch_util {\n \t/* For the search for an exact match */\n@@ -567,5 +568,26 @@ int show_range_diff(const char *range1, const char *range2,\n \n int is_range_diff_range(const char *arg)\n {\n-\treturn !!strstr(arg, \"..\");\n+\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n+\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n+\tint i, positive = 0, negative = 0;\n+\tstruct rev_info revs;\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(3, argv, &revs, 0) == 1)\n+\t\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\t\tstruct object *obj = revs.pending.objects[i].item;\n+\n+\t\t\tif (obj->flags & UNINTERESTING)\n+\t\t\t\tnegative++;\n+\t\t\telse\n+\t\t\t\tpositive++;\n+\t\t\tif (obj->type == OBJ_COMMIT)\n+\t\t\t\tclear_commit_marks((struct commit *)obj,\n+\t\t\t\t\t\t ALL_REV_FLAGS);\n+\t\t}\n+\n+\tfree(copy);\n+\tobject_array_clear(&revs.pending);\n+\treturn negative > 0 && positive > 0;\n }\ndiff --git a/range-diff.h b/range-diff.h\nindex c17dbc2e75a8..4abd70c40fed 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -18,9 +18,7 @@ int show_range_diff(const char *range1, const char *range2,\n \n /*\n * Determine whether the given argument is usable as a range argument of `git\n- * range-diff`, e.g. A..B. Note that this only validates the format but does\n- * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n- * local repository.\n+ * range-diff`, e.g. A..B.\n */\n int is_range_diff_range(const char *arg);\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be0312..45f21ee215d7 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,19 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'A^{/..} is not mistaken for a range' '\n+\ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n+\ttest_i18ngrep \"not a commit rang\" error\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"416228","messageId":"xmqqmtwj2ugg.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"04b5d75adbc3d80e9f9cf9cd380294949e7c68e8.1612481392.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-05T01:07:27Z","receivedAt":"2021-02-05T01:08:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> +\tinit_revisions(&revs, NULL);\n> +\tif (setup_revisions(3, argv, &revs, 0) == 1)\n> +\t\tfor (i = 0; i < revs.pending.nr; i++) {\n> +\t\t\tstruct object *obj = revs.pending.objects[i].item;\n> +\n> +\t\t\tif (obj->flags & UNINTERESTING)\n> +\t\t\t\tnegative++;\n> +\t\t\telse\n> +\t\t\t\tpositive++;\n> +\t\t\tif (obj->type == OBJ_COMMIT)\n> +\t\t\t\tclear_commit_marks((struct commit *)obj,\n> +\t\t\t\t\t\t ALL_REV_FLAGS);\n\nAh, I didn't think of adding this inside the same loop. As long as\nthe pending objects are independent, this should work, but it is\nunsafe, I think. Imagine what happens if revs.pending[0].item can\nreach the object revs.pending[1].item? By the time the loop wants\nto inspect the latter, its flags may have been cleared already\nbecause we clear flags in objects that are reachable from the\nformer.\n\nLet's do this as a post-processing for safety (or correctness).\n\n\tint negpos = 0;\n\n\tif (setup_revisions(3, argv, &revs, 0) != 1)\n\t\tgoto cleanup;\n\tfor (i = 0; i < revs.pending.nr && negpos != 3; i++) {\n\t\tstruct object *obj = revs.pending.objects[i].item;\n\t\tnegpos |= (obj->flags & UNINTERESTING) ? 01 : 02;\n\t}\n\n\tfor (i = 0; i < revs.pending.nr; i++) {\n\t\tstruct object *obj = revs.pending.objects[i].item;\n if (obj->type == OBJ_COMMIT)\n\t\t\tclear_commit_marks((struct commit *)obj, ALL_REV_FLAGS);\n\t}\n\n cleanup:\n> +\tfree(copy);\n> +\tobject_array_clear(&revs.pending);\n> +\treturn negative > 0 && positive > 0;\n> }\n\nor something like that.\n\n> +test_expect_success 'A^{/..} is not mistaken for a range' '\n> +\ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n> +\ttest_i18ngrep \"not a commit rang\" error\n> +'\n\ns/rang/range/, I would think.\n\nThanks.\n"},{"id":"416229","messageId":"xmqqim772tal.fsf@gitster.c.googlers.com","threadId":"55025","inReplyTo":"xmqqmtwj2ugg.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-05T01:32:34Z","receivedAt":"2021-02-05T01:33:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ah, I didn't think of adding this inside the same loop. As long as\n> the pending objects are independent, this should work, but it is\n> unsafe, I think.\n\nHere is what I added on top of the 3-patch series for tonight's\npushout.\n\n * No need to count positive/negative; just seeing both exists is\n sufficient and there is no need to examine any later elements, if\n any.\n\n * Clearing commit marks needs to be done after we have seen enough,\n or it can clear the stuff we would want to inspect before we\n have a chance to. Do it in a separate post-cleaning loop.\n\n * Dedent by judicious use of 'goto'.\n\n * The last parameter to setup_revisions() is a pointer, so spell it\n NULL not 0.\n\n * \"rang\" -> \"range\" typofix (it might be even better to look for\n \"range:\")\n\n----- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 -----\nSubject: [PATCH] fixup! range-diff/format-patch: handle commit ranges other than A..B\n\n range-diff.c | 30 +++++++++++++++++-------------\n t/t3206-range-diff.sh | 2 +-\n 2 files changed, 18 insertions(+), 14 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex c307bca9de..7a38dc8715 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -570,24 +570,28 @@ int is_range_diff_range(const char *arg)\n {\n \tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n \tconst char *argv[] = { \"\", copy, \"--\", NULL };\n-\tint i, positive = 0, negative = 0;\n+\tint i;\n \tstruct rev_info revs;\n+\tunsigned npmask = 0;\n \n \tinit_revisions(&revs, NULL);\n-\tif (setup_revisions(3, argv, &revs, 0) == 1)\n-\t\tfor (i = 0; i < revs.pending.nr; i++) {\n-\t\t\tstruct object *obj = revs.pending.objects[i].item;\n+\tif (setup_revisions(3, argv, &revs, NULL) != 1)\n+\t\tgoto cleanup;\n \n-\t\t\tif (obj->flags & UNINTERESTING)\n-\t\t\t\tnegative++;\n-\t\t\telse\n-\t\t\t\tpositive++;\n-\t\t\tif (obj->type == OBJ_COMMIT)\n-\t\t\t\tclear_commit_marks((struct commit *)obj,\n-\t\t\t\t\t\t ALL_REV_FLAGS);\n-\t\t}\n+\tfor (i = 0; i < revs.pending.nr && npmask != 3; i++) {\n+\t\tstruct object *obj = revs.pending.objects[i].item;\n+\n+\t\tnpmask |= (obj->flags & UNINTERESTING) ? 01 : 02;\n+\t}\n+\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object *obj = revs.pending.objects[i].item;\n+\t\tif (obj->type == OBJ_COMMIT)\n+\t\t\tclear_commit_marks((struct commit *)obj, ALL_REV_FLAGS);\n+\t}\n \n+cleanup:\n \tfree(copy);\n \tobject_array_clear(&revs.pending);\n-\treturn negative > 0 && positive > 0;\n+\treturn npmask == 3;\n }\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 45f21ee215..2b518378d4 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -160,7 +160,7 @@ test_expect_success 'A^! and A^- (unmodified)' '\n \n test_expect_success 'A^{/..} is not mistaken for a range' '\n \ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n-\ttest_i18ngrep \"not a commit rang\" error\n+\ttest_i18ngrep \"not a commit range\" error\n '\n \n test_expect_success 'trivial reordering' '\n-- \n2.30.0-601-g9b6178ed87\n\n"},{"id":"416262","messageId":"f8e6a1ad9d3df20383edbb30592461974f439db8.1612536290.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v6.git.1612536290.gitgitgadget@gmail.com","subject":"[PATCH v6 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:44:48Z","receivedAt":"2021-02-05T16:35:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn the `SPECIFYING RANGES` section of gitrevisions[7], two ways are\ndescribed to specify commit ranges that `range-diff` does not yet\naccept: \"^!\" and \"^-\".\n\nLet's accept them, by parsing them via the revision machinery and\nlooking for at least one interesting and one uninteresting revision in\nthe resulting `pending` array.\n\nThis also finally lets us reject arguments that _do_ contain `..` but\nare not actually ranges, e.g. `HEAD^{/do.. match this}`.\n\nSigned-off-by: Johannes Schindelin \n---\n range-diff.c | 26 +++++++++++++++++++++++++-\n range-diff.h | 4 +---\n t/t3206-range-diff.sh | 13 +++++++++++++\n 3 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 9b93e08e8407..a88612cb8923 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -11,6 +11,7 @@\n #include \"pretty.h\"\n #include \"userdiff.h\"\n #include \"apply.h\"\n+#include \"revision.h\"\n \n struct patch_util {\n \t/* For the search for an exact match */\n@@ -567,5 +568,28 @@ int show_range_diff(const char *range1, const char *range2,\n \n int is_range_diff_range(const char *arg)\n {\n-\treturn !!strstr(arg, \"..\");\n+\tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n+\tconst char *argv[] = { \"\", copy, \"--\", NULL };\n+\tint i, positive = 0, negative = 0;\n+\tstruct rev_info revs;\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(3, argv, &revs, NULL) == 1) {\n+\t\tfor (i = 0; i < revs.pending.nr; i++)\n+\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n+\t\t\t\tnegative++;\n+\t\t\telse\n+\t\t\t\tpositive++;\n+\t\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\t\tstruct object *obj = revs.pending.objects[i].item;\n+\n+\t\t\tif (obj->type == OBJ_COMMIT)\n+\t\t\t\tclear_commit_marks((struct commit *)obj,\n+\t\t\t\t\t\t ALL_REV_FLAGS);\n+\t\t}\n+\t}\n+\n+\tfree(copy);\n+\tobject_array_clear(&revs.pending);\n+\treturn negative > 0 && positive > 0;\n }\ndiff --git a/range-diff.h b/range-diff.h\nindex c17dbc2e75a8..4abd70c40fed 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -18,9 +18,7 @@ int show_range_diff(const char *range1, const char *range2,\n \n /*\n * Determine whether the given argument is usable as a range argument of `git\n- * range-diff`, e.g. A..B. Note that this only validates the format but does\n- * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n- * local repository.\n+ * range-diff`, e.g. A..B.\n */\n int is_range_diff_range(const char *arg);\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 6eb344be0312..2b518378d4a0 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -150,6 +150,19 @@ test_expect_success 'simple A B C (unmodified)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'A^! and A^- (unmodified)' '\n+\tgit range-diff --no-color topic^! unmodified^-1 >actual &&\n+\tcat >expect <<-EOF &&\n+\t1: $(test_oid t4) = 1: $(test_oid u4) s/12/B/\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'A^{/..} is not mistaken for a range' '\n+\ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n+\ttest_i18ngrep \"not a commit range\" error\n+'\n+\n test_expect_success 'trivial reordering' '\n \tgit range-diff --no-color master topic reordered >actual &&\n \tcat >expect <<-EOF &&\n-- \ngitgitgadget\n\n"},{"id":"416286","messageId":"08c5f8732747de138c6a98322beca95e919ba11d.1612536290.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v6.git.1612536290.gitgitgadget@gmail.com","subject":"[PATCH v6 3/3] range-diff(docs): explain how to specify commit ranges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:44:49Z","receivedAt":"2021-02-05T20:42:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThere are three forms, depending whether the user specifies one, two or\nthree non-option arguments. We've never actually explained how this\nworks in the manual, so let's explain it.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-range-diff.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 9701c1e5fdd5..a968d5237dae 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -28,6 +28,17 @@ Finally, the list of matching commits is shown in the order of the\n second commit range, with unmatched commits being inserted just after\n all of their ancestors have been shown.\n \n+There are three ways to specify the commit ranges:\n+\n+- ` `: Either commit range can be of the form\n+ `..`, `^!` or `^-`. See `SPECIFYING RANGES`\n+ in linkgit:gitrevisions[7] for more details.\n+\n+- `...`. This is equivalent to\n+ `.. ..`.\n+\n+- ` `: This is equivalent to `..\n+ ..`.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n"},{"id":"416297","messageId":"pull.841.v6.git.1612536290.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v5.git.1612481392.gitgitgadget@gmail.com","subject":"[PATCH v6 0/3] Range diff with ranges lacking dotdot","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:44:46Z","receivedAt":"2021-02-05T21:59:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In\nhttps://lore.kernel.org/git/20200306091933.mx2jmurmdnsjua4b@pengutronix.de/,\nit was reported that git range-diff does not handle commit ranges like\nrev^!. This patch series fixes that.\n\nChanges since v5:\n\n * The commit marks are now cleared in a separate loop.\n * The regression test no longer looks only for \"rang\" but for \"range\" in\n the error message.\n * We now pass NULL as opt parameter to setup_revisions(), not 0.\n\nChanges since v4:\n\n * The commit marks are now cleared in is_range_diff_range().\n * A regression test now verifies that HEAD^{/something..or other} isn't\n mistaken for a commit range.\n * The manual page no longer mentions \"symmetric range\", to avoid\n contentious language.\n\nChanges since v3:\n\n * The revision machinery is now used directly to validate the commit\n ranges.\n\nChanges since v2:\n\n * Move the helper function from revision.c to range-diff.c and rename it.\n * Use a regex to make it easier to understand what we're trying to match.\n * Fix the documentation that claimed that we used git merge-base internally\n when git range-diff parses ...-style arguments, which is not the case.\n\nChanges since v1:\n\n * In addition to git range-diff, git format-patch --range-diff gets the\n same improvement.\n * The comment talking about ^@ was removed.\n * The parsing was made a bit safer (e.g. catching ! by its own as an\n invalid range).\n\nJohannes Schindelin (3):\n range-diff/format-patch: refactor check for commit range\n range-diff/format-patch: handle commit ranges other than A..B\n range-diff(docs): explain how to specify commit ranges\n\n Documentation/git-range-diff.txt | 11 +++++++++++\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 29 +++++++++++++++++++++++++++++\n range-diff.h | 6 ++++++\n t/t3206-range-diff.sh | 13 +++++++++++++\n 6 files changed, 65 insertions(+), 5 deletions(-)\n\n\nbase-commit: 71ca53e8125e36efbda17293c50027d31681a41f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-841%2Fdscho%2Frange-diff-with-ranges-lacking-dotdot-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-841/dscho/range-diff-with-ranges-lacking-dotdot-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/841\n\nRange-diff vs v5:\n\n 1: b98fa94b8703 = 1: b98fa94b8703 range-diff/format-patch: refactor check for commit range\n 2: 04b5d75adbc3 ! 2: f8e6a1ad9d3d range-diff/format-patch: handle commit ranges other than A..B\n @@ range-diff.c: int show_range_diff(const char *range1, const char *range2,\n +\tstruct rev_info revs;\n +\n +\tinit_revisions(&revs, NULL);\n -+\tif (setup_revisions(3, argv, &revs, 0) == 1)\n -+\t\tfor (i = 0; i < revs.pending.nr; i++) {\n -+\t\t\tstruct object *obj = revs.pending.objects[i].item;\n -+\n -+\t\t\tif (obj->flags & UNINTERESTING)\n ++\tif (setup_revisions(3, argv, &revs, NULL) == 1) {\n ++\t\tfor (i = 0; i < revs.pending.nr; i++)\n ++\t\t\tif (revs.pending.objects[i].item->flags & UNINTERESTING)\n +\t\t\t\tnegative++;\n +\t\t\telse\n +\t\t\t\tpositive++;\n ++\t\tfor (i = 0; i < revs.pending.nr; i++) {\n ++\t\t\tstruct object *obj = revs.pending.objects[i].item;\n ++\n +\t\t\tif (obj->type == OBJ_COMMIT)\n +\t\t\t\tclear_commit_marks((struct commit *)obj,\n +\t\t\t\t\t\t ALL_REV_FLAGS);\n +\t\t}\n ++\t}\n +\n +\tfree(copy);\n +\tobject_array_clear(&revs.pending);\n @@ t/t3206-range-diff.sh: test_expect_success 'simple A B C (unmodified)' '\n +\n +test_expect_success 'A^{/..} is not mistaken for a range' '\n +\ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n -+\ttest_i18ngrep \"not a commit rang\" error\n ++\ttest_i18ngrep \"not a commit range\" error\n +'\n +\n test_expect_success 'trivial reordering' '\n 3: bc5de807735d = 3: 08c5f8732747 range-diff(docs): explain how to specify commit ranges\n\n-- \ngitgitgadget\n"},{"id":"416324","messageId":"b98fa94b87037b811ea973c1aeb7cfe08d7c1bd6.1612536290.git.gitgitgadget@gmail.com","threadId":"55025","inReplyTo":"pull.841.v6.git.1612536290.gitgitgadget@gmail.com","subject":"[PATCH v6 1/3] range-diff/format-patch: refactor check for commit range","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:44:47Z","receivedAt":"2021-02-05T22:31:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nCurrently, when called with exactly two arguments, `git range-diff`\ntests for a literal `..` in each of the two. Likewise, the argument\nprovided via `--range-diff` to `git format-patch` is checked in the same\nmanner.\n\nHowever, `^!` is a perfectly valid commit range, equivalent to\n`^..` according to the `SPECIFYING RANGES` section of\ngitrevisions[7].\n\nIn preparation for allowing more sophisticated ways to specify commit\nranges, let's refactor the check into its own function.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/log.c | 2 +-\n builtin/range-diff.c | 9 +++++----\n range-diff.c | 5 +++++\n range-diff.h | 8 ++++++++\n 4 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bd6ff4f9f956..aeece57e86a2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1680,7 +1680,7 @@ static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n-\tint prev_is_range = !!strstr(prev, \"..\");\n+\tint prev_is_range = is_range_diff_range(prev);\n \n \tif (prev_is_range)\n \t\tstrbuf_addstr(r1, prev);\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 24c4162f7446..5b1f6326322f 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -3,6 +3,7 @@\n #include \"parse-options.h\"\n #include \"range-diff.h\"\n #include \"config.h\"\n+#include \"revision.h\"\n \n static const char * const builtin_range_diff_usage[] = {\n N_(\"git range-diff [] .. ..\"),\n@@ -46,12 +47,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n-\t\tif (!strstr(argv[0], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[0]);\n+\t\tif (!is_range_diff_range(argv[0]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[0]);\n \t\tstrbuf_addstr(&range1, argv[0]);\n \n-\t\tif (!strstr(argv[1], \"..\"))\n-\t\t\tdie(_(\"no .. in range: '%s'\"), argv[1]);\n+\t\tif (!is_range_diff_range(argv[1]))\n+\t\t\tdie(_(\"not a commit range: '%s'\"), argv[1]);\n \t\tstrbuf_addstr(&range2, argv[1]);\n \t} else if (argc == 3) {\n \t\tstrbuf_addf(&range1, \"%s..%s\", argv[0], argv[1]);\ndiff --git a/range-diff.c b/range-diff.c\nindex b9950f10c8c4..9b93e08e8407 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -564,3 +564,8 @@ int show_range_diff(const char *range1, const char *range2,\n \n \treturn res;\n }\n+\n+int is_range_diff_range(const char *arg)\n+{\n+\treturn !!strstr(arg, \"..\");\n+}\ndiff --git a/range-diff.h b/range-diff.h\nindex 583ced2e8e74..c17dbc2e75a8 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -16,4 +16,12 @@ int show_range_diff(const char *range1, const char *range2,\n \t\t const struct diff_options *diffopt,\n \t\t const struct strvec *other_arg);\n \n+/*\n+ * Determine whether the given argument is usable as a range argument of `git\n+ * range-diff`, e.g. A..B. Note that this only validates the format but does\n+ * _not_ parse it, i.e. it does _not_ look up the specified commits in the\n+ * local repository.\n+ */\n+int is_range_diff_range(const char *arg);\n+\n #endif\n-- \ngitgitgadget\n\n"},{"id":"416325","messageId":"nycvar.QRO.7.76.6.2102051504080.54@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqim772tal.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-05T14:09:04Z","receivedAt":"2021-02-05T22:31:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 4 Feb 2021, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> > Ah, I didn't think of adding this inside the same loop. As long as\n> > the pending objects are independent, this should work, but it is\n> > unsafe, I think.\n>\n> Here is what I added on top of the 3-patch series for tonight's\n> pushout.\n>\n> * No need to count positive/negative; just seeing both exists is\n> sufficient and there is no need to examine any later elements, if\n> any.\n\nYour code is substantially harder to read. I really dislike that `npmask`\nidea.\n\n> * Clearing commit marks needs to be done after we have seen enough,\n> or it can clear the stuff we would want to inspect before we\n> have a chance to. Do it in a separate post-cleaning loop.\n\nI implemented that.\n\n> * Dedent by judicious use of 'goto'.\n\nNot necessary: the code is short and narrow enough.\n\n> * The last parameter to setup_revisions() is a pointer, so spell it\n> NULL not 0.\n\nI had missed that in your review. Fixed, too.\n\n> * \"rang\" -> \"range\" typofix (it might be even better to look for\n> \"range:\")\n\nTechnically, it is not necessary, as we're only verifying that the\nintended error message is shown, and that \"e\" does not make any\ndifference.\n\nBut it _was_ a typo, so I fixed it, too.\n\nNext iteration is coming soon,\nDscho\n\n>\n> ----- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 -----\n> Subject: [PATCH] fixup! range-diff/format-patch: handle commit ranges other than A..B\n>\n> range-diff.c | 30 +++++++++++++++++-------------\n> t/t3206-range-diff.sh | 2 +-\n> 2 files changed, 18 insertions(+), 14 deletions(-)\n>\n> diff --git a/range-diff.c b/range-diff.c\n> index c307bca9de..7a38dc8715 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -570,24 +570,28 @@ int is_range_diff_range(const char *arg)\n> {\n> \tchar *copy = xstrdup(arg); /* setup_revisions() modifies it */\n> \tconst char *argv[] = { \"\", copy, \"--\", NULL };\n> -\tint i, positive = 0, negative = 0;\n> +\tint i;\n> \tstruct rev_info revs;\n> +\tunsigned npmask = 0;\n>\n> \tinit_revisions(&revs, NULL);\n> -\tif (setup_revisions(3, argv, &revs, 0) == 1)\n> -\t\tfor (i = 0; i < revs.pending.nr; i++) {\n> -\t\t\tstruct object *obj = revs.pending.objects[i].item;\n> +\tif (setup_revisions(3, argv, &revs, NULL) != 1)\n> +\t\tgoto cleanup;\n>\n> -\t\t\tif (obj->flags & UNINTERESTING)\n> -\t\t\t\tnegative++;\n> -\t\t\telse\n> -\t\t\t\tpositive++;\n> -\t\t\tif (obj->type == OBJ_COMMIT)\n> -\t\t\t\tclear_commit_marks((struct commit *)obj,\n> -\t\t\t\t\t\t ALL_REV_FLAGS);\n> -\t\t}\n> +\tfor (i = 0; i < revs.pending.nr && npmask != 3; i++) {\n> +\t\tstruct object *obj = revs.pending.objects[i].item;\n> +\n> +\t\tnpmask |= (obj->flags & UNINTERESTING) ? 01 : 02;\n> +\t}\n> +\n> +\tfor (i = 0; i < revs.pending.nr; i++) {\n> +\t\tstruct object *obj = revs.pending.objects[i].item;\n> +\t\tif (obj->type == OBJ_COMMIT)\n> +\t\t\tclear_commit_marks((struct commit *)obj, ALL_REV_FLAGS);\n> +\t}\n>\n> +cleanup:\n> \tfree(copy);\n> \tobject_array_clear(&revs.pending);\n> -\treturn negative > 0 && positive > 0;\n> +\treturn npmask == 3;\n> }\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index 45f21ee215..2b518378d4 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -160,7 +160,7 @@ test_expect_success 'A^! and A^- (unmodified)' '\n>\n> test_expect_success 'A^{/..} is not mistaken for a range' '\n> \ttest_must_fail git range-diff topic^.. topic^{/..} 2>error &&\n> -\ttest_i18ngrep \"not a commit rang\" error\n> +\ttest_i18ngrep \"not a commit range\" error\n> '\n>\n> test_expect_success 'trivial reordering' '\n> --\n> 2.30.0-601-g9b6178ed87\n>\n>\n"},{"id":"416331","messageId":"nycvar.QRO.7.76.6.2102060137440.29765@tvgsbejvaqbjf.bet","threadId":"55025","inReplyTo":"xmqqim7gx41d.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-06T00:39:41Z","receivedAt":"2021-02-06T03:19:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 28 Jan 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > Worse than the `die()` is probably the allocated memory; IIRC there is no\n> > way to release the memory allocated by `setup_revisions()` unless one\n> > performs a revwalk.\n>\n> Hmph, do you mean the singleton instances of \"struct object\" and\n> those specific types that contain it?\n\nI am concerned when I see what `add_rev_cmdline()` does, for example.\n\nCiao,\nDscho\n"}]}