{"thread":{"id":"65511","subject":"[PATCH] revision.c: implement --reverse=before for walks","startedAt":"2026-04-18T16:56:59Z","lastAt":"2026-06-02T17:50:49Z","messageCount":63,"participants":["Mirko Faina","Tian Yuchen","Ben Knoble","D. Ben Knoble","Jeff King","Junio C Hamano","Johannes Sixt","Chris Torek","Jean-Noël AVILA"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"541864","messageId":"20260418164736.2367523-2-mroik@delayed.space","threadId":"65511","inReplyTo":null,"subject":"[PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-18T16:47:35Z","receivedAt":"2026-04-18T16:56:59Z","isPatch":true,"body":"In a revision walk `--reverse` can only be applied after any commit\nlimiting option. This makes getting a limited amount of commits from the\ntail impossible. E.g.\n\n git log --reverse --max-count=3\n\nSome would expect this to give back the first 3 commits of the project.\nInstead it returns the last 3 but in reversed order.\n\nTeach `get_revision()` to accpet an argument `(after|before)` from the\nCLI, and apply the reversal before or after the commit limiting options\nbased on this argument. If no argument is provided default to the\ncurrent behaviour, applying `--reverse` after the commit limiting\noptions.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 6 ++--\n revision.c | 42 ++++++++++++++++++++++---\n revision.h | 7 ++++-\n t/t4202-log.sh | 49 +++++++++++++++++++++++++++++\n 4 files changed, 97 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..eed1813a92 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -914,10 +914,12 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n avoid showing the commits from two parallel development track mixed\n together.\n \n-`--reverse`::\n+`--reverse[=(after|before)]`::\n \tOutput the commits chosen to be shown (see 'Commit Limiting'\n \tsection above) in reverse order. Cannot be combined with\n-\t`--walk-reflogs`.\n+\t`--walk-reflogs`. `when` can either be `after` or `before`, if\n+\tomitted it defaults to `after`. If `before` is chosen,\n+\t`--reverse` will be applied before any commit limiting options.\n endif::git-shortlog[]\n \n ifndef::git-shortlog[]\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..8338ea7448 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2685,8 +2685,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\telse\n \t\t\tgit_log_output_encoding = xstrdup(\"\");\n \t\treturn argcount;\n-\t} else if (!strcmp(arg, \"--reverse\")) {\n-\t\trevs->reverse ^= 1;\n+\t} else if (starts_with(arg, \"--reverse\")) {\n+\t\tif (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n+\t\t\tif (argc < 2) {\n+\t\t\t\trevs->reverse = 1;\n+\t\t\t\treturn 1;\n+\t\t\t} else {\n+\t\t\t\toptarg = argv[1];\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!strcmp(optarg, \"after\")) {\n+\t\t\trevs->reverse = 1;\n+\t\t} else if (!strcmp(optarg, \"before\")) {\n+\t\t\trevs->reverse = 2;\n+\t\t} else {\n+\t\t\trevs->reverse = 1;\n+\t\t\treturn 1;\n+\t\t}\n+\n+\t\treturn optarg == argv[1] ? 2 : 1;\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n@@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tint max_count = revs->max_count;\n+\n+\tif (revs->reverse && !revs->reverse_output_stage) {\n+\t\tif (revs->reverse == 3) {\n+\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n+\t\t\trevs->reverse = 1;\n+\t\t}\n+\n+\t\tif (revs->reverse == 2)\n+\t\t\trevs->max_count = -1;\n \n-\tif (revs->reverse) {\n \t\treversed = NULL;\n \t\twhile ((c = get_revision_internal(revs)))\n \t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n-\t\trevs->reverse = 0;\n \t\trevs->reverse_output_stage = 1;\n+\n+\t\tif (revs->reverse == 2)\n+\t\t\trevs->max_count = max_count;\n \t}\n \n \tif (revs->reverse_output_stage) {\n+\t\tif (revs->reverse == 2 && revs->max_count == 0)\n+\t\t\treturn NULL;\n+\n \t\tc = pop_commit(&revs->commits);\n+\t\tif (revs->reverse == 2)\n+\t\t\trevs->max_count--;\n \t\tif (revs->track_linear)\n \t\t\trevs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n \t\treturn c;\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..5b23343f17 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -196,7 +196,12 @@ struct rev_info {\n \t\t\trewrite_parents:1,\n \t\t\tprint_parents:1,\n \t\t\tshow_decorations:1,\n-\t\t\treverse:1,\n+\t\t\t/*\n+\t\t\t * 0 no reverse\n+\t\t\t * 1 after\n+\t\t\t * 2 before\n+\t\t\t */\n+\t\t\treverse:2,\n \t\t\treverse_output_stage:1,\n \t\t\tcherry_pick:1,\n \t\t\tcherry_mark:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..21e9a61994 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,55 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+cat >expect <<-\\EOF\n+c3f451c Merge tag 'reach'\n+046b221 to remove\n+EOF\n+\n+test_expect_success 'log --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse after --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse after --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse=after --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse=after --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+3a2fdcb initial\n+f7dab8e second\n+EOF\n+\n+test_expect_success 'log --reverse before --oneline --max-count=2' '\n+\ttest_when_finished rm actual &&\n+\tgit log --reverse before --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse=before --oneline --max-count=2' '\n+\ttest_when_finished rm actual &&\n+\tgit log --reverse=before --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n\nbase-commit: e8955061076952cc5eab0300424fc48b601fe12d\n-- \n2.54.0.rc2.9.ge895506107\n\n"},{"id":"541866","messageId":"fbea5f1c-946b-400e-a9a2-2c6d7b088d46@malon.dev","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-04-18T18:20:41Z","receivedAt":"2026-04-18T18:20:56Z","isPatch":true,"body":"Hi Mirco,\n\nOn 4/19/26 00:47, Mirko Faina wrote:\n> In a revision walk `--reverse` can only be applied after any commit\n> limiting option. This makes getting a limited amount of commits from the\n> tail impossible. E.g.\n> \n> git log --reverse --max-count=3\n> \n> Some would expect this to give back the first 3 commits of the project.\n> Instead it returns the last 3 but in reversed order.\n> \n> Teach `get_revision()` to accpet an argument `(after|before)` from the\n> CLI, and apply the reversal before or after the commit limiting options\n> based on this argument. If no argument is provided default to the\n> current behaviour, applying `--reverse` after the commit limiting\n> options.\n\nReasoning looks good to me.\n\nNit: I think we could gather more feedback on the naming. If I were a \nuser unfamiliar with how Git works, the most natural and intuitive \noperation for me would be: \"Show me the three *oldest* commits\" (to be \nmore precise, *oldest* in the sense of topological order, rather than \nthe commit date or author date order. It’s really annoying. ), rather \nthan \"reverse the entire list and then select the three most recent \nones\". I think the confusion arises because users do not (and should \nnot) know that '--max-count' only returns the most recent commits; \nconsequently, they might wonder: \"Well, the 'after' and 'before' \nparameters do make a difference, but why?\". I believe it is best not to \nlead users to this point.\n\n\n> Signed-off-by: Mirko Faina \n> ---\n> Documentation/rev-list-options.adoc | 6 ++--\n> revision.c | 42 ++++++++++++++++++++++---\n> revision.h | 7 ++++-\n> t/t4202-log.sh | 49 +++++++++++++++++++++++++++++\n> 4 files changed, 97 insertions(+), 7 deletions(-)\n> \n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..eed1813a92 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -914,10 +914,12 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n> avoid showing the commits from two parallel development track mixed\n> together.\n> \n> -`--reverse`::\n> +`--reverse[=(after|before)]`::\n> \tOutput the commits chosen to be shown (see 'Commit Limiting'\n> \tsection above) in reverse order. Cannot be combined with\n> -\t`--walk-reflogs`.\n> +\t`--walk-reflogs`. `when` can either be `after` or `before`, if\n> +\tomitted it defaults to `after`. If `before` is chosen,\n> +\t`--reverse` will be applied before any commit limiting options.\n> endif::git-shortlog[]\n> \n> ifndef::git-shortlog[]\n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..8338ea7448 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2685,8 +2685,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t\telse\n> \t\t\tgit_log_output_encoding = xstrdup(\"\");\n> \t\treturn argcount;\n> -\t} else if (!strcmp(arg, \"--reverse\")) {\n> -\t\trevs->reverse ^= 1;\n> +\t} else if (starts_with(arg, \"--reverse\")) {\n> +\t\tif (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> +\t\t\tif (argc < 2) {\n> +\t\t\t\trevs->reverse = 1;\n> +\t\t\t\treturn 1;\n> +\t\t\t} else {\n> +\t\t\t\toptarg = argv[1];\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (!strcmp(optarg, \"after\")) {\n> +\t\t\trevs->reverse = 1;\n> +\t\t} else if (!strcmp(optarg, \"before\")) {\n> +\t\t\trevs->reverse = 2;\n> +\t\t} else {\n> +\t\t\trevs->reverse = 1;\n> +\t\t\treturn 1;\n> +\t\t}\n> +\n> +\t\treturn optarg == argv[1] ? 2 : 1;\n> \t} else if (!strcmp(arg, \"--children\")) {\n> \t\trevs->children.name = \"children\";\n> \t\trevs->limited = 1;\n> @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n> {\n> \tstruct commit *c;\n> \tstruct commit_list *reversed;\n> +\tint max_count = revs->max_count;\n> +\n> +\tif (revs->reverse && !revs->reverse_output_stage) {\n> +\t\tif (revs->reverse == 3) {\n> +\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n> +\t\t\trevs->reverse = 1;\n> +\t\t}\n> +\n> +\t\tif (revs->reverse == 2)\n> +\t\t\trevs->max_count = -1;\n\nI think the space complexity here could be reduced a little. After all, \nsince we’re only retrieving a few commits, there’s no need to load the \nentire reversed commit history into memory.\n\nPerhaps we could maintain a window (or perhaps max heap) of finite length?\n\n> \n> -\tif (revs->reverse) {\n> \t\treversed = NULL;\n> \t\twhile ((c = get_revision_internal(revs)))\n> \t\t\tcommit_list_insert(c, &reversed);\n> \t\tcommit_list_free(revs->commits);\n> \t\trevs->commits = reversed;\n> -\t\trevs->reverse = 0;\n> \t\trevs->reverse_output_stage = 1;\n> +\n> +\t\tif (revs->reverse == 2)\n> +\t\t\trevs->max_count = max_count;\n> \t}\n> \n> \tif (revs->reverse_output_stage) {\n> +\t\tif (revs->reverse == 2 && revs->max_count == 0)\n> +\t\t\treturn NULL;\n> +\n> \t\tc = pop_commit(&revs->commits);\n> +\t\tif (revs->reverse == 2)\n> +\t\t\trevs->max_count--;\n> \t\tif (revs->track_linear)\n> \t\t\trevs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n> \t\treturn c;\n> diff --git a/revision.h b/revision.h\n> index 584f1338b5..5b23343f17 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -196,7 +196,12 @@ struct rev_info {\n> \t\t\trewrite_parents:1,\n> \t\t\tprint_parents:1,\n> \t\t\tshow_decorations:1,\n> -\t\t\treverse:1,\n> +\t\t\t/*\n> +\t\t\t * 0 no reverse\n> +\t\t\t * 1 after\n> +\t\t\t * 2 before\n> +\t\t\t */\n> +\t\t\treverse:2,\n> \t\t\treverse_output_stage:1,\n> \t\t\tcherry_pick:1,\n> \t\t\tcherry_mark:1,\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 05cee9e41b..21e9a61994 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -1882,6 +1882,55 @@ test_expect_success 'log --graph with --name-status' '\n> \ttest_cmp_graph --name-status tangle..reach\n> '\n> \n> +cat >expect <<-\\EOF\n> +c3f451c Merge tag 'reach'\n> +046b221 to remove\n> +EOF\n> +\n> +test_expect_success 'log --reverse --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse after --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse after --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse=after --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse=after --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +cat >expect <<-\\EOF\n> +3a2fdcb initial\n> +f7dab8e second\n> +EOF\n> +\n> +test_expect_success 'log --reverse before --oneline --max-count=2' '\n> +\ttest_when_finished rm actual &&\n> +\tgit log --reverse before --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse=before --oneline --max-count=2' '\n> +\ttest_when_finished rm actual &&\n> +\tgit log --reverse=before --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> cat >expect <<-\\EOF\n> * reach\n> |\n> \n> base-commit: e8955061076952cc5eab0300424fc48b601fe12d\n\nRegards, Yuchen\n\n"},{"id":"541868","messageId":"aePNILp-yB_8gfiY@exploit","threadId":"65511","inReplyTo":"fbea5f1c-946b-400e-a9a2-2c6d7b088d46@malon.dev","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-18T18:42:15Z","receivedAt":"2026-04-18T18:42:19Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 02:20:41AM +0800, Tian Yuchen wrote:\n> Hi Mirco,\n\nUnfortunate miss spell T_T\n\n> On 4/19/26 00:47, Mirko Faina wrote:\n> > In a revision walk `--reverse` can only be applied after any commit\n> > limiting option. This makes getting a limited amount of commits from the\n> > tail impossible. E.g.\n> > \n> > git log --reverse --max-count=3\n> > \n> > Some would expect this to give back the first 3 commits of the project.\n> > Instead it returns the last 3 but in reversed order.\n> > \n> > Teach `get_revision()` to accpet an argument `(after|before)` from the\n> > CLI, and apply the reversal before or after the commit limiting options\n> > based on this argument. If no argument is provided default to the\n> > current behaviour, applying `--reverse` after the commit limiting\n> > options.\n> \n> Reasoning looks good to me.\n> \n> Nit: I think we could gather more feedback on the naming. If I were a user\n> unfamiliar with how Git works, the most natural and intuitive operation for\n> me would be: \"Show me the three *oldest* commits\" (to be more precise,\n> *oldest* in the sense of topological order, rather than the commit date or\n> author date order. It’s really annoying. ), rather than \"reverse the entire\n> list and then select the three most recent ones\". I think the confusion\n> arises because users do not (and should not) know that '--max-count' only\n> returns the most recent commits; consequently, they might wonder: \"Well, the\n> 'after' and 'before' parameters do make a difference, but why?\". I believe\n> it is best not to lead users to this point.\n\nThe rest of the flags do apply during traversal, and since git goes\nthrough the whole history the relative order should be preserved, so\ndepending on the flags that are passed the user can order topologically\nor chronologically (tho some flags are forbidden together).\n\nI'm up to change the naming tho (and if someone has better phrasing for\nthe docs please tell).\n\n> > @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n> > {\n> > \tstruct commit *c;\n> > \tstruct commit_list *reversed;\n> > +\tint max_count = revs->max_count;\n> > +\n> > +\tif (revs->reverse && !revs->reverse_output_stage) {\n> > +\t\tif (revs->reverse == 3) {\n> > +\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n> > +\t\t\trevs->reverse = 1;\n> > +\t\t}\n> > +\n> > +\t\tif (revs->reverse == 2)\n> > +\t\t\trevs->max_count = -1;\n> \n> I think the space complexity here could be reduced a little. After all,\n> since we’re only retrieving a few commits, there’s no need to load the\n> entire reversed commit history into memory.\n> \n> Perhaps we could maintain a window (or perhaps max heap) of finite length?\n\nUnfortunately since the underlying data structure is a linked list we\nhave to traverse the whole tree to get the first one from the tail. The\nway get_revision() loads the next commit is through process_parents().\nEven if we were able to start from the tail we wouldn't have any\nreference to the children.\n\nI suspect reducing space complexity would require to change a lot of\ninner workings of git to make the history traversable both ways.\n\nThank you\n"},{"id":"541869","messageId":"aePSGXu5l9x-NKVG@exploit","threadId":"65511","inReplyTo":"aePNILp-yB_8gfiY@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-18T18:51:12Z","receivedAt":"2026-04-18T18:51:15Z","isPatch":true,"body":"On Sat, Apr 18, 2026 at 08:42:15PM +0200, Mirko Faina wrote:\n> > I think the space complexity here could be reduced a little. After all,\n> > since we’re only retrieving a few commits, there’s no need to load the\n> > entire reversed commit history into memory.\n> > \n> > Perhaps we could maintain a window (or perhaps max heap) of finite length?\n> \n> Unfortunately since the underlying data structure is a linked list we\n> have to traverse the whole tree to get the first one from the tail. The\n> way get_revision() loads the next commit is through process_parents().\n> Even if we were able to start from the tail we wouldn't have any\n> reference to the children.\n> \n> I suspect reducing space complexity would require to change a lot of\n> inner workings of git to make the history traversable both ways.\n\nOh, I missunderstood what you meant, sorry. Yes, a window should allow\nus to reduce the amount of the memory used. Will do in v2.\n"},{"id":"541883","messageId":"C60EE993-97DA-45F7-89DE-2F97ABB0F685@gmail.com","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-04-19T12:06:24Z","receivedAt":"2026-04-19T12:06:38Z","isPatch":true,"body":"\n> Le 18 avr. 2026 à 12:57, Mirko Faina a écrit :\n> \n> In a revision walk `--reverse` can only be applied after any commit\n> limiting option. This makes getting a limited amount of commits from the\n> tail impossible. E.g.\n> \n> git log --reverse --max-count=3\n> \n> Some would expect this to give back the first 3 commits of the project.\n> Instead it returns the last 3 but in reversed order.\n> \n> Teach `get_revision()` to accpet an argument `(after|before)` from the\n> CLI, and apply the reversal before or after the commit limiting options\n> based on this argument. If no argument is provided default to the\n> current behaviour, applying `--reverse` after the commit limiting\n> options.\n> \n> Signed-off-by: Mirko Faina \n> ---\n> Documentation/rev-list-options.adoc | 6 ++--\n> revision.c | 42 ++++++++++++++++++++++---\n> revision.h | 7 ++++-\n> t/t4202-log.sh | 49 +++++++++++++++++++++++++++++\n> 4 files changed, 97 insertions(+), 7 deletions(-)\n> \n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..eed1813a92 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -914,10 +914,12 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n> avoid showing the commits from two parallel development track mixed\n> together.\n> \n> -`--reverse`::\n> +`--reverse[=(after|before)]`::\n> Output the commits chosen to be shown (see 'Commit Limiting'\n> section above) in reverse order. Cannot be combined with\n> - `--walk-reflogs`.\n> + `--walk-reflogs`. `when` can either be `after` or `before`, if\n\n“When” is not mentioned prior to here, so it’s explanation leaves the reader wondering what it refers to.\n\n> + omitted it defaults to `after`. If `before` is chosen,\n> + `--reverse` will be applied before any commit limiting options.\n> endif::git-shortlog[]\n> \n> ifndef::git-shortlog[]\n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..8338ea7448 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2685,8 +2685,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> else\n> git_log_output_encoding = xstrdup(\"\");\n> return argcount;\n> - } else if (!strcmp(arg, \"--reverse\")) {\n> - revs->reverse ^= 1;\n\nThe original handles multiple reverse options inverting each other…\n\n> + } else if (starts_with(arg, \"--reverse\")) {\n> + if (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> + if (argc < 2) {\n> + revs->reverse = 1;\n> + return 1;\n> + } else {\n> + optarg = argv[1];\n> + }\n> + }\n> +\n> + if (!strcmp(optarg, \"after\")) {\n> + revs->reverse = 1;\n> + } else if (!strcmp(optarg, \"before\")) {\n> + revs->reverse = 2;\n> + } else {\n> + revs->reverse = 1;\n> + return 1;\n> + }\n> +\n> + return optarg == argv[1] ? 2 : 1;\n\n…which I don’t see here.\n\nI’m not familiar with this parsing code though so I can’t add much about the test other than to say it is a bit hard to follow :/\n\n> } else if (!strcmp(arg, \"--children\")) {\n> revs->children.name = \"children\";\n> revs->limited = 1;\n> @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n> {\n> struct commit *c;\n> struct commit_list *reversed;\n> + int max_count = revs->max_count;\n> +\n> + if (revs->reverse && !revs->reverse_output_stage) {\n> + if (revs->reverse == 3) {\n> + BUG(\"allowed values for reverse are 0, 1 and 2\");\n> + revs->reverse = 1;\n> + }\n\nIs this possible? I guess I can see from the expanded bit width that it’s a valid input, and there’s no protection stopping other callers accidentally adding this.\n\nI haven’t looked, but it would be nice if we could use an enum instead. Unfortunately that would probably take up more space in the struct, and I suppose the bit-packing is done intentionally for performance. \n\n> +\n> + if (revs->reverse == 2)\n> + revs->max_count = -1;\n> \n> - if (revs->reverse) {\n> reversed = NULL;\n> while ((c = get_revision_internal(revs)))\n> commit_list_insert(c, &reversed);\n> commit_list_free(revs->commits);\n> revs->commits = reversed;\n> - revs->reverse = 0;\n> revs->reverse_output_stage = 1;\n> +\n> + if (revs->reverse == 2)\n> + revs->max_count = max_count;\n> }\n\nIt looks we temporarily disable reversing and then re-enable it here, which makes some sense to me as a way to do “after” mode. \n\n> \n> if (revs->reverse_output_stage) {\n> + if (revs->reverse == 2 && revs->max_count == 0)\n> + return NULL;\n> +\n> c = pop_commit(&revs->commits);\n> + if (revs->reverse == 2)\n> + revs->max_count--;\n\nHm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising. \n\n> if (revs->track_linear)\n> revs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n> return c;\n> diff --git a/revision.h b/revision.h\n> index 584f1338b5..5b23343f17 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -196,7 +196,12 @@ struct rev_info {\n> rewrite_parents:1,\n> print_parents:1,\n> show_decorations:1,\n> - reverse:1,\n> + /*\n> + * 0 no reverse\n> + * 1 after\n> + * 2 before\n> + */\n> + reverse:2,\n> reverse_output_stage:1,\n> cherry_pick:1,\n> cherry_mark:1,\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 05cee9e41b..21e9a61994 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -1882,6 +1882,55 @@ test_expect_success 'log --graph with --name-status' '\n> test_cmp_graph --name-status tangle..reach\n> '\n> \n> +cat >expect <<-\\EOF\n> +c3f451c Merge tag 'reach'\n> +046b221 to remove\n> +EOF\n> +\n> +test_expect_success 'log --reverse --oneline --max-count=2' '\n> + test_when_finished git reset --hard HEAD~1 &&\n> + touch to_remove &&\n> + git add to_remove &&\n> + git commit -m \"to remove\" &&\n> + git log --reverse --oneline --max-count=2 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse after --oneline --max-count=2' '\n> + test_when_finished git reset --hard HEAD~1 &&\n> + touch to_remove &&\n> + git add to_remove &&\n> + git commit -m \"to remove\" &&\n> + git log --reverse after --oneline --max-count=2 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse=after --oneline --max-count=2' '\n> + test_when_finished git reset --hard HEAD~1 &&\n> + touch to_remove &&\n> + git add to_remove &&\n> + git commit -m \"to remove\" &&\n> + git log --reverse=after --oneline --max-count=2 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +cat >expect <<-\\EOF\n> +3a2fdcb initial\n> +f7dab8e second\n> +EOF\n> +\n> +test_expect_success 'log --reverse before --oneline --max-count=2' '\n> + test_when_finished rm actual &&\n> + git log --reverse before --oneline --max-count=2 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse=before --oneline --max-count=2' '\n> + test_when_finished rm actual &&\n> + git log --reverse=before --oneline --max-count=2 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> cat >expect <<-\\EOF\n> * reach\n> |\n> \n> base-commit: e8955061076952cc5eab0300424fc48b601fe12d\n> --\n> 2.54.0.rc2.9.ge895506107\n> \n> \n"},{"id":"541887","messageId":"aeUZUqSQI8FvRUco@exploit","threadId":"65511","inReplyTo":"C60EE993-97DA-45F7-89DE-2F97ABB0F685@gmail.com","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-19T18:11:13Z","receivedAt":"2026-04-19T18:11:23Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 08:06:24AM -0400, Ben Knoble wrote:\n> > -`--reverse`::\n> > +`--reverse[=(after|before)]`::\n> > Output the commits chosen to be shown (see 'Commit Limiting'\n> > section above) in reverse order. Cannot be combined with\n> > - `--walk-reflogs`.\n> > + `--walk-reflogs`. `when` can either be `after` or `before`, if\n> \n> “When” is not mentioned prior to here, so it’s explanation leaves the reader wondering what it refers to.\n\nYes, when I first wrote it it said `--reverse[=when]` but forgot to\nchange the text after deciding to give the valid inputs instead. Will\nfix in v2.\n\n> The original handles multiple reverse options inverting each other…\n> \n> > + } else if (starts_with(arg, \"--reverse\")) {\n> > + if (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> > + if (argc < 2) {\n> > + revs->reverse = 1;\n> > + return 1;\n> > + } else {\n> > + optarg = argv[1];\n> > + }\n> > + }\n> > +\n> > + if (!strcmp(optarg, \"after\")) {\n> > + revs->reverse = 1;\n> > + } else if (!strcmp(optarg, \"before\")) {\n> > + revs->reverse = 2;\n> > + } else {\n> > + revs->reverse = 1;\n> > + return 1;\n> > + }\n> > +\n> > + return optarg == argv[1] ? 2 : 1;\n> \n> …which I don’t see here.\n> \n> I’m not familiar with this parsing code though so I can’t add much about the test other than to say it is a bit hard to follow :/\n\nGiven that it is no longer binary handling multiple reverse can't simply\nbe inverting bits, it wouldn't make sense. This is done before the walk\nitself, so even from the POV of the user it wouldn't make much sense to\nreverse multiple times as the order of the applied options before this\npatch (commit limiting options then reverse) doesn't change.\n\nThis doesn't break any tests so I assumed it was fine.\n\n> > } else if (!strcmp(arg, \"--children\")) {\n> > revs->children.name = \"children\";\n> > revs->limited = 1;\n> > @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n> > {\n> > struct commit *c;\n> > struct commit_list *reversed;\n> > + int max_count = revs->max_count;\n> > +\n> > + if (revs->reverse && !revs->reverse_output_stage) {\n> > + if (revs->reverse == 3) {\n> > + BUG(\"allowed values for reverse are 0, 1 and 2\");\n> > + revs->reverse = 1;\n> > + }\n> \n> Is this possible? I guess I can see from the expanded bit width that it’s a valid input, and there’s no protection stopping other callers accidentally adding this.\n\nCurrent code should never generate a 3, but in case it happens I assume\nthe user wants to use the original behaviour of reverse, so I set the\nvalue accordingly instead of stopping the program and notify that\nthere's a bug.\n\nShould this be changed?\n\n> I haven’t looked, but it would be nice if we could use an enum instead. Unfortunately that would probably take up more space in the struct, and I suppose the bit-packing is done intentionally for performance. \n\nCould define new macros so that the readers don't have to mentally keep\ntrack of which value rapresents what. I didn't think that was\nnecessary, should I change it?\n\n> > \n> > if (revs->reverse_output_stage) {\n> > + if (revs->reverse == 2 && revs->max_count == 0)\n> > + return NULL;\n> > +\n> > c = pop_commit(&revs->commits);\n> > + if (revs->reverse == 2)\n> > + revs->max_count--;\n> \n> Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising. \n\nget_revision() (in revision.c) handles the reverse option and updates\nthe \"struct git_graph\". get_revision() then calls\nget_revision_internal(), which handles commit boundaries and max_count,\nhere is where it gets decreased. Since max_count gets decreased\neverytime get_revision_internal() is called, if we were to leave\nmax_count as is before the walk (in get_revision() at line 4558), the\nwalk would stop before reaching the root commit. This is why the current\n--reverse option is applied only after commit limiting options. So\ninstead we set max_count at -1 walking the whole history and storing it\nin 'reversed'. Now we're in \"reverse_output_stage = 1\", and in this\nstate we never call get_revision_internal() again, instead we pop\ncommits from 'reversed'. Because of this we have to handle max_count\noutside get_revision_internal(), so we decrement it in the snippet of\ncode you referenced.\n\nA bit verbose but hopefully it'll get my point across.\n\nThank you\n"},{"id":"541888","messageId":"CALnO6CACfSyzyguX4623Dk3y+QEM_Dbmfko8dTyM1p3JxBjZFg@mail.gmail.com","threadId":"65511","inReplyTo":"aeUZUqSQI8FvRUco@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-04-19T19:12:27Z","receivedAt":"2026-04-19T19:12:39Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 2:11 PM Mirko Faina wrote:\n>\n> On Sun, Apr 19, 2026 at 08:06:24AM -0400, Ben Knoble wrote:\n> > The original handles multiple reverse options inverting each other…\n> >\n> > > + } else if (starts_with(arg, \"--reverse\")) {\n> > > + if (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> > > + if (argc < 2) {\n> > > + revs->reverse = 1;\n> > > + return 1;\n> > > + } else {\n> > > + optarg = argv[1];\n> > > + }\n> > > + }\n> > > +\n> > > + if (!strcmp(optarg, \"after\")) {\n> > > + revs->reverse = 1;\n> > > + } else if (!strcmp(optarg, \"before\")) {\n> > > + revs->reverse = 2;\n> > > + } else {\n> > > + revs->reverse = 1;\n> > > + return 1;\n> > > + }\n> > > +\n> > > + return optarg == argv[1] ? 2 : 1;\n> >\n> > …which I don’t see here.\n> >\n> > I’m not familiar with this parsing code though so I can’t add much about the test other than to say it is a bit hard to follow :/\n>\n> Given that it is no longer binary handling multiple reverse can't simply\n> be inverting bits, it wouldn't make sense. This is done before the walk\n> itself, so even from the POV of the user it wouldn't make much sense to\n> reverse multiple times as the order of the applied options before this\n> patch (commit limiting options then reverse) doesn't change.\n>\n> This doesn't break any tests so I assumed it was fine.\n\nI think I mean that\n\n git log --reverse --reverse\n\nshows commits in the same order as \"git log\"; what should\n\n git log --reverse=after --reverse\n\ndo? Or what about preserving the behavior of the original \"git log\n--reverse --reverse,\" which I don't think is done here?\n\nGranted, I don't see this ability documented, and I cannot tell how\nmany may scream if we change this behavior, so it's a bit\nhypothetical. But there is an argument for backward compatibility as a\ndefault, which I think we'd need to justify changing. Perhaps in the\nproposed log message?\n\n(The original seems nonsensical to type, but of course you can imagine\nalias.A=log --reverse , and then sometimes you want to do\n\"git A --reverse\" to un-reverse the commits.)\n\n> > > } else if (!strcmp(arg, \"--children\")) {\n> > > revs->children.name = \"children\";\n> > > revs->limited = 1;\n> > > @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)\n> > > {\n> > > struct commit *c;\n> > > struct commit_list *reversed;\n> > > + int max_count = revs->max_count;\n> > > +\n> > > + if (revs->reverse && !revs->reverse_output_stage) {\n> > > + if (revs->reverse == 3) {\n> > > + BUG(\"allowed values for reverse are 0, 1 and 2\");\n> > > + revs->reverse = 1;\n> > > + }\n> >\n> > Is this possible? I guess I can see from the expanded bit width that it’s a valid input, and there’s no protection stopping other callers accidentally adding this.\n>\n> Current code should never generate a 3, but in case it happens I assume\n> the user wants to use the original behaviour of reverse, so I set the\n> value accordingly instead of stopping the program and notify that\n> there's a bug.\n>\n> Should this be changed?\n\nI don't have any strong opinions on this.\n\n> > I haven’t looked, but it would be nice if we could use an enum instead. Unfortunately that would probably take up more space in the struct, and I suppose the bit-packing is done intentionally for performance.\n>\n> Could define new macros so that the readers don't have to mentally keep\n> track of which value rapresents what. I didn't think that was\n> necessary, should I change it?\n\nYeah, a few `#define`d constants would make things more readable to\nme, at least, since we can't use the enum without space concerns\n(unless there's a way to bit-pack the enum to only 2 bits?).\n\n> > > if (revs->reverse_output_stage) {\n> > > + if (revs->reverse == 2 && revs->max_count == 0)\n> > > + return NULL;\n> > > +\n\nPS: something I spotted on a second read. [Ignoring reverse=after\nmode] This hunk looks to me like a nice little optimization (return\nnothing if we know max_count says we yield no commits). Of course, I\ncould see that being viable early in the function, right? When asking\nget_revision for commits, if max_count is 0, just return NULL.\n\nFor reverse=after mode, this condition is only true if the max_count\nwas 0 in the previous conditional, also, since we use max_count=-1\nbefore iterating get_revision_internal. That means the original\nmax_count isn't touched. At any rate, it _seems_ to me that the whole\nfunction could benefit from this optimization… but I wonder if it is\n_necessary_ for correctness of reverse=after in some way that I'm not\nseeing? Since the current version doesn't need the early bailout, why\ndoes reverse=after?\n\n> > > c = pop_commit(&revs->commits);\n> > > + if (revs->reverse == 2)\n> > > + revs->max_count--;\n> >\n> > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.\n>\n> get_revision() (in revision.c) handles the reverse option and updates\n> the \"struct git_graph\". get_revision() then calls\n> get_revision_internal(), which handles commit boundaries and max_count,\n> here is where it gets decreased. Since max_count gets decreased\n> everytime get_revision_internal() is called, if we were to leave\n> max_count as is before the walk (in get_revision() at line 4558), the\n> walk would stop before reaching the root commit. This is why the current\n> --reverse option is applied only after commit limiting options. So\n> instead we set max_count at -1 walking the whole history and storing it\n> in 'reversed'. Now we're in \"reverse_output_stage = 1\", and in this\n> state we never call get_revision_internal() again, instead we pop\n> commits from 'reversed'. Because of this we have to handle max_count\n> outside get_revision_internal(), so we decrement it in the snippet of\n> code you referenced.\n>\n> A bit verbose but hopefully it'll get my point across.\n\nI don't 100% follow, but I'm out of my depth :)\n\nI think I see that get_revision() effectively has 2 modes pertaining\nto reverse: reverse and reverse output stage (the former falls\ndirectly into the latter, though).\n\nAfter some setup, the reverse mode calls get_revision_internal() as\nyou said. That decrements max_count as a way of counting how many\ncommits we've seen through the loop, so if we asked for 5 we'd only\nprocess 5 commits.\n\nThen we fall into the output stage mode, which pops a commit [1].\n\nWith this patch, in reverse=after we disable max_count in the first\n(reverse) mode, as you said. Ok: we get the whole (filtered) history\nthen, at which point we can now shrink. That makes sense.\n\nThen in the reverse output stage mode, we pretend to have one less\nmax_count. That's what I can't figure out. Is it because of the\npop_commit()? I guess I'm not totally seeing how that interacted with\nthe max_count in the original code: does the current code yield one\nextra commit in get_revision_internal() ?\n\nYou wrote that \"we never call get_revision_internal() again,\" but I\ndon't see why that's true with this patch and not true before it.\n\nI do agree that _somebody_ has to handle max_count after\nget_revision() returns with reverse=after. I'm just not sure what\n\n if (revs->reverse == 2)\n revs->max_count--;\n\nis doing.\n\nOf course if I'm the only one confused and others make sense of it,\nthat's ok, too.\n\n> Thank you\n\nThanks!\n\n-- \nD. Ben Knoble\n\n[1]: I traced this to 498bcd3159 (rev-list: fix --reverse interaction\nwith --parents, 2008-08-29), but I can't fathom what the pop is doing\nthere.\n"},{"id":"541890","messageId":"aeUqSltEWIWaPDh3@exploit","threadId":"65511","inReplyTo":"CALnO6CACfSyzyguX4623Dk3y+QEM_Dbmfko8dTyM1p3JxBjZFg@mail.gmail.com","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-19T20:31:37Z","receivedAt":"2026-04-19T20:31:41Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 03:12:27PM -0400, D. Ben Knoble wrote:\n> On Sun, Apr 19, 2026 at 2:11 PM Mirko Faina wrote:\n> >\n> > On Sun, Apr 19, 2026 at 08:06:24AM -0400, Ben Knoble wrote:\n> > > The original handles multiple reverse options inverting each other…\n> > >\n> > > > + } else if (starts_with(arg, \"--reverse\")) {\n> > > > + if (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> > > > + if (argc < 2) {\n> > > > + revs->reverse = 1;\n> > > > + return 1;\n> > > > + } else {\n> > > > + optarg = argv[1];\n> > > > + }\n> > > > + }\n> > > > +\n> > > > + if (!strcmp(optarg, \"after\")) {\n> > > > + revs->reverse = 1;\n> > > > + } else if (!strcmp(optarg, \"before\")) {\n> > > > + revs->reverse = 2;\n> > > > + } else {\n> > > > + revs->reverse = 1;\n> > > > + return 1;\n> > > > + }\n> > > > +\n> > > > + return optarg == argv[1] ? 2 : 1;\n> > >\n> > > …which I don’t see here.\n> > >\n> > > I’m not familiar with this parsing code though so I can’t add much about the test other than to say it is a bit hard to follow :/\n> >\n> > Given that it is no longer binary handling multiple reverse can't simply\n> > be inverting bits, it wouldn't make sense. This is done before the walk\n> > itself, so even from the POV of the user it wouldn't make much sense to\n> > reverse multiple times as the order of the applied options before this\n> > patch (commit limiting options then reverse) doesn't change.\n> >\n> > This doesn't break any tests so I assumed it was fine.\n> \n> I think I mean that\n> \n> git log --reverse --reverse\n> \n> shows commits in the same order as \"git log\"; what should\n> \n> git log --reverse=after --reverse\n> \n> do? Or what about preserving the behavior of the original \"git log\n> --reverse --reverse,\" which I don't think is done here?\n\nYes, this is what I was getting at. Since it is no longer binary what\nwould a double reverse mean? What if \"--reverse=after --reverse=before\"?\nHow should that be handled?\n\n> Granted, I don't see this ability documented, and I cannot tell how\n> many may scream if we change this behavior, so it's a bit\n> hypothetical. But there is an argument for backward compatibility as a\n> default, which I think we'd need to justify changing. Perhaps in the\n> proposed log message?\n> \n> (The original seems nonsensical to type, but of course you can imagine\n> alias.A=log --reverse , and then sometimes you want to do\n> \"git A --reverse\" to un-reverse the commits.)\n\nWill do in v2.\n\n> > > I haven’t looked, but it would be nice if we could use an enum instead. Unfortunately that would probably take up more space in the struct, and I suppose the bit-packing is done intentionally for performance.\n> >\n> > Could define new macros so that the readers don't have to mentally keep\n> > track of which value rapresents what. I didn't think that was\n> > necessary, should I change it?\n> \n> Yeah, a few `#define`d constants would make things more readable to\n> me, at least, since we can't use the enum without space concerns\n> (unless there's a way to bit-pack the enum to only 2 bits?).\n\nWill do in v2.\n\n> > > > if (revs->reverse_output_stage) {\n> > > > + if (revs->reverse == 2 && revs->max_count == 0)\n> > > > + return NULL;\n> > > > +\n> \n> PS: something I spotted on a second read. [Ignoring reverse=after\n> mode] This hunk looks to me like a nice little optimization (return\n> nothing if we know max_count says we yield no commits). Of course, I\n> could see that being viable early in the function, right? When asking\n> get_revision for commits, if max_count is 0, just return NULL.\n> \n> For reverse=after mode, this condition is only true if the max_count\n> was 0 in the previous conditional, also, since we use max_count=-1\n> before iterating get_revision_internal. That means the original\n> max_count isn't touched. At any rate, it _seems_ to me that the whole\n> function could benefit from this optimization… but I wonder if it is\n> _necessary_ for correctness of reverse=after in some way that I'm not\n> seeing? Since the current version doesn't need the early bailout, why\n> does reverse=after?\n\nJust to clarify, \"reverse = 2\" is \"--reverse=before\" and not\n\"--reverse=after\".\n\nWith \"reverse = 2\", the snippet of code you're referencing is not an\noptimization but a requirement for correctness. With \"reverse = 1\" we\njust keep the max_count as is and it's used by get_revision_internal()\nto stop if that limit is reached. What we find in 'reversed' are already\njust the commits we need to return.\n\nWith \"reverse = 2\", we first set max_count to -1 and then retrieve the\nwhole history, then we set max_count to its original value. Then we\nreturn the commits on each call of get_revision(). Now, unlike with\n\"reverse = 1\", we have the whole history in 'reversed', because of that\nwe need to know when to stop. That's the reason we decrement max_count\nonly for \"reverse = 2\" and why \"max_count == 0\" is checked only for\n\"reverse = 2\".\n\n> > > > c = pop_commit(&revs->commits);\n> > > > + if (revs->reverse == 2)\n> > > > + revs->max_count--;\n> > >\n> > > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.\n> >\n> > get_revision() (in revision.c) handles the reverse option and updates\n> > the \"struct git_graph\". get_revision() then calls\n> > get_revision_internal(), which handles commit boundaries and max_count,\n> > here is where it gets decreased. Since max_count gets decreased\n> > everytime get_revision_internal() is called, if we were to leave\n> > max_count as is before the walk (in get_revision() at line 4558), the\n> > walk would stop before reaching the root commit. This is why the current\n> > --reverse option is applied only after commit limiting options. So\n> > instead we set max_count at -1 walking the whole history and storing it\n> > in 'reversed'. Now we're in \"reverse_output_stage = 1\", and in this\n> > state we never call get_revision_internal() again, instead we pop\n> > commits from 'reversed'. Because of this we have to handle max_count\n> > outside get_revision_internal(), so we decrement it in the snippet of\n> > code you referenced.\n> >\n> > A bit verbose but hopefully it'll get my point across.\n> \n> I don't 100% follow, but I'm out of my depth :)\n> \n> I think I see that get_revision() effectively has 2 modes pertaining\n> to reverse: reverse and reverse output stage (the former falls\n> directly into the latter, though).\n> \n> After some setup, the reverse mode calls get_revision_internal() as\n> you said. That decrements max_count as a way of counting how many\n> commits we've seen through the loop, so if we asked for 5 we'd only\n> process 5 commits.\n> \n> Then we fall into the output stage mode, which pops a commit [1].\n> \n> With this patch, in reverse=after we disable max_count in the first\n> (reverse) mode, as you said. Ok: we get the whole (filtered) history\n> then, at which point we can now shrink. That makes sense.\n> \n> Then in the reverse output stage mode, we pretend to have one less\n> max_count. That's what I can't figure out. Is it because of the\n> pop_commit()? I guess I'm not totally seeing how that interacted with\n> the max_count in the original code: does the current code yield one\n> extra commit in get_revision_internal() ?\n\nI'm not sure I understand what you're referencing with \"Then in the\nreverse output stage mode, we pretend to have one less max_count\".\n\nIf you're referring to line 4573, then...\n\n> You wrote that \"we never call get_revision_internal() again,\" but I\n> don't see why that's true with this patch and not true before it.\n> \n> I do agree that _somebody_ has to handle max_count after\n> get_revision() returns with reverse=after. I'm just not sure what\n> \n> if (revs->reverse == 2)\n> revs->max_count--;\n> \n> is doing.\n\n...we're not pretending we have fewer commits. Every subsequent call to\nget_revision() after the first call will never enter the branch at line\n4548 and will only enter the branch at 4568. Everytime we pop a commit\nfrom 'reversed' we decrease max_count so we can limit only to the amount\nof commits the user wants.\n\nSo, to recap, with \"reverse = 2\", on the first call to get_revision() we\nwalk the whole history and store it in 'reversed' in reversed order and\nreturn the first commit.\nOn subsequent calls to get_revision() we do not walk the history again,\nwe simply return the commits that have been stored in 'reversed'.\nEverytime we pop a commit we have to decrease max_count, and we check\nagaints max_count to know if we shouldn't return anymore commits (by\nreturning NULL).\n\n> Of course if I'm the only one confused and others make sense of it,\n> that's ok, too.\n\nNo, I completely understand. I did have to retouch the function a few\ntimes after writing the tests :P\n\n> [1]: I traced this to 498bcd3159 (rev-list: fix --reverse interaction\n> with --parents, 2008-08-29), but I can't fathom what the pop is doing\n> there.\n\nIt's pretty much doing the same thing it does now, it's returning stored\ncommits. In both versions, the initial setup when \"revs->reverse\" is\ntrue, becomes \"dead code\" after the first call.\n\nThank you\n"},{"id":"541896","messageId":"20260420000440.GA1238475@coredump.intra.peff.net","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-20T00:04:40Z","receivedAt":"2026-04-20T00:04:47Z","isPatch":true,"body":"On Sat, Apr 18, 2026 at 06:47:35PM +0200, Mirko Faina wrote:\n\n> @@ -2685,8 +2685,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t\telse\n> \t\t\tgit_log_output_encoding = xstrdup(\"\");\n> \t\treturn argcount;\n> -\t} else if (!strcmp(arg, \"--reverse\")) {\n> -\t\trevs->reverse ^= 1;\n> +\t} else if (starts_with(arg, \"--reverse\")) {\n> +\t\tif (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> +\t\t\tif (argc < 2) {\n> +\t\t\t\trevs->reverse = 1;\n> +\t\t\t\treturn 1;\n> +\t\t\t} else {\n> +\t\t\t\toptarg = argv[1];\n> +\t\t\t}\n> +\t\t}\n\nIt looks like you're trying to support \"--reverse after\" here, but don't\ndo that. Flags with optional arguments must use the \"stuck\" form,\n\"--reverse=after\", which is covered in the \"gitcli\" manpage.\n\nThat's to prevent \"--reverse --foo\" from being ambiguous. It looks like\nyou try to limit that with the final \"else\" here:\n\n> +\n> +\t\tif (!strcmp(optarg, \"after\")) {\n> +\t\t\trevs->reverse = 1;\n> +\t\t} else if (!strcmp(optarg, \"before\")) {\n> +\t\t\trevs->reverse = 2;\n> +\t\t} else {\n> +\t\t\trevs->reverse = 1;\n> +\t\t\treturn 1;\n> +\t\t}\n\nbut that just makes things more complicated:\n\n - doing \"git log --reverse=bogus\" is silently accepted\n\n - trying to show a branch named \"after\" with \"git log --reverse after\"\n has changed meanings\n\nSo I think you really just want to handle \"--reverse=\" separately from\n\"--reverse\", and the latter should behave as it always has.\n\n-Peff\n"},{"id":"541898","messageId":"20260420002118.GB1238475@coredump.intra.peff.net","threadId":"65511","inReplyTo":"aeUqSltEWIWaPDh3@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-20T00:21:18Z","receivedAt":"2026-04-20T00:21:20Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 10:31:37PM +0200, Mirko Faina wrote:\n\n> > I think I mean that\n> > \n> > git log --reverse --reverse\n> > \n> > shows commits in the same order as \"git log\"; what should\n> > \n> > git log --reverse=after --reverse\n> > \n> > do? Or what about preserving the behavior of the original \"git log\n> > --reverse --reverse,\" which I don't think is done here?\n> \n> Yes, this is what I was getting at. Since it is no longer binary what\n> would a double reverse mean? What if \"--reverse=after --reverse=before\"?\n> How should that be handled?\n\nYeah, I agree it gets weird, and I think it is OK if we don't try to\ncombine before/after reverses (either making it an error, or using the\nusual last-one-wins to have \"before\" override \"after\" in this example).\n\nBut we should keep \"--reverse --reverse\" working as before, as there is\nno other way to countermand a previously-given reverse option, and\nbecause it has always worked.\n\nUsually we'd spell the option \"--no-reverse\", and it probably makes\nsense to add it (to override an earlier \"--reverse=after\"), but we'd\nstill want to keep \"--reverse --reverse\" working for historical\ncompatibility.\n\nSo combined with the earlier suggestions for using an enum and\ndisallowing the un-stuck \"--reverse after\" form, we probably want\nsomething like (totally untested):\n\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..89a58a65b7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2686,7 +2686,20 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\tgit_log_output_encoding = xstrdup(\"\");\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n-\t\trevs->reverse ^= 1;\n+\t\t/*\n+\t\t * This relies on \"do not reverse\" being the 0 value for our\n+\t\t * enum, and historical \"reverse after\" having value 1.\n+\t\t */\n+\t\trevs->reverse = !revs->reverse;\n+\t} else if (!strcmp(arg, \"--no-reverse\")) {\n+\t\trevs->reverse = 0;\n+\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n+\t\tif (!strcmp(optarg, \"after\"))\n+\t\t\trevs->reverse = REVS_REVERSE_AFTER;\n+\t\telse if (!strcmp(optarg, \"before\"))\n+\t\t\trevs->reverse = REVS_REVERSE_BEFORE;\n+\t\telse\n+\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n\nNote that your original also allowed --reverse-o-matic, which we\nprobably don't want (and is fixed here).\n\nI _think_ the negation from using \"--reverse\" after \"--reverse=before\"\nshould be sensible here. And \"--reverse=\" with two different modes just\noverrides rather than trying to be clever. But you may want to\ndouble-check all of the combinations.\n\nThis would all be much easier if revision.c used parse-options, of\ncourse, which has all of these sorts of rules baked-in. But that's a\nmuch bigger conversion, and probably not something you want to make a\nprerequisite for your series. ;)\n\n-Peff\n"},{"id":"541949","messageId":"aeXvuuhjTmHyumGq@exploit","threadId":"65511","inReplyTo":"20260420000440.GA1238475@coredump.intra.peff.net","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-20T09:22:09Z","receivedAt":"2026-04-20T09:22:13Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 08:04:40PM -0400, Jeff King wrote:\n> On Sat, Apr 18, 2026 at 06:47:35PM +0200, Mirko Faina wrote:\n> \n> > @@ -2685,8 +2685,26 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> > \t\telse\n> > \t\t\tgit_log_output_encoding = xstrdup(\"\");\n> > \t\treturn argcount;\n> > -\t} else if (!strcmp(arg, \"--reverse\")) {\n> > -\t\trevs->reverse ^= 1;\n> > +\t} else if (starts_with(arg, \"--reverse\")) {\n> > +\t\tif (!skip_prefix(arg, \"--reverse=\", &optarg)) {\n> > +\t\t\tif (argc < 2) {\n> > +\t\t\t\trevs->reverse = 1;\n> > +\t\t\t\treturn 1;\n> > +\t\t\t} else {\n> > +\t\t\t\toptarg = argv[1];\n> > +\t\t\t}\n> > +\t\t}\n> \n> It looks like you're trying to support \"--reverse after\" here, but don't\n> do that. Flags with optional arguments must use the \"stuck\" form,\n> \"--reverse=after\", which is covered in the \"gitcli\" manpage.\n\nOh I see, I thought I had to parse both ways. If I just have to allow\nfor the stuck form then it becomes way easier to deal with.\n\n> That's to prevent \"--reverse --foo\" from being ambiguous. It looks like\n> you try to limit that with the final \"else\" here:\n> \n> > +\n> > +\t\tif (!strcmp(optarg, \"after\")) {\n> > +\t\t\trevs->reverse = 1;\n> > +\t\t} else if (!strcmp(optarg, \"before\")) {\n> > +\t\t\trevs->reverse = 2;\n> > +\t\t} else {\n> > +\t\t\trevs->reverse = 1;\n> > +\t\t\treturn 1;\n> > +\t\t}\n> \n> but that just makes things more complicated:\n> \n> - doing \"git log --reverse=bogus\" is silently accepted\n> \n> - trying to show a branch named \"after\" with \"git log --reverse after\"\n> has changed meanings\n> \n> So I think you really just want to handle \"--reverse=\" separately from\n> \"--reverse\", and the latter should behave as it always has.\n> \n> -Peff\n\nThanks you\n"},{"id":"541950","messageId":"aeXxC8eR0Mn3dGEn@exploit","threadId":"65511","inReplyTo":"20260420002118.GB1238475@coredump.intra.peff.net","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-20T09:33:25Z","receivedAt":"2026-04-20T09:33:29Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 08:21:18PM -0400, Jeff King wrote:\n> On Sun, Apr 19, 2026 at 10:31:37PM +0200, Mirko Faina wrote:\n> \n> > > I think I mean that\n> > > \n> > > git log --reverse --reverse\n> > > \n> > > shows commits in the same order as \"git log\"; what should\n> > > \n> > > git log --reverse=after --reverse\n> > > \n> > > do? Or what about preserving the behavior of the original \"git log\n> > > --reverse --reverse,\" which I don't think is done here?\n> > \n> > Yes, this is what I was getting at. Since it is no longer binary what\n> > would a double reverse mean? What if \"--reverse=after --reverse=before\"?\n> > How should that be handled?\n> \n> Yeah, I agree it gets weird, and I think it is OK if we don't try to\n> combine before/after reverses (either making it an error, or using the\n> usual last-one-wins to have \"before\" override \"after\" in this example).\n> \n> But we should keep \"--reverse --reverse\" working as before, as there is\n> no other way to countermand a previously-given reverse option, and\n> because it has always worked.\n\nWhat about a triple reverse? That would mean the original reverse choice\nis lost and it defaults to the historical \"after\", which I'm fine with,\nbut this will need some extra caveat in the documentation :')\n\n> Usually we'd spell the option \"--no-reverse\", and it probably makes\n> sense to add it (to override an earlier \"--reverse=after\"), but we'd\n> still want to keep \"--reverse --reverse\" working for historical\n> compatibility.\n\nYes, I will add a negated form as well.\n\n> So combined with the earlier suggestions for using an enum and\n> disallowing the un-stuck \"--reverse after\" form, we probably want\n> something like (totally untested):\n> \n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..89a58a65b7 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2686,7 +2686,20 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t\t\tgit_log_output_encoding = xstrdup(\"\");\n> \t\treturn argcount;\n> \t} else if (!strcmp(arg, \"--reverse\")) {\n> -\t\trevs->reverse ^= 1;\n> +\t\t/*\n> +\t\t * This relies on \"do not reverse\" being the 0 value for our\n> +\t\t * enum, and historical \"reverse after\" having value 1.\n> +\t\t */\n> +\t\trevs->reverse = !revs->reverse;\n> +\t} else if (!strcmp(arg, \"--no-reverse\")) {\n> +\t\trevs->reverse = 0;\n> +\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n> +\t\tif (!strcmp(optarg, \"after\"))\n> +\t\t\trevs->reverse = REVS_REVERSE_AFTER;\n> +\t\telse if (!strcmp(optarg, \"before\"))\n> +\t\t\trevs->reverse = REVS_REVERSE_BEFORE;\n> +\t\telse\n> +\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n> \t} else if (!strcmp(arg, \"--children\")) {\n> \t\trevs->children.name = \"children\";\n> \t\trevs->limited = 1;\n\nThis unfortunately wouldn't work as the first condition is a prefix of\nthe third, so no free copy-paste for me.\n\nWill have separate parsing for omitted and explicit forms in v2.\n\n> Note that your original also allowed --reverse-o-matic, which we\n> probably don't want (and is fixed here).\n> \n> I _think_ the negation from using \"--reverse\" after \"--reverse=before\"\n> should be sensible here. And \"--reverse=\" with two different modes just\n> overrides rather than trying to be clever. But you may want to\n> double-check all of the combinations.\n> \n> This would all be much easier if revision.c used parse-options, of\n> course, which has all of these sorts of rules baked-in. But that's a\n> much bigger conversion, and probably not something you want to make a\n> prerequisite for your series. ;)\n\nI'm sure someone will be fed up enough to bring in parse-options at some\npoint.\n\n> -Peff\n\nThank you\n"},{"id":"541963","messageId":"aeX_3tJicFsmDfCX@exploit","threadId":"65511","inReplyTo":"aeXxC8eR0Mn3dGEn@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-20T10:30:15Z","receivedAt":"2026-04-20T10:30:19Z","isPatch":true,"body":"On Mon, Apr 20, 2026 at 11:33:25AM +0200, Mirko Faina wrote:\n> > diff --git a/revision.c b/revision.c\n> > index 599b3a66c3..89a58a65b7 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -2686,7 +2686,20 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> > \t\t\tgit_log_output_encoding = xstrdup(\"\");\n> > \t\treturn argcount;\n> > \t} else if (!strcmp(arg, \"--reverse\")) {\n> > -\t\trevs->reverse ^= 1;\n> > +\t\t/*\n> > +\t\t * This relies on \"do not reverse\" being the 0 value for our\n> > +\t\t * enum, and historical \"reverse after\" having value 1.\n> > +\t\t */\n> > +\t\trevs->reverse = !revs->reverse;\n> > +\t} else if (!strcmp(arg, \"--no-reverse\")) {\n> > +\t\trevs->reverse = 0;\n> > +\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n> > +\t\tif (!strcmp(optarg, \"after\"))\n> > +\t\t\trevs->reverse = REVS_REVERSE_AFTER;\n> > +\t\telse if (!strcmp(optarg, \"before\"))\n> > +\t\t\trevs->reverse = REVS_REVERSE_BEFORE;\n> > +\t\telse\n> > +\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n> > \t} else if (!strcmp(arg, \"--children\")) {\n> > \t\trevs->children.name = \"children\";\n> > \t\trevs->limited = 1;\n> \n> This unfortunately wouldn't work as the first condition is a prefix of\n> the third, so no free copy-paste for me.\n> \n> Will have separate parsing for omitted and explicit forms in v2.\n\nJust realized it's a strcmp and not start_with, so this should work\nfine.\n\nThank you\n"},{"id":"541982","messageId":"xmqqv7dlr4yz.fsf@gitster.g","threadId":"65511","inReplyTo":"fbea5f1c-946b-400e-a9a2-2c6d7b088d46@malon.dev","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-20T16:06:44Z","receivedAt":"2026-04-20T16:06:47Z","isPatch":true,"body":"Tian Yuchen writes:\n\n> I think the space complexity here could be reduced a little. After all, \n> since we’re only retrieving a few commits, there’s no need to load the \n> entire reversed commit history into memory.\n\nDoes \"we're only retrieving a few commits\" come from the fact that\nthe command example is \"log --reverse -3\"? \n\n - What should happen when you give \"git log --reverse=before\"\n without \"--max-count=3\"?\n\n - What should happen without \"--max-count\" but other limiting\n options, like \"--author=Tian\" or \"--min-parents=2\"?\n\nIt might be that the right way to look at this new feature is not\nthat \"we are changing where reverse is applied\", but \"count limit is\napplied much later than usual\", which may mean at the UI level, it\nmay not be good at the conceptual level to sell this as an extension\nto the \"--reverse\" option? I dunno.\n"},{"id":"541987","messageId":"e071f152-c718-4680-ad15-769591080ac8@malon.dev","threadId":"65511","inReplyTo":"xmqqv7dlr4yz.fsf@gitster.g","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-04-20T17:08:52Z","receivedAt":"2026-04-20T17:09:02Z","isPatch":true,"body":"On 4/21/26 00:06, Junio C Hamano wrote:\n> Tian Yuchen writes:\n> \n>> I think the space complexity here could be reduced a little. After all,\n>> since we’re only retrieving a few commits, there’s no need to load the\n>> entire reversed commit history into memory.\n> \n> Does \"we're only retrieving a few commits\" come from the fact that\n> the command example is \"log --reverse -3\"?\n> \n> - What should happen when you give \"git log --reverse=before\"\n> without \"--max-count=3\"?\n\nI still wanna talk my way out of it ;)\n\nIf we maintain a window of length K out of N nodes:\n\n - If K is small (e.g. 3), The space complexity is strictly O(K), \nwhich saves a considerable amount compared to the unoptimised one;\n\n - If K is huge, reduce to traversing all N nodes. At this point, the \nspace complexity is the same as when unoptimised (O(N)). At most, there \nis an additional time cost of O(log N) for stack operation, which can be \ndisregarded.\n\nIt seems like a strategy that could be applied consistently, but...\n\n> - What should happen without \"--max-count\" but other limiting\n> options, like \"--author=Tian\" or \"--min-parents=2\"?\n\nI must admit you’re absolutely right on this point.\n\nIf we were to introduce a new state machine and data structure for every \ntype of parameter, it would indeed be rather cumbersome to maintain and \nnot particularly cost-effective.\n\n> It might be that the right way to look at this new feature is not\n> that \"we are changing where reverse is applied\", but \"count limit is\n> applied much later than usual\", which may mean at the UI level, it\n> may not be good at the conceptual level to sell this as an extension\n> to the \"--reverse\" option? I dunno.\n\nThat's what I meant to say earlier in the 'nit' section. I couldn’t \nquite put it into words at that time :((\n\nThanks, Yuchen\n\n"},{"id":"541998","messageId":"aea3Kun-XxGJMesz@exploit","threadId":"65511","inReplyTo":"xmqqv7dlr4yz.fsf@gitster.g","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-20T23:50:32Z","receivedAt":"2026-04-20T23:50:42Z","isPatch":true,"body":"On Mon, Apr 20, 2026 at 09:06:44AM -0700, Junio C Hamano wrote:\n> Tian Yuchen writes:\n> \n> > I think the space complexity here could be reduced a little. After all, \n> > since we’re only retrieving a few commits, there’s no need to load the \n> > entire reversed commit history into memory.\n> \n> Does \"we're only retrieving a few commits\" come from the fact that\n> the command example is \"log --reverse -3\"? \n> \n> - What should happen when you give \"git log --reverse=before\"\n> without \"--max-count=3\"?\n> \n> - What should happen without \"--max-count\" but other limiting\n> options, like \"--author=Tian\" or \"--min-parents=2\"?\n> \n> It might be that the right way to look at this new feature is not\n> that \"we are changing where reverse is applied\", but \"count limit is\n> applied much later than usual\", which may mean at the UI level, it\n> may not be good at the conceptual level to sell this as an extension\n> to the \"--reverse\" option? I dunno.\n\nI'm not sure about that. The way max_count actually interacts with\n--reverse in the code is an implementation detail that the user doesn't\nneed to worry about. It should be fine to tell the user that this\nfeature is an extention of how --reverse behaves. Regarding \"Commit\nLimiting options\", we already tell the user\n\n\tNote that these are applied before commit ordering and formatting\n\toptions, such as --reverse.\n\nso explaining to the user that this new feature acts on reverse's\nbehaviour might be easier (and not necessarily wrong on a conceptual\nlevel). I find \"you can choose to apply reverse before any commit\nlimiting option\" easier to understand than \"--max-count can be applied\nlast, or before reverse but after all other Commit Limiting options\".\n"},{"id":"542006","messageId":"20260421034816.GA1883014@coredump.intra.peff.net","threadId":"65511","inReplyTo":"aeXxC8eR0Mn3dGEn@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-21T03:48:16Z","receivedAt":"2026-04-21T03:48:18Z","isPatch":true,"body":"On Mon, Apr 20, 2026 at 11:33:25AM +0200, Mirko Faina wrote:\n\n> > But we should keep \"--reverse --reverse\" working as before, as there is\n> > no other way to countermand a previously-given reverse option, and\n> > because it has always worked.\n> \n> What about a triple reverse? That would mean the original reverse choice\n> is lost and it defaults to the historical \"after\", which I'm fine with,\n> but this will need some extra caveat in the documentation :')\n\nIf \"--reverse\" means \"reverse after\", then:\n\n --reverse=before --reverse --reverse\n\nis back to reversing after. You could also make it retain before/after\nif you stored that as a separate bit. I.e.,:\n\n reverse=before:\n revs->reverse = 1;\n revs->reverse_when = REVERSE_BEFORE;\n reverse=after:\n revs->reverse = 1;\n revs->reverse_when = REVERSE_AFTER;\n reverse:\n revs->reverse ^= 1; /* flip reversing */\n /* do not touch reverse_when! */\n\nAnd then the triple-reverse takes you back to reverse=before. I'm not\nsure if that is more or less confusing, though. ;)\n\nAt any rate, I agree that the behavior should be mentioned in the docs,\nespecially since \"--reverse\" is not a true synonym for \"--reverse=after\"\nbecause of the override vs negation behavior.\n\n-Peff\n"},{"id":"542088","messageId":"20260422002840.303477-4-mroik@delayed.space","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"[PATCH v2 0/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T00:28:39Z","receivedAt":"2026-04-22T00:29:19Z","isPatch":true,"body":"Since v1 I've:\n\t* removed the non stuck form for the --reverse option\n\t* --reverse with no argument now flips \"no reverse -> reverse\n\t after\", \"reverse after -> no reverse\" and \"reverse before ->\n\t no reverse\"\n\t* implemented a window to reduce memory usage when\n\t --reverse=before with --max-count=\n\t* updated the docs to highlight the peculiarities of --reverse\n\t when it's specified multiple times\n\n[1/2] revision.c: implement --reverse=before for walks (Mirko Faina)\n[2/2] revision.c: reduce memory usage on reverse before (Mirko Faina)\n\n Documentation/rev-list-options.adoc | 14 +++--\n revision.c | 85 +++++++++++++++++++++++++++--\n revision.h | 8 ++-\n t/t4202-log.sh | 66 ++++++++++++++++++++++\n 4 files changed, 163 insertions(+), 10 deletions(-)\n\n\nbase-commit: e8955061076952cc5eab0300424fc48b601fe12d\n-- \n2.54.0\n\n"},{"id":"542089","messageId":"20260422002840.303477-6-mroik@delayed.space","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"[PATCH v2 2/2] revision.c: reduce memory usage on reverse before","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T00:28:41Z","receivedAt":"2026-04-22T00:29:19Z","isPatch":true,"body":"Due to the nature of --reverse=before we have to walk all of the history\nand store each non-filtered processed commit, this can be expensive on\nmemory for projects with a long history. When --max-count is being used\nwe don't really have to keep every processed commit, we can discard\nolder commits (as in have been processed before than the ones we're now\nconsidering, from a chronological commit order they are the newer\ncommits) as we surpass the --max-count limit.\n\nTeach get_revision() to keep only the newer commits as we walk a\nrevision with --reverse=before and --max-count=. We do this through a\nsimple queue. With N nodes and K as the --max-count argument, assuming K\n< N, we go from a space complexity of O(N) to O(K). When it comes down\nto time complexity, the queue has an ammortized time of O(1) for pops,\nso the complexity remains O(N).\n\nSigned-off-by: Mirko Faina \n---\n revision.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 52 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex d581f5e38e..4730f21ea6 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -4530,6 +4530,52 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_with_window(struct rev_info *revs, int max_count,\n+\t\t\t struct commit_list **reversed)\n+{\n+\tstruct commit *c;\n+\tstruct commit_list *into_queue = NULL;\n+\tstruct commit_list *outo_queue = NULL;\n+\tint into_count = 0;\n+\tint outo_count = 0;\n+\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\tcommit_list_insert(c, &into_queue);\n+\t\tinto_count++;\n+\t\tif (into_count + outo_count > max_count) {\n+\t\t\tif (!outo_count) {\n+\t\t\t\twhile (into_count) {\n+\t\t\t\t\tc = pop_commit(&into_queue);\n+\t\t\t\t\tinto_count--;\n+\t\t\t\t\tcommit_list_insert(c, &outo_queue);\n+\t\t\t\t\touto_count++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tpop_commit(&outo_queue);\n+\t\t\touto_count--;\n+\t\t}\n+\t}\n+\n+\twhile (outo_count) {\n+\t\tc = pop_commit(&outo_queue);\n+\t\touto_count--;\n+\t\tcommit_list_insert(c, reversed);\n+\t}\n+\n+\twhile (into_count) {\n+\t\tc = pop_commit(&into_queue);\n+\t\tinto_count--;\n+\t\tcommit_list_insert(c, &outo_queue);\n+\t\touto_count++;\n+\t}\n+\n+\twhile (outo_count) {\n+\t\tc = pop_commit(&outo_queue);\n+\t\touto_count--;\n+\t\tcommit_list_insert(c, reversed);\n+\t}\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n@@ -4546,8 +4592,12 @@ struct commit *get_revision(struct rev_info *revs)\n \t\t\trevs->max_count = -1;\n \n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->reverse == REVERSE_BEFORE && max_count != -1) {\n+\t\t\tretrieve_with_window(revs, max_count, &reversed);\n+\t\t} else {\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\t}\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse_output_stage = 1;\n-- \n2.54.0\n\n"},{"id":"542090","messageId":"20260422002840.303477-5-mroik@delayed.space","threadId":"65511","inReplyTo":"20260418164736.2367523-2-mroik@delayed.space","subject":"[PATCH v2 1/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T00:28:40Z","receivedAt":"2026-04-22T00:29:19Z","isPatch":true,"body":"In a revision walk `--reverse` can only be applied after any commit\nlimiting option. This makes getting a limited amount of commits from the\ntail impossible. E.g.\n\n git log --reverse --max-count=3\n\nSome would expect this to give back the first 3 commits of the project.\nInstead it returns the last 3 but in reversed order.\n\nTeach `get_revision()` to accpet an argument `(after|before)` from the\nCLI, and apply the reversal before or after the commit limiting options\nbased on this argument. If no argument is provided default to the\ncurrent behaviour, applying `--reverse` after the commit limiting\noptions.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 14 ++++--\n revision.c | 31 ++++++++++++--\n revision.h | 8 +++-\n t/t4202-log.sh | 66 +++++++++++++++++++++++++++++\n 4 files changed, 111 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..7244e85108 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -914,10 +914,16 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n avoid showing the commits from two parallel development track mixed\n together.\n \n-`--reverse`::\n-\tOutput the commits chosen to be shown (see 'Commit Limiting'\n-\tsection above) in reverse order. Cannot be combined with\n-\t`--walk-reflogs`.\n+`--[no-]reverse[=(after|before)]`::\n+\tAccepts `after` or `before`. Cannot be combined with\n+\t`--walk-reflogs`. If `after`, output the commits chosen to be\n+\tshown (see 'Commit Limiting' section above) in reverse order. If\n+\t`before`, reverse the commits before filtering with `Commit\n+\tLimiting` options. This option can be used multiple times, last\n+\tone is applied. When the argument for `--reverse` is omitted, if\n+\tthe current state is in no reverse, it defaults to `after`. If\n+\tit is in any reversed state, it restores the original ordering\n+\tby removing the reverse state.\n endif::git-shortlog[]\n \n ifndef::git-shortlog[]\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..d581f5e38e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2686,7 +2686,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\tgit_log_output_encoding = xstrdup(\"\");\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n-\t\trevs->reverse ^= 1;\n+\t\trevs->reverse = !revs->reverse;\n+\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n+\t\tif (!strcmp(optarg, \"after\"))\n+\t\t\trevs->reverse = REVERSE_AFTER;\n+\t\telse if(!strcmp(optarg, \"before\"))\n+\t\t\trevs->reverse = REVERSE_BEFORE;\n+\t\telse\n+\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n+\t} else if (!strcmp(arg, \"--no-reverse\")) {\n+\t\trevs->reverse = NO_REVERSE;\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n@@ -4525,19 +4534,35 @@ struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tint max_count = revs->max_count;\n+\n+\tif (revs->reverse && !revs->reverse_output_stage) {\n+\t\tif (revs->reverse == 3) {\n+\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n+\t\t\trevs->reverse = 1;\n+\t\t}\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = -1;\n \n-\tif (revs->reverse) {\n \t\treversed = NULL;\n \t\twhile ((c = get_revision_internal(revs)))\n \t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n-\t\trevs->reverse = 0;\n \t\trevs->reverse_output_stage = 1;\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = max_count;\n \t}\n \n \tif (revs->reverse_output_stage) {\n+\t\tif (revs->reverse == REVERSE_BEFORE && revs->max_count == 0)\n+\t\t\treturn NULL;\n+\n \t\tc = pop_commit(&revs->commits);\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count--;\n \t\tif (revs->track_linear)\n \t\t\trevs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n \t\treturn c;\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..02881577dc 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,6 +121,12 @@ struct ref_exclusions {\n struct oidset;\n struct topo_walk_info;\n \n+enum rev_reverse {\n+\tNO_REVERSE = 0,\n+\tREVERSE_AFTER = 1,\n+\tREVERSE_BEFORE = 2,\n+};\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -167,6 +173,7 @@ struct rev_info {\n \t\t\tignore_missing_links:1;\n \n \t/* Traversal flags */\n+\tenum rev_reverse reverse:2;\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n \t\t\tno_walk:1,\n@@ -196,7 +203,6 @@ struct rev_info {\n \t\t\trewrite_parents:1,\n \t\t\tprint_parents:1,\n \t\t\tshow_decorations:1,\n-\t\t\treverse:1,\n \t\t\treverse_output_stage:1,\n \t\t\tcherry_pick:1,\n \t\t\tcherry_mark:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..3bfe2c99b8 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+cat >expect <<-\\EOF\n+c3f451c Merge tag 'reach'\n+046b221 to remove\n+EOF\n+\n+test_expect_success 'log --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse=after --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse=after --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+3a2fdcb initial\n+f7dab8e second\n+EOF\n+\n+test_expect_success 'log --reverse=before --oneline --max-count=2' '\n+\ttest_when_finished rm actual &&\n+\tgit log --reverse=before --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+046b221 to remove\n+c3f451c Merge tag 'reach'\n+EOF\n+\n+test_expect_success 'log --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --no-reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"542091","messageId":"aegWh9h3V74o7A96@exploit","threadId":"65511","inReplyTo":"20260422002840.303477-4-mroik@delayed.space","subject":"Re: [PATCH v2 0/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T00:30:20Z","receivedAt":"2026-04-22T00:30:23Z","isPatch":true,"body":"Sorry, I forgot to thread it\n"},{"id":"542152","messageId":"CALnO6CAjMAZhBk_WXW1wbKk1kpQScFtbY0R+mCxHTFB7=CcEDg@mail.gmail.com","threadId":"65511","inReplyTo":"aeUqSltEWIWaPDh3@exploit","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-04-22T18:24:48Z","receivedAt":"2026-04-22T18:25:00Z","isPatch":true,"body":"On Sun, Apr 19, 2026 at 4:31 PM Mirko Faina wrote:\n> > > > > if (revs->reverse_output_stage) {\n> > > > > + if (revs->reverse == 2 && revs->max_count == 0)\n> > > > > + return NULL;\n> > > > > +\n> >\n> > PS: something I spotted on a second read. [Ignoring reverse=after\n> > mode] This hunk looks to me like a nice little optimization (return\n> > nothing if we know max_count says we yield no commits). Of course, I\n> > could see that being viable early in the function, right? When asking\n> > get_revision for commits, if max_count is 0, just return NULL.\n> >\n> > For reverse=after mode, this condition is only true if the max_count\n> > was 0 in the previous conditional, also, since we use max_count=-1\n> > before iterating get_revision_internal. That means the original\n> > max_count isn't touched. At any rate, it _seems_ to me that the whole\n> > function could benefit from this optimization… but I wonder if it is\n> > _necessary_ for correctness of reverse=after in some way that I'm not\n> > seeing? Since the current version doesn't need the early bailout, why\n> > does reverse=after?\n>\n> Just to clarify, \"reverse = 2\" is \"--reverse=before\" and not\n> \"--reverse=after\".\n\nOh golly, sorry about that!\n\n> With \"reverse = 2\", the snippet of code you're referencing is not an\n> optimization but a requirement for correctness. With \"reverse = 1\" we\n> just keep the max_count as is and it's used by get_revision_internal()\n> to stop if that limit is reached. What we find in 'reversed' are already\n> just the commits we need to return.\n>\n> With \"reverse = 2\", we first set max_count to -1 and then retrieve the\n> whole history, then we set max_count to its original value. Then we\n> return the commits on each call of get_revision(). Now, unlike with\n> \"reverse = 1\", we have the whole history in 'reversed', because of that\n> we need to know when to stop. That's the reason we decrement max_count\n> only for \"reverse = 2\" and why \"max_count == 0\" is checked only for\n> \"reverse = 2\".\n\nOk, this explanation hasn't yet clicked…\n\n> > > > > c = pop_commit(&revs->commits);\n> > > > > + if (revs->reverse == 2)\n> > > > > + revs->max_count--;\n> > > >\n> > > > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.\n> > >\n> > > get_revision() (in revision.c) handles the reverse option and updates\n> > > the \"struct git_graph\". get_revision() then calls\n> > > get_revision_internal(), which handles commit boundaries and max_count,\n> > > here is where it gets decreased. Since max_count gets decreased\n> > > everytime get_revision_internal() is called, if we were to leave\n> > > max_count as is before the walk (in get_revision() at line 4558), the\n> > > walk would stop before reaching the root commit. This is why the current\n> > > --reverse option is applied only after commit limiting options. So\n> > > instead we set max_count at -1 walking the whole history and storing it\n> > > in 'reversed'. Now we're in \"reverse_output_stage = 1\", and in this\n> > > state we never call get_revision_internal() again, instead we pop\n> > > commits from 'reversed'. Because of this we have to handle max_count\n> > > outside get_revision_internal(), so we decrement it in the snippet of\n> > > code you referenced.\n> > >\n> > > A bit verbose but hopefully it'll get my point across.\n> >\n> > I don't 100% follow, but I'm out of my depth :)\n> >\n> > I think I see that get_revision() effectively has 2 modes pertaining\n> > to reverse: reverse and reverse output stage (the former falls\n> > directly into the latter, though).\n> >\n> > After some setup, the reverse mode calls get_revision_internal() as\n> > you said. That decrements max_count as a way of counting how many\n> > commits we've seen through the loop, so if we asked for 5 we'd only\n> > process 5 commits.\n> >\n> > Then we fall into the output stage mode, which pops a commit [1].\n> >\n> > With this patch, in reverse=after we disable max_count in the first\n> > (reverse) mode, as you said. Ok: we get the whole (filtered) history\n> > then, at which point we can now shrink. That makes sense.\n> >\n> > Then in the reverse output stage mode, we pretend to have one less\n> > max_count. That's what I can't figure out. Is it because of the\n> > pop_commit()? I guess I'm not totally seeing how that interacted with\n> > the max_count in the original code: does the current code yield one\n> > extra commit in get_revision_internal() ?\n>\n> I'm not sure I understand what you're referencing with \"Then in the\n> reverse output stage mode, we pretend to have one less max_count\".\n>\n> If you're referring to line 4573, then...\n>\n> > You wrote that \"we never call get_revision_internal() again,\" but I\n> > don't see why that's true with this patch and not true before it.\n> >\n> > I do agree that _somebody_ has to handle max_count after\n> > get_revision() returns with reverse=after. I'm just not sure what\n> >\n> > if (revs->reverse == 2)\n> > revs->max_count--;\n> >\n> > is doing.\n>\n> ...we're not pretending we have fewer commits. Every subsequent call to\n> get_revision() after the first call will never enter the branch at line\n> 4548 and will only enter the branch at 4568. Everytime we pop a commit\n> from 'reversed' we decrease max_count so we can limit only to the amount\n> of commits the user wants.\n>\n> So, to recap, with \"reverse = 2\", on the first call to get_revision() we\n> walk the whole history and store it in 'reversed' in reversed order and\n> return the first commit.\n> On subsequent calls to get_revision() we do not walk the history again,\n> we simply return the commits that have been stored in 'reversed'.\n> Everytime we pop a commit we have to decrease max_count, and we check\n> againts max_count to know if we shouldn't return anymore commits (by\n> returning NULL).\n\n…but I think this one does. I think what I missed is that in all\n\"reverse\" modes, get_revision() does some pre-computation and then\nyields one at a time the commits. In traditional \"after\" mode, the\ncounting is done by get_revision_internal() [before reversal]. In the\nnew mode, get_revision takes on that responsibility of\nget_revision_internal instead.\n\nHm. That suggests to me that get_revision's responsibilities are\nbecoming complex. Might be worth some version of a refactor, but idk\nwhich.\n\n> > Of course if I'm the only one confused and others make sense of it,\n> > that's ok, too.\n>\n> No, I completely understand. I did have to retouch the function a few\n> times after writing the tests :P\n>\n> > [1]: I traced this to 498bcd3159 (rev-list: fix --reverse interaction\n> > with --parents, 2008-08-29), but I can't fathom what the pop is doing\n> > there.\n>\n> It's pretty much doing the same thing it does now, it's returning stored\n> commits. In both versions, the initial setup when \"revs->reverse\" is\n> true, becomes \"dead code\" after the first call.\n\nAnd this pop makes more sense now, too. Phew!\n-- \nD. Ben Knoble\n"},{"id":"542158","messageId":"aekjhDIUH_joIH0b@exploit","threadId":"65511","inReplyTo":"CALnO6CAjMAZhBk_WXW1wbKk1kpQScFtbY0R+mCxHTFB7=CcEDg@mail.gmail.com","subject":"Re: [PATCH] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T19:42:54Z","receivedAt":"2026-04-22T19:42:58Z","isPatch":true,"body":"On Wed, Apr 22, 2026 at 02:24:48PM -0400, D. Ben Knoble wrote:\n> > > > > > c = pop_commit(&revs->commits);\n> > > > > > + if (revs->reverse == 2)\n> > > > > > + revs->max_count--;\n> > > > >\n> > > > > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.\n> > > >\n> > > > get_revision() (in revision.c) handles the reverse option and updates\n> > > > the \"struct git_graph\". get_revision() then calls\n> > > > get_revision_internal(), which handles commit boundaries and max_count,\n> > > > here is where it gets decreased. Since max_count gets decreased\n> > > > everytime get_revision_internal() is called, if we were to leave\n> > > > max_count as is before the walk (in get_revision() at line 4558), the\n> > > > walk would stop before reaching the root commit. This is why the current\n> > > > --reverse option is applied only after commit limiting options. So\n> > > > instead we set max_count at -1 walking the whole history and storing it\n> > > > in 'reversed'. Now we're in \"reverse_output_stage = 1\", and in this\n> > > > state we never call get_revision_internal() again, instead we pop\n> > > > commits from 'reversed'. Because of this we have to handle max_count\n> > > > outside get_revision_internal(), so we decrement it in the snippet of\n> > > > code you referenced.\n> > > >\n> > > > A bit verbose but hopefully it'll get my point across.\n> > >\n> > > I don't 100% follow, but I'm out of my depth :)\n> > >\n> > > I think I see that get_revision() effectively has 2 modes pertaining\n> > > to reverse: reverse and reverse output stage (the former falls\n> > > directly into the latter, though).\n> > >\n> > > After some setup, the reverse mode calls get_revision_internal() as\n> > > you said. That decrements max_count as a way of counting how many\n> > > commits we've seen through the loop, so if we asked for 5 we'd only\n> > > process 5 commits.\n> > >\n> > > Then we fall into the output stage mode, which pops a commit [1].\n> > >\n> > > With this patch, in reverse=after we disable max_count in the first\n> > > (reverse) mode, as you said. Ok: we get the whole (filtered) history\n> > > then, at which point we can now shrink. That makes sense.\n> > >\n> > > Then in the reverse output stage mode, we pretend to have one less\n> > > max_count. That's what I can't figure out. Is it because of the\n> > > pop_commit()? I guess I'm not totally seeing how that interacted with\n> > > the max_count in the original code: does the current code yield one\n> > > extra commit in get_revision_internal() ?\n> >\n> > I'm not sure I understand what you're referencing with \"Then in the\n> > reverse output stage mode, we pretend to have one less max_count\".\n> >\n> > If you're referring to line 4573, then...\n> >\n> > > You wrote that \"we never call get_revision_internal() again,\" but I\n> > > don't see why that's true with this patch and not true before it.\n> > >\n> > > I do agree that _somebody_ has to handle max_count after\n> > > get_revision() returns with reverse=after. I'm just not sure what\n> > >\n> > > if (revs->reverse == 2)\n> > > revs->max_count--;\n> > >\n> > > is doing.\n> >\n> > ...we're not pretending we have fewer commits. Every subsequent call to\n> > get_revision() after the first call will never enter the branch at line\n> > 4548 and will only enter the branch at 4568. Everytime we pop a commit\n> > from 'reversed' we decrease max_count so we can limit only to the amount\n> > of commits the user wants.\n> >\n> > So, to recap, with \"reverse = 2\", on the first call to get_revision() we\n> > walk the whole history and store it in 'reversed' in reversed order and\n> > return the first commit.\n> > On subsequent calls to get_revision() we do not walk the history again,\n> > we simply return the commits that have been stored in 'reversed'.\n> > Everytime we pop a commit we have to decrease max_count, and we check\n> > againts max_count to know if we shouldn't return anymore commits (by\n> > returning NULL).\n> \n> …but I think this one does. I think what I missed is that in all\n> \"reverse\" modes, get_revision() does some pre-computation and then\n> yields one at a time the commits. In traditional \"after\" mode, the\n> counting is done by get_revision_internal() [before reversal]. In the\n> new mode, get_revision takes on that responsibility of\n> get_revision_internal instead.\n> \n> Hm. That suggests to me that get_revision's responsibilities are\n> becoming complex. Might be worth some version of a refactor, but idk\n> which.\n\nIf the refactor is to yield the responsibility of max_count to\nget_revision_internal() for the new reverse mode too, then I'm not sure\nif we want that. To maintain state across calls to\nget_revision_internal() we'd have to store the temporary max_count that\ncurrently lives in get_revision() in rev_info. I don't want to add\nunnecessary new fields to the struct when its space is so valuable\n(afterall we're using bitpacking).\n\nIn the current implementation all of the reversal happens in the same\ncall to get_revision(), so state is preserved across multiple\nget_revision_internal() calls through the temporary int max_count.\n"},{"id":"542161","messageId":"20260422224442.GB110382@coredump.intra.peff.net","threadId":"65511","inReplyTo":"20260422002840.303477-5-mroik@delayed.space","subject":"Re: [PATCH v2 1/2] revision.c: implement --reverse=before for walks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-04-22T22:44:42Z","receivedAt":"2026-04-22T22:44:44Z","isPatch":true,"body":"On Wed, Apr 22, 2026 at 02:28:40AM +0200, Mirko Faina wrote:\n\n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..7244e85108 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -914,10 +914,16 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n> avoid showing the commits from two parallel development track mixed\n> together.\n> \n> -`--reverse`::\n> -\tOutput the commits chosen to be shown (see 'Commit Limiting'\n> -\tsection above) in reverse order. Cannot be combined with\n> -\t`--walk-reflogs`.\n> +`--[no-]reverse[=(after|before)]`::\n> +\tAccepts `after` or `before`. Cannot be combined with\n> +\t`--walk-reflogs`. If `after`, output the commits chosen to be\n> +\tshown (see 'Commit Limiting' section above) in reverse order. If\n> +\t`before`, reverse the commits before filtering with `Commit\n> +\tLimiting` options. This option can be used multiple times, last\n> +\tone is applied. When the argument for `--reverse` is omitted, if\n> +\tthe current state is in no reverse, it defaults to `after`. If\n> +\tit is in any reversed state, it restores the original ordering\n> +\tby removing the reverse state.\n\nI think this is all correct, but I found the final sentences a bit hard\nto follow (especially the phrase \"reversed state\"). Let me take a stab\nat it.\n\n When multiple `--reverse=` options are given, the final option\n overrides any previous options. The `--reverse` option (with no\n specifier) behaves as `--reverse=after`, except that for historical\n reasons it negates any previous reversed state (so `--reverse\n --reverse` does nothing, nor does `--reverse=before --reverse`).\n\nI dunno if that is any better, really. I hoped by mentioning \"historical\nreasons\" that excuses any weirdness, and readers interested in sane\nbehavior can stop reading. ;)\n\nSo anyway, I offer it in case you find it useful or can pick out useful\nbits from it.\n\n-Peff\n"},{"id":"542162","messageId":"aelQm3cznMc1EkQG@exploit","threadId":"65511","inReplyTo":"20260422224442.GB110382@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-22T22:53:54Z","receivedAt":"2026-04-22T22:53:59Z","isPatch":true,"body":"On Wed, Apr 22, 2026 at 06:44:42PM -0400, Jeff King wrote:\n> On Wed, Apr 22, 2026 at 02:28:40AM +0200, Mirko Faina wrote:\n> \n> > diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> > index 2d195a1474..7244e85108 100644\n> > --- a/Documentation/rev-list-options.adoc\n> > +++ b/Documentation/rev-list-options.adoc\n> > @@ -914,10 +914,16 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n> > avoid showing the commits from two parallel development track mixed\n> > together.\n> > \n> > -`--reverse`::\n> > -\tOutput the commits chosen to be shown (see 'Commit Limiting'\n> > -\tsection above) in reverse order. Cannot be combined with\n> > -\t`--walk-reflogs`.\n> > +`--[no-]reverse[=(after|before)]`::\n> > +\tAccepts `after` or `before`. Cannot be combined with\n> > +\t`--walk-reflogs`. If `after`, output the commits chosen to be\n> > +\tshown (see 'Commit Limiting' section above) in reverse order. If\n> > +\t`before`, reverse the commits before filtering with `Commit\n> > +\tLimiting` options. This option can be used multiple times, last\n> > +\tone is applied. When the argument for `--reverse` is omitted, if\n> > +\tthe current state is in no reverse, it defaults to `after`. If\n> > +\tit is in any reversed state, it restores the original ordering\n> > +\tby removing the reverse state.\n> \n> I think this is all correct, but I found the final sentences a bit hard\n> to follow (especially the phrase \"reversed state\"). Let me take a stab\n> at it.\n> \n> When multiple `--reverse=` options are given, the final option\n> overrides any previous options. The `--reverse` option (with no\n> specifier) behaves as `--reverse=after`, except that for historical\n> reasons it negates any previous reversed state (so `--reverse\n> --reverse` does nothing, nor does `--reverse=before --reverse`).\n> \n> I dunno if that is any better, really. I hoped by mentioning \"historical\n> reasons\" that excuses any weirdness, and readers interested in sane\n> behavior can stop reading. ;)\n> \n> So anyway, I offer it in case you find it useful or can pick out useful\n> bits from it.\n\nYes, it does read better, thank you.\n\nI'd personally put a \"truth table\" but that would be very verbose, and I\nsuspect most would not find it useful :D\n\nThank you\n"},{"id":"542232","messageId":"00489b0e5282028292a6a2f582b9bef3f8a50041.1776984666.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1776984666.git.mroik@delayed.space","subject":"[PATCH v3 2/2] revision.c: reduce memory usage on reverse before","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-23T22:52:00Z","receivedAt":"2026-04-23T22:52:32Z","isPatch":true,"body":"Due to the nature of --reverse=before we have to walk all of the history\nand store each non-filtered processed commit, this can be expensive on\nmemory for projects with a long history. When --max-count is being used\nwe don't really have to keep every processed commit, we can discard\nolder commits (as in have been processed before than the ones we're now\nconsidering, from a chronological commit order they are the newer\ncommits) as we surpass the --max-count limit.\n\nTeach get_revision() to keep only the newer commits as we walk a\nrevision with --reverse=before and --max-count=. We do this through a\nsimple queue. With N nodes and K as the --max-count argument, assuming K\n< N, we go from a space complexity of O(N) to O(K). When it comes down\nto time complexity, the queue has an ammortized time of O(1) for pops,\nso the complexity remains O(N).\n\nSigned-off-by: Mirko Faina \n---\n revision.c | 42 ++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex d581f5e38e..03acff9bac 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -4530,6 +4530,40 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_with_window(struct rev_info *revs, int max_count,\n+\t\t\t \t struct commit_list **reversed)\n+{\n+\tstruct commit *c;\n+\tstruct commit_list *into_queue = NULL;\n+\tstruct commit_list *outo_queue = NULL;\n+\tint into_count = 0;\n+\tint outo_count = 0;\n+\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\tcommit_list_insert(c, &into_queue);\n+\t\tinto_count++;\n+\t\tif (into_count + outo_count > max_count) {\n+\t\t\tif (!outo_count) {\n+\t\t\t\twhile (into_count) {\n+\t\t\t\t\tc = pop_commit(&into_queue);\n+\t\t\t\t\tinto_count--;\n+\t\t\t\t\tcommit_list_insert(c, &outo_queue);\n+\t\t\t\t\touto_count++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tpop_commit(&outo_queue);\n+\t\t\touto_count--;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&outo_queue)))\n+\t\tcommit_list_insert(c, reversed);\n+\twhile ((c = pop_commit(&into_queue)))\n+\t\tcommit_list_insert(c, &outo_queue);\n+\twhile ((c = pop_commit(&outo_queue)))\n+\t\tcommit_list_insert(c, reversed);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n@@ -4546,8 +4580,12 @@ struct commit *get_revision(struct rev_info *revs)\n \t\t\trevs->max_count = -1;\n \n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->reverse == REVERSE_BEFORE && max_count != -1) {\n+\t\t\tretrieve_with_window(revs, max_count, &reversed);\n+\t\t} else {\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\t}\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse_output_stage = 1;\n-- \n2.54.0\n\n"},{"id":"542233","messageId":"4864ac46dd8ef4b704c29efc96c45f4e1412373b.1776984666.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1776984666.git.mroik@delayed.space","subject":"[PATCH v3 1/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-23T22:51:59Z","receivedAt":"2026-04-23T22:52:33Z","isPatch":true,"body":"In a revision walk `--reverse` can only be applied after any commit\nlimiting option. This makes getting a limited amount of commits from the\ntail impossible. E.g.\n\n git log --reverse --max-count=3\n\nSome would expect this to give back the first 3 commits of the project.\nInstead it returns the last 3 but in reversed order.\n\nTeach `get_revision()` to accpet an argument `(after|before)` from the\nCLI, and apply the reversal before or after the commit limiting options\nbased on this argument. If no argument is provided default to the\ncurrent behaviour, applying `--reverse` after the commit limiting\noptions.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 16 +++++--\n revision.c | 31 ++++++++++++--\n revision.h | 8 +++-\n t/t4202-log.sh | 66 +++++++++++++++++++++++++++++\n 4 files changed, 113 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..e97f6f2aff 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -914,10 +914,18 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n avoid showing the commits from two parallel development track mixed\n together.\n \n-`--reverse`::\n-\tOutput the commits chosen to be shown (see 'Commit Limiting'\n-\tsection above) in reverse order. Cannot be combined with\n-\t`--walk-reflogs`.\n+`--[no-]reverse[=(after|before)]`::\n+\tAccepts `after` or `before`. Cannot be combined with\n+\t`--walk-reflogs`. If `after`, output the commits chosen to be\n+\tshown (see 'Commit Limiting' section above) in reverse order. If\n+\t`before`, reverse the commits before filtering with `Commit\n+\tLimiting` options. When multiple `--reverse=` options are given,\n+\tthe final option overrides any previous options. The `--reverse`\n+\toption (with no specifier) behaves as `--reverse=after`, except\n+\tthat, for historical reasons, it negates any previous reversed\n+\tstate (so `--reverse --reverse` does nothing, nor does\n+\t`--reverse=before --reverse`. Note that `--reverse=before\n+\t--reverse --reverse` is the same as `--reverse=after`).\n endif::git-shortlog[]\n \n ifndef::git-shortlog[]\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..d581f5e38e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2686,7 +2686,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\tgit_log_output_encoding = xstrdup(\"\");\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n-\t\trevs->reverse ^= 1;\n+\t\trevs->reverse = !revs->reverse;\n+\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n+\t\tif (!strcmp(optarg, \"after\"))\n+\t\t\trevs->reverse = REVERSE_AFTER;\n+\t\telse if(!strcmp(optarg, \"before\"))\n+\t\t\trevs->reverse = REVERSE_BEFORE;\n+\t\telse\n+\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n+\t} else if (!strcmp(arg, \"--no-reverse\")) {\n+\t\trevs->reverse = NO_REVERSE;\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n@@ -4525,19 +4534,35 @@ struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tint max_count = revs->max_count;\n+\n+\tif (revs->reverse && !revs->reverse_output_stage) {\n+\t\tif (revs->reverse == 3) {\n+\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n+\t\t\trevs->reverse = 1;\n+\t\t}\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = -1;\n \n-\tif (revs->reverse) {\n \t\treversed = NULL;\n \t\twhile ((c = get_revision_internal(revs)))\n \t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n-\t\trevs->reverse = 0;\n \t\trevs->reverse_output_stage = 1;\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = max_count;\n \t}\n \n \tif (revs->reverse_output_stage) {\n+\t\tif (revs->reverse == REVERSE_BEFORE && revs->max_count == 0)\n+\t\t\treturn NULL;\n+\n \t\tc = pop_commit(&revs->commits);\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count--;\n \t\tif (revs->track_linear)\n \t\t\trevs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n \t\treturn c;\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..02881577dc 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,6 +121,12 @@ struct ref_exclusions {\n struct oidset;\n struct topo_walk_info;\n \n+enum rev_reverse {\n+\tNO_REVERSE = 0,\n+\tREVERSE_AFTER = 1,\n+\tREVERSE_BEFORE = 2,\n+};\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -167,6 +173,7 @@ struct rev_info {\n \t\t\tignore_missing_links:1;\n \n \t/* Traversal flags */\n+\tenum rev_reverse reverse:2;\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n \t\t\tno_walk:1,\n@@ -196,7 +203,6 @@ struct rev_info {\n \t\t\trewrite_parents:1,\n \t\t\tprint_parents:1,\n \t\t\tshow_decorations:1,\n-\t\t\treverse:1,\n \t\t\treverse_output_stage:1,\n \t\t\tcherry_pick:1,\n \t\t\tcherry_mark:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..3bfe2c99b8 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+cat >expect <<-\\EOF\n+c3f451c Merge tag 'reach'\n+046b221 to remove\n+EOF\n+\n+test_expect_success 'log --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse=after --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse=after --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+3a2fdcb initial\n+f7dab8e second\n+EOF\n+\n+test_expect_success 'log --reverse=before --oneline --max-count=2' '\n+\ttest_when_finished rm actual &&\n+\tgit log --reverse=before --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+046b221 to remove\n+c3f451c Merge tag 'reach'\n+EOF\n+\n+test_expect_success 'log --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --no-reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"542234","messageId":"cover.1776984666.git.mroik@delayed.space","threadId":"65511","inReplyTo":"20260422002840.303477-4-mroik@delayed.space","subject":"[PATCH v3 0/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-23T22:51:58Z","receivedAt":"2026-04-23T22:52:36Z","isPatch":true,"body":"I've fixed the docs with the suggested changes by Jeff and applied some\nstyling fixes.\n\n[1/2] revision.c: implement --reverse=before for walks (Mirko Faina)\n[2/2] revision.c: reduce memory usage on reverse before (Mirko Faina)\n\n Documentation/rev-list-options.adoc | 16 +++++--\n revision.c | 73 +++++++++++++++++++++++++++--\n revision.h | 8 +++-\n t/t4202-log.sh | 66 ++++++++++++++++++++++++++\n 4 files changed, 153 insertions(+), 10 deletions(-)\n\nRange-diff against v2:\n1: 599a247d82 ! 1: 4864ac46dd revision.c: implement --reverse=before for walks\n @@ Documentation/rev-list-options.adoc: With `--topo-order`, they would show 8 6 5\n +\t`--walk-reflogs`. If `after`, output the commits chosen to be\n +\tshown (see 'Commit Limiting' section above) in reverse order. If\n +\t`before`, reverse the commits before filtering with `Commit\n -+\tLimiting` options. This option can be used multiple times, last\n -+\tone is applied. When the argument for `--reverse` is omitted, if\n -+\tthe current state is in no reverse, it defaults to `after`. If\n -+\tit is in any reversed state, it restores the original ordering\n -+\tby removing the reverse state.\n ++\tLimiting` options. When multiple `--reverse=` options are given,\n ++\tthe final option overrides any previous options. The `--reverse`\n ++\toption (with no specifier) behaves as `--reverse=after`, except\n ++\tthat, for historical reasons, it negates any previous reversed\n ++\tstate (so `--reverse --reverse` does nothing, nor does\n ++\t`--reverse=before --reverse`. Note that `--reverse=before\n ++\t--reverse --reverse` is the same as `--reverse=after`).\n endif::git-shortlog[]\n \n ifndef::git-shortlog[]\n2: 480b322cf8 ! 2: 00489b0e52 revision.c: reduce memory usage on reverse before\n @@ revision.c: static struct commit *get_revision_internal(struct rev_info *revs)\n }\n \n +static void retrieve_with_window(struct rev_info *revs, int max_count,\n -+\t\t\t struct commit_list **reversed)\n ++\t\t\t \t struct commit_list **reversed)\n +{\n +\tstruct commit *c;\n +\tstruct commit_list *into_queue = NULL;\n @@ revision.c: static struct commit *get_revision_internal(struct rev_info *revs)\n +\t\t}\n +\t}\n +\n -+\twhile (outo_count) {\n -+\t\tc = pop_commit(&outo_queue);\n -+\t\touto_count--;\n ++\twhile ((c = pop_commit(&outo_queue)))\n +\t\tcommit_list_insert(c, reversed);\n -+\t}\n -+\n -+\twhile (into_count) {\n -+\t\tc = pop_commit(&into_queue);\n -+\t\tinto_count--;\n ++\twhile ((c = pop_commit(&into_queue)))\n +\t\tcommit_list_insert(c, &outo_queue);\n -+\t\touto_count++;\n -+\t}\n -+\n -+\twhile (outo_count) {\n -+\t\tc = pop_commit(&outo_queue);\n -+\t\touto_count--;\n ++\twhile ((c = pop_commit(&outo_queue)))\n +\t\tcommit_list_insert(c, reversed);\n -+\t}\n +}\n +\n struct commit *get_revision(struct rev_info *revs)\n-- \n2.54.0\n\n"},{"id":"542337","messageId":"cover.1777249165.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1776984666.git.mroik@delayed.space","subject":"[PATCH v4 0/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-27T00:24:56Z","receivedAt":"2026-04-27T00:25:46Z","isPatch":true,"body":"v4 aligns the behaviour for --reverse=before and --reverse=after on\nnegative numbers that are less than -1 to treat them the same as -1,\nwhich is current behaviour (before this series).\n\n[1/2] revision.c: implement --reverse=before for walks (Mirko Faina)\n[2/2] revision.c: reduce memory usage on reverse before (Mirko Faina)\n\n Documentation/rev-list-options.adoc | 16 +++++--\n revision.c | 73 +++++++++++++++++++++++++++--\n revision.h | 8 +++-\n t/t4202-log.sh | 66 ++++++++++++++++++++++++++\n 4 files changed, 153 insertions(+), 10 deletions(-)\n\nRange-diff against v3:\n1: 4864ac46dd = 1: 4864ac46dd revision.c: implement --reverse=before for walks\n2: 00489b0e52 ! 2: 7c0bab5d14 revision.c: reduce memory usage on reverse before\n @@ Commit message\n revision with --reverse=before and --max-count=. We do this through a\n simple queue. With N nodes and K as the --max-count argument, assuming K\n < N, we go from a space complexity of O(N) to O(K). When it comes down\n - to time complexity, the queue has an ammortized time of O(1) for pops,\n - so the complexity remains O(N).\n + to time complexity, the queue has an amortized time of O(1) for pops, so\n + the complexity remains O(N).\n \n Signed-off-by: Mirko Faina \n \n @@ revision.c: struct commit *get_revision(struct rev_info *revs)\n \t\treversed = NULL;\n -\t\twhile ((c = get_revision_internal(revs)))\n -\t\t\tcommit_list_insert(c, &reversed);\n -+\t\tif (revs->reverse == REVERSE_BEFORE && max_count != -1) {\n ++\t\tif (revs->reverse == REVERSE_BEFORE && max_count >= 0) {\n +\t\t\tretrieve_with_window(revs, max_count, &reversed);\n +\t\t} else {\n +\t\t\twhile ((c = get_revision_internal(revs)))\n\nbase-commit: e8955061076952cc5eab0300424fc48b601fe12d\n-- \n2.54.0\n\n"},{"id":"542338","messageId":"7c0bab5d14bb2ce2a10d35d93e3d911ed4c386eb.1777249165.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1777249165.git.mroik@delayed.space","subject":"[PATCH v4 2/2] revision.c: reduce memory usage on reverse before","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-27T00:24:58Z","receivedAt":"2026-04-27T00:25:47Z","isPatch":true,"body":"Due to the nature of --reverse=before we have to walk all of the history\nand store each non-filtered processed commit, this can be expensive on\nmemory for projects with a long history. When --max-count is being used\nwe don't really have to keep every processed commit, we can discard\nolder commits (as in have been processed before than the ones we're now\nconsidering, from a chronological commit order they are the newer\ncommits) as we surpass the --max-count limit.\n\nTeach get_revision() to keep only the newer commits as we walk a\nrevision with --reverse=before and --max-count=. We do this through a\nsimple queue. With N nodes and K as the --max-count argument, assuming K\n< N, we go from a space complexity of O(N) to O(K). When it comes down\nto time complexity, the queue has an amortized time of O(1) for pops, so\nthe complexity remains O(N).\n\nSigned-off-by: Mirko Faina \n---\n revision.c | 42 ++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex d581f5e38e..41c3d185c5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -4530,6 +4530,40 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_with_window(struct rev_info *revs, int max_count,\n+\t\t\t \t struct commit_list **reversed)\n+{\n+\tstruct commit *c;\n+\tstruct commit_list *into_queue = NULL;\n+\tstruct commit_list *outo_queue = NULL;\n+\tint into_count = 0;\n+\tint outo_count = 0;\n+\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\tcommit_list_insert(c, &into_queue);\n+\t\tinto_count++;\n+\t\tif (into_count + outo_count > max_count) {\n+\t\t\tif (!outo_count) {\n+\t\t\t\twhile (into_count) {\n+\t\t\t\t\tc = pop_commit(&into_queue);\n+\t\t\t\t\tinto_count--;\n+\t\t\t\t\tcommit_list_insert(c, &outo_queue);\n+\t\t\t\t\touto_count++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tpop_commit(&outo_queue);\n+\t\t\touto_count--;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&outo_queue)))\n+\t\tcommit_list_insert(c, reversed);\n+\twhile ((c = pop_commit(&into_queue)))\n+\t\tcommit_list_insert(c, &outo_queue);\n+\twhile ((c = pop_commit(&outo_queue)))\n+\t\tcommit_list_insert(c, reversed);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n@@ -4546,8 +4580,12 @@ struct commit *get_revision(struct rev_info *revs)\n \t\t\trevs->max_count = -1;\n \n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->reverse == REVERSE_BEFORE && max_count >= 0) {\n+\t\t\tretrieve_with_window(revs, max_count, &reversed);\n+\t\t} else {\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\t}\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse_output_stage = 1;\n-- \n2.54.0\n\n"},{"id":"542339","messageId":"4864ac46dd8ef4b704c29efc96c45f4e1412373b.1777249165.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1777249165.git.mroik@delayed.space","subject":"[PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-27T00:24:57Z","receivedAt":"2026-04-27T00:25:47Z","isPatch":true,"body":"In a revision walk `--reverse` can only be applied after any commit\nlimiting option. This makes getting a limited amount of commits from the\ntail impossible. E.g.\n\n git log --reverse --max-count=3\n\nSome would expect this to give back the first 3 commits of the project.\nInstead it returns the last 3 but in reversed order.\n\nTeach `get_revision()` to accpet an argument `(after|before)` from the\nCLI, and apply the reversal before or after the commit limiting options\nbased on this argument. If no argument is provided default to the\ncurrent behaviour, applying `--reverse` after the commit limiting\noptions.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 16 +++++--\n revision.c | 31 ++++++++++++--\n revision.h | 8 +++-\n t/t4202-log.sh | 66 +++++++++++++++++++++++++++++\n 4 files changed, 113 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..e97f6f2aff 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -914,10 +914,18 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5\n avoid showing the commits from two parallel development track mixed\n together.\n \n-`--reverse`::\n-\tOutput the commits chosen to be shown (see 'Commit Limiting'\n-\tsection above) in reverse order. Cannot be combined with\n-\t`--walk-reflogs`.\n+`--[no-]reverse[=(after|before)]`::\n+\tAccepts `after` or `before`. Cannot be combined with\n+\t`--walk-reflogs`. If `after`, output the commits chosen to be\n+\tshown (see 'Commit Limiting' section above) in reverse order. If\n+\t`before`, reverse the commits before filtering with `Commit\n+\tLimiting` options. When multiple `--reverse=` options are given,\n+\tthe final option overrides any previous options. The `--reverse`\n+\toption (with no specifier) behaves as `--reverse=after`, except\n+\tthat, for historical reasons, it negates any previous reversed\n+\tstate (so `--reverse --reverse` does nothing, nor does\n+\t`--reverse=before --reverse`. Note that `--reverse=before\n+\t--reverse --reverse` is the same as `--reverse=after`).\n endif::git-shortlog[]\n \n ifndef::git-shortlog[]\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..d581f5e38e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2686,7 +2686,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\tgit_log_output_encoding = xstrdup(\"\");\n \t\treturn argcount;\n \t} else if (!strcmp(arg, \"--reverse\")) {\n-\t\trevs->reverse ^= 1;\n+\t\trevs->reverse = !revs->reverse;\n+\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n+\t\tif (!strcmp(optarg, \"after\"))\n+\t\t\trevs->reverse = REVERSE_AFTER;\n+\t\telse if(!strcmp(optarg, \"before\"))\n+\t\t\trevs->reverse = REVERSE_BEFORE;\n+\t\telse\n+\t\t\tdie(_(\"unknown value for --reverse: %s\"), optarg);\n+\t} else if (!strcmp(arg, \"--no-reverse\")) {\n+\t\trevs->reverse = NO_REVERSE;\n \t} else if (!strcmp(arg, \"--children\")) {\n \t\trevs->children.name = \"children\";\n \t\trevs->limited = 1;\n@@ -4525,19 +4534,35 @@ struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tint max_count = revs->max_count;\n+\n+\tif (revs->reverse && !revs->reverse_output_stage) {\n+\t\tif (revs->reverse == 3) {\n+\t\t\tBUG(\"allowed values for reverse are 0, 1 and 2\");\n+\t\t\trevs->reverse = 1;\n+\t\t}\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = -1;\n \n-\tif (revs->reverse) {\n \t\treversed = NULL;\n \t\twhile ((c = get_revision_internal(revs)))\n \t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n-\t\trevs->reverse = 0;\n \t\trevs->reverse_output_stage = 1;\n+\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count = max_count;\n \t}\n \n \tif (revs->reverse_output_stage) {\n+\t\tif (revs->reverse == REVERSE_BEFORE && revs->max_count == 0)\n+\t\t\treturn NULL;\n+\n \t\tc = pop_commit(&revs->commits);\n+\t\tif (revs->reverse == REVERSE_BEFORE)\n+\t\t\trevs->max_count--;\n \t\tif (revs->track_linear)\n \t\t\trevs->linear = !!(c && c->object.flags & TRACK_LINEAR);\n \t\treturn c;\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..02881577dc 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,6 +121,12 @@ struct ref_exclusions {\n struct oidset;\n struct topo_walk_info;\n \n+enum rev_reverse {\n+\tNO_REVERSE = 0,\n+\tREVERSE_AFTER = 1,\n+\tREVERSE_BEFORE = 2,\n+};\n+\n struct rev_info {\n \t/* Starting list */\n \tstruct commit_list *commits;\n@@ -167,6 +173,7 @@ struct rev_info {\n \t\t\tignore_missing_links:1;\n \n \t/* Traversal flags */\n+\tenum rev_reverse reverse:2;\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n \t\t\tno_walk:1,\n@@ -196,7 +203,6 @@ struct rev_info {\n \t\t\trewrite_parents:1,\n \t\t\tprint_parents:1,\n \t\t\tshow_decorations:1,\n-\t\t\treverse:1,\n \t\t\treverse_output_stage:1,\n \t\t\tcherry_pick:1,\n \t\t\tcherry_mark:1,\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..3bfe2c99b8 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+cat >expect <<-\\EOF\n+c3f451c Merge tag 'reach'\n+046b221 to remove\n+EOF\n+\n+test_expect_success 'log --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse=after --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse=after --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+3a2fdcb initial\n+f7dab8e second\n+EOF\n+\n+test_expect_success 'log --reverse=before --oneline --max-count=2' '\n+\ttest_when_finished rm actual &&\n+\tgit log --reverse=before --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-\\EOF\n+046b221 to remove\n+c3f451c Merge tag 'reach'\n+EOF\n+\n+test_expect_success 'log --reverse --reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '\n+\ttest_when_finished git reset --hard HEAD~1 &&\n+\ttouch to_remove &&\n+\tgit add to_remove &&\n+\tgit commit -m \"to remove\" &&\n+\tgit log --reverse --no-reverse --oneline --max-count=2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"542347","messageId":"xmqq8qa852b5.fsf@gitster.g","threadId":"65511","inReplyTo":"4864ac46dd8ef4b704c29efc96c45f4e1412373b.1777249165.git.mroik@delayed.space","subject":"Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-27T06:45:34Z","receivedAt":"2026-04-27T06:45:37Z","isPatch":true,"body":"Mirko Faina writes:\n\n> In a revision walk `--reverse` can only be applied after any commit\n> limiting option. This makes getting a limited amount of commits from the\n> tail impossible. E.g.\n>\n> git log --reverse --max-count=3\n\nCan we rephrase \"from the tail\" somehow to reduce ambiguity?\n\nNormally we generate a list of commits from newer to older, and you\nare saying that it is not possible to take the oldest three commits\nand show them from older to newer (i.e., in reverse). But that, to\nsome readers, is showing commits from the beginning end, not from\nthe tail end.\n\nPerhaps \"... limited number of oldest commits impossible\"?\n\n> Teach `get_revision()` to accpet an argument `(after|before)` from the\n> CLI, and apply the reversal before or after the commit limiting options\n> based on this argument.\n\nI think \"after\" and \"before\" comes from \"Do other things (including\ncount limiting) and then apply reverse after all that\" and would be\nvery much understandable to those who know how the machinery works,\nbut should mere mortals need to know the machinery only to use \"git\nlog\"?\n\nTo put it another way, do you tnink experienced Git users who\nhaven't seen the actual implementation of revision traversal can\nimmediately answer this question:\n\n Now we have --reverse=after and --reverse=before to let you take\n a limited history from both ends when used with --max-count.\n Which between after and before do you think corresponds to the\n traditional --reverse that allowed you to only see the newest\n part of the history?\n\nI doubt that the population to answer correctly would not exceed a\nhalf by large margin (if it is 50% then it means nobody understood\nthe difference correctly and they just flipped a coin).\n\nI wonder --reverse=oldest and --reverse=newest is easier to teach\nand explain? I dunno.\n"},{"id":"542348","messageId":"971f19db-eb10-4c88-8d5d-3f4f7f92db73@kdbg.org","threadId":"65511","inReplyTo":"xmqq8qa852b5.fsf@gitster.g","subject":"Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-04-27T07:33:59Z","receivedAt":"2026-04-27T08:08:03Z","isPatch":true,"body":"Am 27.04.26 um 08:45 schrieb Junio C Hamano:\n> I think \"after\" and \"before\" comes from \"Do other things (including\n> count limiting) and then apply reverse after all that\" and would be\n> very much understandable to those who know how the machinery works,\n> but should mere mortals need to know the machinery only to use \"git\n> log\"?\n\nI fully share your sentiments regarding \"after\" and \"before\" being too\nmuch tied to the machinery, but...\n\n> I wonder --reverse=oldest and --reverse=newest is easier to teach\n> and explain? I dunno.\nWhat does it mean to \"revert the oldest\"? Or \"the newest\"? If at all,\nthen this \"newest\" and \"oldest\" must be a restriction that applies to\n--max-count in some way. Perhaps we need a --max-count-oldest option,\nthen --reverse does not have to be touched at all, because it is still\napplied only after the set of commits to show has been determined.\n\n-- Hannes\n\n"},{"id":"542367","messageId":"xmqq1pg04mbt.fsf@gitster.g","threadId":"65511","inReplyTo":"971f19db-eb10-4c88-8d5d-3f4f7f92db73@kdbg.org","subject":"Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-27T12:30:46Z","receivedAt":"2026-04-27T12:30:49Z","isPatch":true,"body":"Johannes Sixt writes:\n\n>> I wonder --reverse=oldest and --reverse=newest is easier to teach\n>> and explain? I dunno.\n\n> What does it mean to \"revert the oldest\"? Or \"the newest\"?\n\nI do not quite understand where the \"revert\" comes from, though.\n\n> If at all,\n> then this \"newest\" and \"oldest\" must be a restriction that applies to\n> --max-count in some way. Perhaps we need a --max-count-oldest option,\n> then --reverse does not have to be touched at all, because it is still\n> applied only after the set of commits to show has been determined.\n\nThat makes two of us to suspect that this is more about --max-count\nthan --reverse.\n\ncf. https://lore.kernel.org/git/xmqqv7dlr4yz.fsf@gitster.g/\n\n\"git log --max-count-oldest=3\" will give us three oldest commit in\nreverse chronological order, the set of commits shown are the same\nwith or without \"--reverse\", which makes tons of sense.\n"},{"id":"542386","messageId":"CAPx1GvcU8b7CfGrXxzZa10Ys2YScGq_2B4M9jkhs2SwywRP3AQ@mail.gmail.com","threadId":"65511","inReplyTo":"xmqq1pg04mbt.fsf@gitster.g","subject":"Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2026-04-27T13:58:55Z","receivedAt":"2026-04-27T13:59:10Z","isPatch":true,"body":"This topic has been rattling around in my head for a while, and I need\nto get it out now. :-)\n\nFirst, a few notes:\n\n * I'm going to delete most of the context because I want to go back\nto first principles here. Instead, I'll list what I think are the real\nissues.\n\n * I have always had a suspicion that `--max-count` / `-n` was \"done\nwrong\" in the first place, it's just that it's generally invisibly\nwrong.\n\n * While all the revision-walking machinery normally shows commits\n\"newest first\", there are several fundamental issues with defining\n\"newest\" anyway. A lot of Git newcomers find this terribly confusing\n-- usually a few weeks or months into use of Git, really.\n\nIt's important to note that the rev-walk machinery uses a priority\nqueue, and that this is necessary because commits are in a directed\ngraph, which cannot be presented linearly unless you're willing to:\n(a) add additional information (graph drawing, parent list, whatever),\nor (b) discard information. The `git log` and `git rev-list`\ndocumentation skimp a bit on this.\n\nThere is of course nothing wrong with discarding information when it's\nirrelevant. In fact, that's the whole point of abstraction, to toss\nout irrelevancies so that one can concentrate only on the relevant.\nAnd that's what all the limiting options for revision walking are for!\n\nSorting options affect the order in which items go into the priority\nqueue. Limiting options affect which items go in, and sometimes, how\nmany items come out. Display options affect what we see when the items\ncome out, and this includes the sorting options since they come out in\nthe order they're in there.\n\nThus, `--reverse` is a *display* option (part of sorting), while\n`--max-count` is a *limiting* option. It's just that, well, there's a\nspecial case when they're combined.\n\nJunio noted:\n\n> That makes two of us to suspect that this is more about --max-count than --reverse.\n\nAnd that's really the case here. Because `--max-count` was \"done\nwrong\" initially, we have a slight problem. Had it been done as a\n\"window of items in the priority queue\", we would always have gotten\nthe limited-to-N items remaining in the queue after the selection\nprocess, displayed according to the display process. But when the\ndisplay is going to be \"in the order of items in the queue\" and the\nlimiting count is N items *and* the display doesn't reverse the queue,\nit suffices to display the first N items and then quit entirely. This\nis of course a nice space-and-time optimization.\n\nAs it turns out, the only display option that causes this optimization\nto be invalid is (or might be) `--reverse`.\n\nUnfortunately, fixing the problem by simply defeating the \"keep N\nitems in the queue and only stop early (and maybe display as we go as\nwell) if we're allowed\" optimization -- the one that was applied\nprematurely, as it were -- will change the existing behavior of `git\nlog -n 10 --reverse` in any repository with more than 10 commits in\nit. Had the over-optimization not been done, and someone wanted to add\na \"gather only N into the queue and then stop traversing, and then\ndisplay\" option, we could perhaps use `--stop-walk-(after|at)=n` as a\nnew option.\n\nAs far as I can tell, the gripe that this exposes the priority queue\nmechanism is valid, but at best trivial, because knowing about the\npriority queue is crucial anyway. Beginners can skip it for a little\nwhile, but as soon as they find out that commits have two separate\ndate stamps, and learn about `--date-order`, `--author-date-order`,\nand `--topo-order`, they need to learn about the queue.\n\nIf it's deemed acceptable to change the historic behavior of\n`--max-count` combined with `--reverse`, I'd suggest simply adding\n`--stop-walk-after` (perhaps with a slightly different name) to take\nover the historic behavior of `--max-count`, and make `--max-count`\nnot over-optimize. If not, I'd suggest a new option, with a note in\nthe documentation that `--max-count` has this odd behavior when\ncombined with `--reverse`. Perhaps the new option could be called\n`--prio-queue-size=n`. The implementation can still optimize this\n(using the same code as before) when `--reverse` isn't in effect,\nsince the effect is only visible with `--reverse`.\n\nChris\n"},{"id":"542392","messageId":"ae-J4ooz2PJ8bZAq@exploit","threadId":"65511","inReplyTo":"xmqq8qa852b5.fsf@gitster.g","subject":"Re: [PATCH v4 1/2] revision.c: implement -b-reverse=before for walks","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-27T16:48:56Z","receivedAt":"2026-04-27T16:49:01Z","isPatch":true,"body":"On Mon, Apr 27, 2026 at 03:45:34PM +0900, Junio C Hamano wrote:\n> > In a revision walk `--reverse` can only be applied after any commit\n> > limiting option. This makes getting a limited amount of commits from the\n> > tail impossible. E.g.\n> >\n> > git log --reverse --max-count=3\n> \n> Can we rephrase \"from the tail\" somehow to reduce ambiguity?\n> \n> Normally we generate a list of commits from newer to older, and you\n> are saying that it is not possible to take the oldest three commits\n> and show them from older to newer (i.e., in reverse). But that, to\n> some readers, is showing commits from the beginning end, not from\n> the tail end.\n> \n> Perhaps \"... limited number of oldest commits impossible\"?\n\nYes, will rephrase.\n\n> > Teach `get_revision()` to accpet an argument `(after|before)` from the\n> > CLI, and apply the reversal before or after the commit limiting options\n> > based on this argument.\n> \n> I think \"after\" and \"before\" comes from \"Do other things (including\n> count limiting) and then apply reverse after all that\" and would be\n> very much understandable to those who know how the machinery works,\n> but should mere mortals need to know the machinery only to use \"git\n> log\"?\n\nNo, they shouldn't, but...\n\n> To put it another way, do you tnink experienced Git users who\n> haven't seen the actual implementation of revision traversal can\n> immediately answer this question:\n> \n> Now we have --reverse=after and --reverse=before to let you take\n> a limited history from both ends when used with --max-count.\n> Which between after and before do you think corresponds to the\n> traditional --reverse that allowed you to only see the newest\n> part of the history?\n\n...while users might not have read the implementation (and they\nshouldn't need to to use log) the order in which the class of options is\napplied is already documented in the man pages, so having that knowledge\nafter and before do make sense despite not having seen the\nimplementation.\n\nBut...\n\n> I doubt that the population to answer correctly would not exceed a\n> half by large margin (if it is 50% then it means nobody understood\n> the difference correctly and they just flipped a coin).\n> \n> I wonder --reverse=oldest and --reverse=newest is easier to teach\n> and explain? I dunno.\n\n...you're right, it is not immediately apparent, but neither are oldest\nand newest. Unfortunately I don't think there's any name we can choose\nthat would make it so without having to read the caveats in the man\npages.\n\nSince many have expressed that this issue is not really about reverse\nbut about max-count, like you initially assesed, then we should move\ntowards making changes to the max-count option.\n\nThe proposed --max-count-oldest doesn't seem right to me as\nmax-count-oldest is not about keeping the oldest commits (even tho\nthat's what we want to achieve when we interact with reverse) but about\nwhen we want to apply max-count. Since [1],\n\n> It might be that the right way to look at this new feature is not that\n> \"we are changing where reverse is applied\", but \"count limit is applied\n> much later than usual\"\n\nmaybe --max-count-later as in max count is being applied later than\nusual? (either way the users will still need to reach for the man pages\nfor clarifications).\n\n[1] https://lore.kernel.org/git/xmqqv7dlr4yz.fsf@gitster.g/\n"},{"id":"542401","messageId":"xmqq1pfz3lj8.fsf@gitster.g","threadId":"65511","inReplyTo":"4864ac46dd8ef4b704c29efc96c45f4e1412373b.1776984666.git.mroik@delayed.space","subject":"Re: [PATCH v3 1/2] revision.c: implement --reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-28T01:45:31Z","receivedAt":"2026-04-28T01:45:35Z","isPatch":true,"body":"Mirko Faina writes:\n\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 05cee9e41b..3bfe2c99b8 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n\nThe hardcoded short object names are setting up traps to fail when \n\n $ GIT_TEST_DEFAULT_HASH=sha256 make test\n\nis run. It also may break when the default abbreviation length\nand other things change.\n\n> @@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '\n> \ttest_cmp_graph --name-status tangle..reach\n> '\n> \n> +cat >expect <<-\\EOF\n> +c3f451c Merge tag 'reach'\n> +046b221 to remove\n> +EOF\n> +test_expect_success 'log --reverse --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse --reverse --reverse --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse=after --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse=after --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +cat >expect <<-\\EOF\n> +3a2fdcb initial\n> +f7dab8e second\n> +EOF\n> +\n> +test_expect_success 'log --reverse=before --oneline --max-count=2' '\n> +\ttest_when_finished rm actual &&\n> +\tgit log --reverse=before --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +cat >expect <<-\\EOF\n> +046b221 to remove\n> +c3f451c Merge tag 'reach'\n> +EOF\n> +\n> +test_expect_success 'log --reverse --reverse --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse --reverse --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '\n> +\ttest_when_finished git reset --hard HEAD~1 &&\n> +\ttouch to_remove &&\n> +\tgit add to_remove &&\n> +\tgit commit -m \"to remove\" &&\n> +\tgit log --reverse --no-reverse --oneline --max-count=2 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> cat >expect <<-\\EOF\n> * reach\n> |\n"},{"id":"542402","messageId":"xmqqv7db26y3.fsf@gitster.g","threadId":"65511","inReplyTo":"4864ac46dd8ef4b704c29efc96c45f4e1412373b.1777249165.git.mroik@delayed.space","subject":"Re: [PATCH v4 1/2] revision.c: implement --reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-28T01:45:56Z","receivedAt":"2026-04-28T01:45:58Z","isPatch":true,"body":"Mirko Faina writes:\n\n> +\t} else if (skip_prefix(arg, \"--reverse=\", &optarg)) {\n> +\t\tif (!strcmp(optarg, \"after\"))\n> +\t\t\trevs->reverse = REVERSE_AFTER;\n> +\t\telse if(!strcmp(optarg, \"before\"))\n\nStyle: missing SP after \"else if\".\n"},{"id":"542403","messageId":"xmqqpl3j26xu.fsf@gitster.g","threadId":"65511","inReplyTo":"7c0bab5d14bb2ce2a10d35d93e3d911ed4c386eb.1777249165.git.mroik@delayed.space","subject":"Re: [PATCH v4 2/2] revision.c: reduce memory usage on reverse before","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-28T01:46:05Z","receivedAt":"2026-04-28T01:46:07Z","isPatch":true,"body":"Mirko Faina writes:\n\n> \t\treversed = NULL;\n> -\t\twhile ((c = get_revision_internal(revs)))\n> -\t\t\tcommit_list_insert(c, &reversed);\n> +\t\tif (revs->reverse == REVERSE_BEFORE && max_count >= 0) {\n> +\t\t\tretrieve_with_window(revs, max_count, &reversed);\n> +\t\t} else {\n> +\t\t\twhile ((c = get_revision_internal(revs)))\n> +\t\t\t\tcommit_list_insert(c, &reversed);\n> +\t\t}\n\nStyle: needless {} around single-statement body of if/else.\n\n> \t\tcommit_list_free(revs->commits);\n> \t\trevs->commits = reversed;\n> \t\trevs->reverse_output_stage = 1;\n"},{"id":"542404","messageId":"xmqqjytr26xe.fsf@gitster.g","threadId":"65511","inReplyTo":"ae-J4ooz2PJ8bZAq@exploit","subject":"Re: [PATCH v4 1/2] revision.c: implement -b-reverse=before for walks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-28T01:46:21Z","receivedAt":"2026-04-28T01:46:23Z","isPatch":true,"body":"Mirko Faina writes:\n\n>> It might be that the right way to look at this new feature is not that\n>> \"we are changing where reverse is applied\", but \"count limit is applied\n>> much later than usual\"\n>\n> maybe --max-count-later as in max count is being applied later than\n> usual? (either way the users will still need to reach for the man pages\n> for clarifications).\n\nI very much more prefer what J6t suggested, which (if I am\nunderstanding him correctly) would make\n\n git log --max-count-oldest=3 $options\n\nconceptually run \"git log\" (without count limit but with other\noptions like --grep, --author, --since, etc. applied) without\nshowing anything until the last three commits remains, and then\nshows these last three commits.\n\n git log --max-count-oldest=3 --reverse $options\n\nwould show these same last three commits, but in the order opposite\nto the first one.\n"},{"id":"542536","messageId":"2f71a00b035e25b971641b77a6fa7626f1e2459c.1777578676.git.mroik@delayed.space","threadId":"65511","inReplyTo":"cover.1777249165.git.mroik@delayed.space","subject":"[PATCH v5] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-04-30T19:52:45Z","receivedAt":"2026-04-30T19:53:29Z","isPatch":true,"body":"--max-count is a commit limiting option sets a maximum amount of commits\nto be shown. If a user wants to see only the first N commits of the\nhistory (the oldest commits) they'd have to combine --max-count with\n--skip. This is not very user-friendly.\n\nTeach get_revision() the --max-count-oldest option.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 3 ++\n revision.c | 77 +++++++++++++++++++++++++++--\n revision.h | 2 +\n t/t4202-log.sh | 14 ++++++\n 4 files changed, 93 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..736f34efab 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -18,6 +18,9 @@ ordering and formatting options, such as `--reverse`.\n `--max-count=`::\n \tLimit the output to __ commits.\n \n+`--max-count-oldest=`::\n+\tLimit the output to the __ oldest commits.\n+\n `--skip=`::\n \tSkip __ commits before starting to show the commit output.\n \ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..3aaa77ced5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t}\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie(_(\"can't use --max-count with --max-count-oldest\"));\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 0;\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n+\t\t\tdie(_(\"can't use --max-count with --max-count-oldest\"));\n+\t\tif (revs->skip_count > 0)\n+\t\t\tdie(_(\"con't use --max-count-oldest with --skip\"));\n+\t\trevs->max_count = parse_count(optarg);\n+\t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 1;\n+\t\trevs->max_count_stage = 0;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie(_(\"con't use --max-count-oldest with --skip\"));\n \t\trevs->skip_count = parse_count(optarg);\n \t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_oldest_commits(struct rev_info *revs,\n+\t\t\t\t struct commit_list **queue)\n+{\n+\tstruct commit *c;\n+\tint max_count = revs->max_count;\n+\tint queuei_count = 0;\n+\tint queueo_count = 0;\n+\tstruct commit_list *queueo = NULL;\n+\tstruct commit_list *queuei = NULL;\n+\tstruct commit_list *reversed_queue = NULL;\n+\n+\trevs->max_count = -1;\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\tc->object.flags &= ~SHOWN;\n+\t\tcommit_list_insert(c, &queuei);\n+\t\tqueuei_count++;\n+\t\twhile (queuei_count + queueo_count > max_count) {\n+\t\t\tif (!queueo_count) {\n+\t\t\t\twhile (queuei_count > 0) {\n+\t\t\t\t\tc = pop_commit(&queuei);\n+\t\t\t\t\tqueuei_count--;\n+\t\t\t\t\tcommit_list_insert(c, &queueo);\n+\t\t\t\t\tqueueo_count++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tpop_commit(&queueo);\n+\t\t\tqueueo_count--;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\twhile ((c = pop_commit(&queuei)))\n+\t\tcommit_list_insert(c, &queueo);\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\n+\twhile ((c = pop_commit(&reversed_queue)))\n+\t\tcommit_list_insert(c, queue);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tstruct commit_list *queue = NULL;\n+\n+\tif (revs->max_count_type == 1 && !revs->max_count_stage) {\n+\t\tretrieve_oldest_commits(revs, &queue);\n+\t\tcommit_list_free(revs->commits);\n+\t\trevs->commits = queue;\n+\t\trevs->max_count_stage = 1;\n+\t}\n \n \tif (revs->reverse) {\n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->max_count_type == 1)\n+\t\t\twhile ((c = pop_commit(&revs->commits)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\telse\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse = 0;\n@@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)\n \t\treturn c;\n \t}\n \n-\tc = get_revision_internal(revs);\n+\tif (revs->max_count_stage)\n+\t\tc = pop_commit(&revs->commits);\n+\telse\n+\t\tc = get_revision_internal(revs);\n+\n \tif (c && revs->graph)\n \t\tgraph_update(revs->graph, c);\n \tif (!c) {\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..e157463cb1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -309,6 +309,8 @@ struct rev_info {\n \t/* special limits */\n \tint skip_count;\n \tint max_count;\n+\tunsigned int max_count_type:1;\n+\tunsigned int max_count_stage:1;\n \ttimestamp_t max_age;\n \ttimestamp_t max_age_as_filter;\n \ttimestamp_t min_age;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..668c231cf1 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,20 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+test_expect_success 'log --max-count-oldest=3 --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline | tail -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline | tail -n3 | tac >expect &&\n+\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"542644","messageId":"xmqqik93px8s.fsf@gitster.g","threadId":"65511","inReplyTo":"2f71a00b035e25b971641b77a6fa7626f1e2459c.1777578676.git.mroik@delayed.space","subject":"Re: [PATCH v5] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-04T05:19:47Z","receivedAt":"2026-05-04T05:20:14Z","isPatch":true,"body":"Mirko Faina writes:\n\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to combine --max-count with\n> --skip. This is not very user-friendly.\n\nTo use \"--skip=\" for this purose, you'd need to know how many\nrecords are going to be omitted to begin with, but that means you'd\nrun the command without count limitation once only to find out how\nmany records there are. \"not very user-friendly\" sounds like an\nunderstatement of the year.\n\nThey can do with something silly like\n\n git rev-list ... |\n tail -n N |\n xargs -n1 git show ...\n\nand that does count as \"not very user-friendly\", I would think.\n\n> Teach get_revision() the --max-count-oldest option.\n>\n> Signed-off-by: Mirko Faina \n> ---\n> Documentation/rev-list-options.adoc | 3 ++\n> revision.c | 77 +++++++++++++++++++++++++++--\n> revision.h | 2 +\n> t/t4202-log.sh | 14 ++++++\n> 4 files changed, 93 insertions(+), 3 deletions(-)\n\nIt looks like this needs measurably smaller damage to the codebase\nthan the other --reverse=before approach ;-).\n\n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..736f34efab 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -18,6 +18,9 @@ ordering and formatting options, such as `--reverse`.\n> `--max-count=`::\n> \tLimit the output to __ commits.\n> \n> +`--max-count-oldest=`::\n> +\tLimit the output to the __ oldest commits.\n> +\n> `--skip=`::\n> \tSkip __ commits before starting to show the commit output.\n> \n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..3aaa77ced5 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> \t}\n> \n> \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> +\t\tif (revs->max_count_type == 1)\n> +\t\t\tdie(_(\"can't use --max-count with --max-count-oldest\"));\n> \t\trevs->max_count = parse_count(optarg);\n> \t\trevs->no_walk = 0;\n> +\t\trevs->max_count_type = 0;\n> \t\treturn argcount;\n> +\t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n> +\t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n> +\t\t\tdie(_(\"can't use --max-count with --max-count-oldest\"));\n> +\t\tif (revs->skip_count > 0)\n> +\t\t\tdie(_(\"con't use --max-count-oldest with --skip\"));\n> +\t\trevs->max_count = parse_count(optarg);\n> +\t\trevs->no_walk = 0;\n> +\t\trevs->max_count_type = 1;\n> +\t\trevs->max_count_stage = 0;\n> \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n> +\t\tif (revs->max_count_type == 1)\n> +\t\t\tdie(_(\"con't use --max-count-oldest with --skip\"));\n> \t\trevs->skip_count = parse_count(optarg);\n> \t\treturn argcount;\n> \t} else if ((*arg == '-') && isdigit(arg[1])) {\n> @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n> \treturn c;\n> }\n> \n> +static void retrieve_oldest_commits(struct rev_info *revs,\n> +\t\t\t\t struct commit_list **queue)\n> +{\n> +\tstruct commit *c;\n> +\tint max_count = revs->max_count;\n> +\tint queuei_count = 0;\n> +\tint queueo_count = 0;\n> +\tstruct commit_list *queueo = NULL;\n> +\tstruct commit_list *queuei = NULL;\n> +\tstruct commit_list *reversed_queue = NULL;\n> +\n> +\trevs->max_count = -1;\n> +\twhile ((c = get_revision_internal(revs))) {\n> +\t\tc->object.flags &= ~SHOWN;\n> +\t\tcommit_list_insert(c, &queuei);\n> +\t\tqueuei_count++;\n> +\t\twhile (queuei_count + queueo_count > max_count) {\n> +\t\t\tif (!queueo_count) {\n> +\t\t\t\twhile (queuei_count > 0) {\n> +\t\t\t\t\tc = pop_commit(&queuei);\n> +\t\t\t\t\tqueuei_count--;\n> +\t\t\t\t\tcommit_list_insert(c, &queueo);\n> +\t\t\t\t\tqueueo_count++;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tpop_commit(&queueo);\n> +\t\t\tqueueo_count--;\n> +\t\t}\n> +\t}\n> +\n> +\twhile ((c = pop_commit(&queueo)))\n> +\t\tcommit_list_insert(c, &reversed_queue);\n> +\twhile ((c = pop_commit(&queuei)))\n> +\t\tcommit_list_insert(c, &queueo);\n> +\twhile ((c = pop_commit(&queueo)))\n> +\t\tcommit_list_insert(c, &reversed_queue);\n> +\n> +\twhile ((c = pop_commit(&reversed_queue)))\n> +\t\tcommit_list_insert(c, queue);\n> +}\n> +\n> struct commit *get_revision(struct rev_info *revs)\n> {\n> \tstruct commit *c;\n> \tstruct commit_list *reversed;\n> +\tstruct commit_list *queue = NULL;\n> +\n> +\tif (revs->max_count_type == 1 && !revs->max_count_stage) {\n> +\t\tretrieve_oldest_commits(revs, &queue);\n> +\t\tcommit_list_free(revs->commits);\n> +\t\trevs->commits = queue;\n> +\t\trevs->max_count_stage = 1;\n> +\t}\n> \n> \tif (revs->reverse) {\n> \t\treversed = NULL;\n> -\t\twhile ((c = get_revision_internal(revs)))\n> -\t\t\tcommit_list_insert(c, &reversed);\n> +\t\tif (revs->max_count_type == 1)\n> +\t\t\twhile ((c = pop_commit(&revs->commits)))\n> +\t\t\t\tcommit_list_insert(c, &reversed);\n> +\t\telse\n> +\t\t\twhile ((c = get_revision_internal(revs)))\n> +\t\t\t\tcommit_list_insert(c, &reversed);\n> \t\tcommit_list_free(revs->commits);\n> \t\trevs->commits = reversed;\n> \t\trevs->reverse = 0;\n> @@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)\n> \t\treturn c;\n> \t}\n> \n> -\tc = get_revision_internal(revs);\n> +\tif (revs->max_count_stage)\n> +\t\tc = pop_commit(&revs->commits);\n> +\telse\n> +\t\tc = get_revision_internal(revs);\n> +\n> \tif (c && revs->graph)\n> \t\tgraph_update(revs->graph, c);\n> \tif (!c) {\n> diff --git a/revision.h b/revision.h\n> index 584f1338b5..e157463cb1 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -309,6 +309,8 @@ struct rev_info {\n> \t/* special limits */\n> \tint skip_count;\n> \tint max_count;\n> +\tunsigned int max_count_type:1;\n> +\tunsigned int max_count_stage:1;\n> \ttimestamp_t max_age;\n> \ttimestamp_t max_age_as_filter;\n> \ttimestamp_t min_age;\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 05cee9e41b..668c231cf1 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -1882,6 +1882,20 @@ test_expect_success 'log --graph with --name-status' '\n> \ttest_cmp_graph --name-status tangle..reach\n> '\n> \n> +test_expect_success 'log --max-count-oldest=3 --oneline' '\n> +\ttest_when_finished rm expect &&\n> +\tgit log --oneline | tail -n3 >expect &&\n> +\tgit log --oneline --max-count-oldest=3 >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n> +\ttest_when_finished rm expect &&\n> +\tgit log --oneline | tail -n3 | tac >expect &&\n> +\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> cat >expect <<-\\EOF\n> * reach\n> |\n"},{"id":"542667","messageId":"afiZigRf8cSrEM_L@exploit","threadId":"65511","inReplyTo":"xmqqik93px8s.fsf@gitster.g","subject":"Re: [PATCH v5] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-04T13:08:14Z","receivedAt":"2026-05-04T13:08:25Z","isPatch":true,"body":"On Mon, May 04, 2026 at 02:19:47PM +0900, Junio C Hamano wrote:\n> Mirko Faina writes:\n> \n> > --max-count is a commit limiting option sets a maximum amount of commits\n> > to be shown. If a user wants to see only the first N commits of the\n> > history (the oldest commits) they'd have to combine --max-count with\n> > --skip. This is not very user-friendly.\n> \n> To use \"--skip=\" for this purose, you'd need to know how many\n> records are going to be omitted to begin with, but that means you'd\n> run the command without count limitation once only to find out how\n> many records there are. \"not very user-friendly\" sounds like an\n> understatement of the year.\n> \n> They can do with something silly like\n> \n> git rev-list ... |\n> tail -n N |\n> xargs -n1 git show ...\n> \n> and that does count as \"not very user-friendly\", I would think.\n\nWill reword in v6.\n\n> > Teach get_revision() the --max-count-oldest option.\n> >\n> > Signed-off-by: Mirko Faina \n> > ---\n> > Documentation/rev-list-options.adoc | 3 ++\n> > revision.c | 77 +++++++++++++++++++++++++++--\n> > revision.h | 2 +\n> > t/t4202-log.sh | 14 ++++++\n> > 4 files changed, 93 insertions(+), 3 deletions(-)\n> \n> It looks like this needs measurably smaller damage to the codebase\n> than the other --reverse=before approach ;-).\n\nYes, the control flow of the program is also easier to read.\n\nThanks\n"},{"id":"542789","messageId":"ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space","threadId":"65511","inReplyTo":"2f71a00b035e25b971641b77a6fa7626f1e2459c.1777578676.git.mroik@delayed.space","subject":"[PATCH v6] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-05T21:54:56Z","receivedAt":"2026-05-05T21:55:33Z","isPatch":true,"body":"--max-count is a commit limiting option sets a maximum amount of commits\nto be shown. If a user wants to see only the first N commits of the\nhistory (the oldest commits) they'd have to do something like\n\n git log $(git rev-list HEAD | tail -n N | head -n 1)\n\nThis is not very user-friendly.\n\nTeach get_revision() the --max-count-oldest option.\n\nSigned-off-by: Mirko Faina \n---\nSince v5 I've reworded the commit message and rewrote the docs for\n--max-count-oldest to be clearer on its functionality.\n\n Documentation/rev-list-options.adoc | 5 ++\n revision.c | 77 +++++++++++++++++++++++++++--\n revision.h | 2 +\n t/t4202-log.sh | 14 ++++++\n 4 files changed, 95 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..9f857cabcc 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.\n `--max-count=`::\n Limit the output to __ commits.\n \n+`--max-count-oldest=`::\n+ Just like `--max-count=`, it limits the output to __\n+ commits. But instead of limiting to the first __ commits it\n+ limits to the last __ commits.\n+\n `--skip=`::\n Skip __ commits before starting to show the commit output.\n \ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..3aaa77ced5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n }\n \n if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n+ if (revs->max_count_type == 1)\n+ die(_(\"can't use --max-count with --max-count-oldest\"));\n revs->max_count = parse_count(optarg);\n revs->no_walk = 0;\n+ revs->max_count_type = 0;\n return argcount;\n+ } else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n+ if (revs->max_count_type == 0 && revs->max_count != -1)\n+ die(_(\"can't use --max-count with --max-count-oldest\"));\n+ if (revs->skip_count > 0)\n+ die(_(\"con't use --max-count-oldest with --skip\"));\n+ revs->max_count = parse_count(optarg);\n+ revs->no_walk = 0;\n+ revs->max_count_type = 1;\n+ revs->max_count_stage = 0;\n } else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n+ if (revs->max_count_type == 1)\n+ die(_(\"con't use --max-count-oldest with --skip\"));\n revs->skip_count = parse_count(optarg);\n return argcount;\n } else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n return c;\n }\n \n+static void retrieve_oldest_commits(struct rev_info *revs,\n+ struct commit_list **queue)\n+{\n+ struct commit *c;\n+ int max_count = revs->max_count;\n+ int queuei_count = 0;\n+ int queueo_count = 0;\n+ struct commit_list *queueo = NULL;\n+ struct commit_list *queuei = NULL;\n+ struct commit_list *reversed_queue = NULL;\n+\n+ revs->max_count = -1;\n+ while ((c = get_revision_internal(revs))) {\n+ c->object.flags &= ~SHOWN;\n+ commit_list_insert(c, &queuei);\n+ queuei_count++;\n+ while (queuei_count + queueo_count > max_count) {\n+ if (!queueo_count) {\n+ while (queuei_count > 0) {\n+ c = pop_commit(&queuei);\n+ queuei_count--;\n+ commit_list_insert(c, &queueo);\n+ queueo_count++;\n+ }\n+ }\n+ pop_commit(&queueo);\n+ queueo_count--;\n+ }\n+ }\n+\n+ while ((c = pop_commit(&queueo)))\n+ commit_list_insert(c, &reversed_queue);\n+ while ((c = pop_commit(&queuei)))\n+ commit_list_insert(c, &queueo);\n+ while ((c = pop_commit(&queueo)))\n+ commit_list_insert(c, &reversed_queue);\n+\n+ while ((c = pop_commit(&reversed_queue)))\n+ commit_list_insert(c, queue);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n struct commit *c;\n struct commit_list *reversed;\n+ struct commit_list *queue = NULL;\n+\n+ if (revs->max_count_type == 1 && !revs->max_count_stage) {\n+ retrieve_oldest_commits(revs, &queue);\n+ commit_list_free(revs->commits);\n+ revs->commits = queue;\n+ revs->max_count_stage = 1;\n+ }\n \n if (revs->reverse) {\n reversed = NULL;\n- while ((c = get_revision_internal(revs)))\n- commit_list_insert(c, &reversed);\n+ if (revs->max_count_type == 1)\n+ while ((c = pop_commit(&revs->commits)))\n+ commit_list_insert(c, &reversed);\n+ else\n+ while ((c = get_revision_internal(revs)))\n+ commit_list_insert(c, &reversed);\n commit_list_free(revs->commits);\n revs->commits = reversed;\n revs->reverse = 0;\n@@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)\n return c;\n }\n \n- c = get_revision_internal(revs);\n+ if (revs->max_count_stage)\n+ c = pop_commit(&revs->commits);\n+ else\n+ c = get_revision_internal(revs);\n+\n if (c && revs->graph)\n graph_update(revs->graph, c);\n if (!c) {\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..e157463cb1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -309,6 +309,8 @@ struct rev_info {\n /* special limits */\n int skip_count;\n int max_count;\n+ unsigned int max_count_type:1;\n+ unsigned int max_count_stage:1;\n timestamp_t max_age;\n timestamp_t max_age_as_filter;\n timestamp_t min_age;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..668c231cf1 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,20 @@ test_expect_success 'log --graph with --name-status' '\n test_cmp_graph --name-status tangle..reach\n '\n \n+test_expect_success 'log --max-count-oldest=3 --oneline' '\n+ test_when_finished rm expect &&\n+ git log --oneline | tail -n3 >expect &&\n+ git log --oneline --max-count-oldest=3 >actual &&\n+ test_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n+ test_when_finished rm expect &&\n+ git log --oneline | tail -n3 | tac >expect &&\n+ git log --oneline --max-count-oldest=3 --reverse >actual &&\n+ test_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"542799","messageId":"7250e6c1-633e-417b-aacb-94e35d240d3f@kdbg.org","threadId":"65511","inReplyTo":"ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-05-06T06:45:36Z","receivedAt":"2026-05-06T06:45:48Z","isPatch":true,"body":"Am 05.05.26 um 23:54 schrieb Mirko Faina:\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to do something like\n> \n> git log $(git rev-list HEAD | tail -n N | head -n 1)\n> \n> This is not very user-friendly.\n> \n> Teach get_revision() the --max-count-oldest option.\n> \n> Signed-off-by: Mirko Faina \n> ---\n> Since v5 I've reworded the commit message and rewrote the docs for\n> --max-count-oldest to be clearer on its functionality.\n> \n> Documentation/rev-list-options.adoc | 5 ++\n> revision.c | 77 +++++++++++++++++++++++++++--\n> revision.h | 2 +\n> t/t4202-log.sh | 14 ++++++\n> 4 files changed, 95 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..9f857cabcc 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.\n> `--max-count=`::\n> Limit the output to __ commits.\n> \n> +`--max-count-oldest=`::\n> + Just like `--max-count=`, it limits the output to __\n> + commits. But instead of limiting to the first __ commits it\n> + limits to the last __ commits.\n> +\n\n\"Just like --max-count\" is a surprising addendum in this sentence,\nbecause the only thing they have in common is the limiting of commits,\nwhich it repeats anyway. It's more like \"Unlike --max-count, limits the\noutput to __ last commits.\"\n\nBTW, this makes me think whether this kind of limiting could be\ntriggered by a negative argument to --max-count.\n\n> `--skip=`::\n> Skip __ commits before starting to show the commit output.\n> \n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..3aaa77ced5 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> }\n> \n> if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n\nTo help translators, the usual pattern is to say (here and later)\n\n\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n\t\t\"--max-count\", \"--max-count-oldest\");\n\n> revs->max_count = parse_count(optarg);\n> revs->no_walk = 0;\n> + revs->max_count_type = 0;\n> return argcount;\n> + } else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n> + if (revs->max_count_type == 0 && revs->max_count != -1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n> + if (revs->skip_count > 0)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n> + revs->max_count = parse_count(optarg);\n> + revs->no_walk = 0;\n> + revs->max_count_type = 1;\n> + revs->max_count_stage = 0;\n> } else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n> revs->skip_count = parse_count(optarg);\n> return argcount;\n> } else if ((*arg == '-') && isdigit(arg[1])) {\n> @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n> return c;\n> }\n> \n> +static void retrieve_oldest_commits(struct rev_info *revs,\n> + struct commit_list **queue)\n> +{\n> + struct commit *c;\n> + int max_count = revs->max_count;\n> + int queuei_count = 0;\n> + int queueo_count = 0;\n> + struct commit_list *queueo = NULL;\n> + struct commit_list *queuei = NULL;\n> + struct commit_list *reversed_queue = NULL;\n> +\n> + revs->max_count = -1;\n> + while ((c = get_revision_internal(revs))) {\n> + c->object.flags &= ~SHOWN;\n> + commit_list_insert(c, &queuei);\n> + queuei_count++;\n> + while (queuei_count + queueo_count > max_count) {\n> + if (!queueo_count) {\n> + while (queuei_count > 0) {\n> + c = pop_commit(&queuei);\n> + queuei_count--;\n> + commit_list_insert(c, &queueo);\n> + queueo_count++;\n> + }\n> + }\n> + pop_commit(&queueo);\n> + queueo_count--;\n> + }\n> + }\n> +\n> + while ((c = pop_commit(&queueo)))\n> + commit_list_insert(c, &reversed_queue);\n> + while ((c = pop_commit(&queuei)))\n> + commit_list_insert(c, &queueo);\n> + while ((c = pop_commit(&queueo)))\n> + commit_list_insert(c, &reversed_queue);\n> +\n> + while ((c = pop_commit(&reversed_queue)))\n> + commit_list_insert(c, queue);\n> +}\n> +\n> struct commit *get_revision(struct rev_info *revs)\n> {\n> struct commit *c;\n> struct commit_list *reversed;\n> + struct commit_list *queue = NULL;\n> +\n> + if (revs->max_count_type == 1 && !revs->max_count_stage) {\n> + retrieve_oldest_commits(revs, &queue);\n> + commit_list_free(revs->commits);\n> + revs->commits = queue;\n> + revs->max_count_stage = 1;\n> + }\n> \n> if (revs->reverse) {\n> reversed = NULL;\n> - while ((c = get_revision_internal(revs)))\n> - commit_list_insert(c, &reversed);\n> + if (revs->max_count_type == 1)\n> + while ((c = pop_commit(&revs->commits)))\n> + commit_list_insert(c, &reversed);\n> + else\n> + while ((c = get_revision_internal(revs)))\n> + commit_list_insert(c, &reversed);\n> commit_list_free(revs->commits);\n> revs->commits = reversed;\n> revs->reverse = 0;\n\nI would have expected that this kind of commit counting is handled at\nthe same spot where --max-count is handled, i.e., in\nget_revision_internal(). It could make a difference in combination with\nsorting options, --boundary, and --graph. The goal is that --max-count\nand --max-count-oldest behave the same in this regard. (But I am in no\nway an expert of the revision walker.)\n\n> @@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)\n> return c;\n> }\n> \n> - c = get_revision_internal(revs);\n> + if (revs->max_count_stage)\n> + c = pop_commit(&revs->commits);\n> + else\n> + c = get_revision_internal(revs);\n> +\n> if (c && revs->graph)\n> graph_update(revs->graph, c);\n> if (!c) {\n> diff --git a/revision.h b/revision.h\n> index 584f1338b5..e157463cb1 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -309,6 +309,8 @@ struct rev_info {\n> /* special limits */\n> int skip_count;\n> int max_count;\n> + unsigned int max_count_type:1;\n> + unsigned int max_count_stage:1;\n> timestamp_t max_age;\n> timestamp_t max_age_as_filter;\n> timestamp_t min_age;\n-- Hannes\n\n"},{"id":"542808","messageId":"afs2QVHerGLALFcl@exploit","threadId":"65511","inReplyTo":"7250e6c1-633e-417b-aacb-94e35d240d3f@kdbg.org","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-06T12:54:19Z","receivedAt":"2026-05-06T12:54:28Z","isPatch":true,"body":"On Wed, May 06, 2026 at 08:45:36AM +0200, Johannes Sixt wrote:\n> > +`--max-count-oldest=`::\n> > + Just like `--max-count=`, it limits the output to __\n> > + commits. But instead of limiting to the first __ commits it\n> > + limits to the last __ commits.\n> > +\n> \n> \"Just like --max-count\" is a surprising addendum in this sentence,\n> because the only thing they have in common is the limiting of commits,\n> which it repeats anyway. It's more like \"Unlike --max-count, limits the\n> output to __ last commits.\"\n\nWill fix in v7.\n\n> BTW, this makes me think whether this kind of limiting could be\n> triggered by a negative argument to --max-count.\n\nWould be a good idea if it weren't for the fact that --max-count < 0 has\nfor a long time acted like no max count. I'd imagine many could be\nasssuming this behaviour in their scripts.\n\n> > `--skip=`::\n> > Skip __ commits before starting to show the commit output.\n> > \n> > diff --git a/revision.c b/revision.c\n> > index 599b3a66c3..3aaa77ced5 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> > }\n> > \n> > if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> > + if (revs->max_count_type == 1)\n> > + die(_(\"can't use --max-count with --max-count-oldest\"));\n> \n> To help translators, the usual pattern is to say (here and later)\n> \n> \tdie(_(\"options '%s' and '%s' cannot be used together\"),\n> \t\t\"--max-count\", \"--max-count-oldest\");\n\nWill do.\n\n> > @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n> > return c;\n> > }\n> > \n> > +static void retrieve_oldest_commits(struct rev_info *revs,\n> > + struct commit_list **queue)\n> > +{\n> > + struct commit *c;\n> > + int max_count = revs->max_count;\n> > + int queuei_count = 0;\n> > + int queueo_count = 0;\n> > + struct commit_list *queueo = NULL;\n> > + struct commit_list *queuei = NULL;\n> > + struct commit_list *reversed_queue = NULL;\n> > +\n> > + revs->max_count = -1;\n> > + while ((c = get_revision_internal(revs))) {\n> > + c->object.flags &= ~SHOWN;\n> > + commit_list_insert(c, &queuei);\n> > + queuei_count++;\n> > + while (queuei_count + queueo_count > max_count) {\n> > + if (!queueo_count) {\n> > + while (queuei_count > 0) {\n> > + c = pop_commit(&queuei);\n> > + queuei_count--;\n> > + commit_list_insert(c, &queueo);\n> > + queueo_count++;\n> > + }\n> > + }\n> > + pop_commit(&queueo);\n> > + queueo_count--;\n> > + }\n> > + }\n> > +\n> > + while ((c = pop_commit(&queueo)))\n> > + commit_list_insert(c, &reversed_queue);\n> > + while ((c = pop_commit(&queuei)))\n> > + commit_list_insert(c, &queueo);\n> > + while ((c = pop_commit(&queueo)))\n> > + commit_list_insert(c, &reversed_queue);\n> > +\n> > + while ((c = pop_commit(&reversed_queue)))\n> > + commit_list_insert(c, queue);\n> > +}\n> > +\n> > struct commit *get_revision(struct rev_info *revs)\n> > {\n> > struct commit *c;\n> > struct commit_list *reversed;\n> > + struct commit_list *queue = NULL;\n> > +\n> > + if (revs->max_count_type == 1 && !revs->max_count_stage) {\n> > + retrieve_oldest_commits(revs, &queue);\n> > + commit_list_free(revs->commits);\n> > + revs->commits = queue;\n> > + revs->max_count_stage = 1;\n> > + }\n> > \n> > if (revs->reverse) {\n> > reversed = NULL;\n> > - while ((c = get_revision_internal(revs)))\n> > - commit_list_insert(c, &reversed);\n> > + if (revs->max_count_type == 1)\n> > + while ((c = pop_commit(&revs->commits)))\n> > + commit_list_insert(c, &reversed);\n> > + else\n> > + while ((c = get_revision_internal(revs)))\n> > + commit_list_insert(c, &reversed);\n> > commit_list_free(revs->commits);\n> > revs->commits = reversed;\n> > revs->reverse = 0;\n> \n> I would have expected that this kind of commit counting is handled at\n> the same spot where --max-count is handled, i.e., in\n> get_revision_internal(). It could make a difference in combination with\n> sorting options, --boundary, and --graph. The goal is that --max-count\n> and --max-count-oldest behave the same in this regard. (But I am in no\n> way an expert of the revision walker.)\n\nIt doesn't affect sorting options and the graph output by itself is\nhandled by setting the commits as not shown when we store them, but the\nboundary option does break.\n\nWill fix in v7.\n\nThank you\n"},{"id":"542836","messageId":"87o6ireftj.fsf@gitster.g","threadId":"65511","inReplyTo":"afs2QVHerGLALFcl@exploit","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-07T09:20:40Z","receivedAt":"2026-05-07T09:20:46Z","isPatch":true,"body":"Mirko Faina writes:\n\n>> BTW, this makes me think whether this kind of limiting could be\n>> triggered by a negative argument to --max-count.\n>\n> Would be a good idea if it weren't for the fact that --max-count < 0 has\n> for a long time acted like no max count. I'd imagine many could be\n> asssuming this behaviour in their scripts.\n\nMany? I am not sure. \n\nWhat do these script try to achieve by having \"--max-count=-1\"? It\nwould be to defeat --max-count= coming from elsewhere, but where?\n"},{"id":"542881","messageId":"af0kZUczSPJrQcU6@exploit","threadId":"65511","inReplyTo":"87o6ireftj.fsf@gitster.g","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-08T00:09:30Z","receivedAt":"2026-05-08T00:09:41Z","isPatch":true,"body":"On Thu, May 07, 2026 at 06:20:40PM +0900, Junio C Hamano wrote:\n> Mirko Faina writes:\n> \n> >> BTW, this makes me think whether this kind of limiting could be\n> >> triggered by a negative argument to --max-count.\n> >\n> > Would be a good idea if it weren't for the fact that --max-count < 0 has\n> > for a long time acted like no max count. I'd imagine many could be\n> > asssuming this behaviour in their scripts.\n> \n> Many? I am not sure. \n> \n> What do these script try to achieve by having \"--max-count=-1\"? It\n> would be to defeat --max-count= coming from elsewhere, but where?\n\nIt's not necessarely to defeat a previous use of --max-count. If I know\nthat --max-count behaves in as certain way, in this case the same as not\nhaving it when below 0, I can always have it in the command that I have\nto run and just work with the argument (of course one would do this only\nif it is easier to work with just the argument). That way I don't have\nto conditionally add the option if it has to be enabled. Something like\n\n\tN=should_use_max_count_and_if_so_much_to_limit\n\tgit rev-list --max-count=$N\n\nOf course these script make use of behaviour that is not documented and\nmight not even be intended, so really their fault if it breaks.\n\nThis has been the behaviour of --max-count for a long time so I'm\nassuming that there is a possibility that it will break many scripts.\nBut like I said, their fault if it breaks, if you think its not that\nwidespread I'll get rid of --max-count-oldest.\n"},{"id":"542939","messageId":"xmqqjytcdeys.fsf@gitster.g","threadId":"65511","inReplyTo":"2f71a00b035e25b971641b77a6fa7626f1e2459c.1777578676.git.mroik@delayed.space","subject":"Re: [PATCH v5] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-09T11:01:15Z","receivedAt":"2026-05-09T11:01:18Z","isPatch":true,"body":"Mirko Faina writes:\n\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to combine --max-count with\n> --skip. This is not very user-friendly.\n> ...\n> +test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n> +\ttest_when_finished rm expect &&\n> +\tgit log --oneline | tail -n3 | tac >expect &&\n> +\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n> +\ttest_cmp expect actual\n> +'\n\n\"tac\" is not portable, and breaks macOS CI jobs.\n\n https://github.com/git/git/actions/runs/25591146540/job/75128929633#step:4:2058\n\nWouldn't\n\n git log --oneline --reverse | head -n3 >expect\n\nbe equivalent?\n"},{"id":"542940","messageId":"2409449.ElGaqSPkdT@piment-oiseau","threadId":"65511","inReplyTo":"ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Jean-Noël AVILA","fromEmail":"jn.avila@free.fr","sentAt":"2026-05-09T12:46:26Z","receivedAt":"2026-05-09T12:46:45Z","isPatch":true,"body":"On Tuesday, 5 May 2026 23:54:56 CEST Mirko Faina wrote:\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to do something like\n> \n> git log $(git rev-list HEAD | tail -n N | head -n 1)\n> \n> This is not very user-friendly.\n> \n> Teach get_revision() the --max-count-oldest option.\n> \n> Signed-off-by: Mirko Faina \n> ---\n> Since v5 I've reworded the commit message and rewrote the docs for\n> --max-count-oldest to be clearer on its functionality.\n> \n> Documentation/rev-list-options.adoc | 5 ++\n> revision.c | 77 +++++++++++++++++++++++++++--\n> revision.h | 2 +\n> t/t4202-log.sh | 14 ++++++\n> 4 files changed, 95 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/rev-list-options.adoc\n> b/Documentation/rev-list-options.adoc index 2d195a1474..9f857cabcc 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.\n> `--max-count=`::\n> Limit the output to __ commits.\n> \n> +`--max-count-oldest=`::\n> + Just like `--max-count=`, it limits the output to __\n> + commits. But instead of limiting to the first __ commits it\n> + limits to the last __ commits.\n> +\n\nPutting aside the discussion of --max-count= vs --max-count-\noldest=, I do not think that defining --max-count-old with respect with \n--max-count is legible. It would be better to refine the definition of --max-\ncount (i.e. \"Limit the output to the __ first commits\") and just \ndefine --max-count-oldest on its own in the same manner. Referring to another \nentry is only practicable when it avoids repeating a long explanation. \nOtherwise, each entry's explanation should be as self-contained as possible.\n\n> `--skip=`::\n> Skip __ commits before starting to show the commit output.\n> \n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..3aaa77ced5 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info \n*revs, int\n> argc, const char **arg }\n> \n> if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n> revs->max_count = parse_count(optarg);\n> revs->no_walk = 0;\n> + revs->max_count_type = 0;\n> return argcount;\n> + } else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, \n&optarg))) {\n> + if (revs->max_count_type == 0 && revs->max_count != -1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n> + if (revs->skip_count > 0)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n\nTypo here (con't → can't). In any case, please prefer \ndie_for_incompatible_opt2, to uniformize the messages and limit the number of \ntranslation strings.\n\nAdding a test for these incompatibilities would be great too.\n\n> + revs->max_count = parse_count(optarg);\n> + revs->no_walk = 0;\n> + revs->max_count_type = 1;\n> + revs->max_count_stage = 0;\n> } else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n\nditto\n\n> revs->skip_count = parse_count(optarg);\n> return argcount;\n> } else if ((*arg == '-') && isdigit(arg[1])) {\n> @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct \nrev_info\n> *revs) return c;\n> }\n> \n\n\n\n"},{"id":"542951","messageId":"xmqqcxz4b8mc.fsf@gitster.g","threadId":"65511","inReplyTo":"ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-09T21:01:15Z","receivedAt":"2026-05-09T21:01:19Z","isPatch":true,"body":"Mirko Faina writes:\n\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to do something like\n>\n> git log $(git rev-list HEAD | tail -n N | head -n 1)\n>\n> This is not very user-friendly.\n>\n> Teach get_revision() the --max-count-oldest option.\n>\n> Signed-off-by: Mirko Faina \n> ---\n> Since v5 I've reworded the commit message and rewrote the docs for\n> --max-count-oldest to be clearer on its functionality.\n>\n> Documentation/rev-list-options.adoc | 5 ++\n> revision.c | 77 +++++++++++++++++++++++++++--\n> revision.h | 2 +\n> t/t4202-log.sh | 14 ++++++\n> 4 files changed, 95 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 2d195a1474..9f857cabcc 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.\n> `--max-count=`::\n> Limit the output to __ commits.\n> \n> +`--max-count-oldest=`::\n> + Just like `--max-count=`, it limits the output to __\n> + commits. But instead of limiting to the first __ commits it\n> + limits to the last __ commits.\n> +\n> `--skip=`::\n> Skip __ commits before starting to show the commit output.\n\nSaving this message to a file and grepping for a tab finds nothing,\nwhich indicates that the patch seems to be unsalvageably whitespace\nbroken, given that most of the context lines should use tabs for\nindent.\n\nWhat did you do differently this time? The previous rounds did not\nhave this problem.\n\n\n> diff --git a/revision.c b/revision.c\n> index 599b3a66c3..3aaa77ced5 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n> }\n> \n> if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n> revs->max_count = parse_count(optarg);\n> revs->no_walk = 0;\n> + revs->max_count_type = 0;\n> return argcount;\n> + } else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n> + if (revs->max_count_type == 0 && revs->max_count != -1)\n> + die(_(\"can't use --max-count with --max-count-oldest\"));\n> + if (revs->skip_count > 0)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n> + revs->max_count = parse_count(optarg);\n> + revs->no_walk = 0;\n> + revs->max_count_type = 1;\n> + revs->max_count_stage = 0;\n> } else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n> + if (revs->max_count_type == 1)\n> + die(_(\"con't use --max-count-oldest with --skip\"));\n> revs->skip_count = parse_count(optarg);\n> return argcount;\n> } else if ((*arg == '-') && isdigit(arg[1])) {\n> @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n> return c;\n> }\n> \n> +static void retrieve_oldest_commits(struct rev_info *revs,\n> + struct commit_list **queue)\n> +{\n> + struct commit *c;\n> + int max_count = revs->max_count;\n> + int queuei_count = 0;\n> + int queueo_count = 0;\n> + struct commit_list *queueo = NULL;\n> + struct commit_list *queuei = NULL;\n> + struct commit_list *reversed_queue = NULL;\n> +\n> + revs->max_count = -1;\n> + while ((c = get_revision_internal(revs))) {\n> + c->object.flags &= ~SHOWN;\n> + commit_list_insert(c, &queuei);\n> + queuei_count++;\n> + while (queuei_count + queueo_count > max_count) {\n> + if (!queueo_count) {\n> + while (queuei_count > 0) {\n> + c = pop_commit(&queuei);\n> + queuei_count--;\n> + commit_list_insert(c, &queueo);\n> + queueo_count++;\n> + }\n> + }\n> + pop_commit(&queueo);\n> + queueo_count--;\n> + }\n> + }\n> +\n> + while ((c = pop_commit(&queueo)))\n> + commit_list_insert(c, &reversed_queue);\n> + while ((c = pop_commit(&queuei)))\n> + commit_list_insert(c, &queueo);\n> + while ((c = pop_commit(&queueo)))\n> + commit_list_insert(c, &reversed_queue);\n> +\n> + while ((c = pop_commit(&reversed_queue)))\n> + commit_list_insert(c, queue);\n> +}\n> +\n> struct commit *get_revision(struct rev_info *revs)\n> {\n> struct commit *c;\n> struct commit_list *reversed;\n> + struct commit_list *queue = NULL;\n> +\n> + if (revs->max_count_type == 1 && !revs->max_count_stage) {\n> + retrieve_oldest_commits(revs, &queue);\n> + commit_list_free(revs->commits);\n> + revs->commits = queue;\n> + revs->max_count_stage = 1;\n> + }\n> \n> if (revs->reverse) {\n> reversed = NULL;\n> - while ((c = get_revision_internal(revs)))\n> - commit_list_insert(c, &reversed);\n> + if (revs->max_count_type == 1)\n> + while ((c = pop_commit(&revs->commits)))\n> + commit_list_insert(c, &reversed);\n> + else\n> + while ((c = get_revision_internal(revs)))\n> + commit_list_insert(c, &reversed);\n> commit_list_free(revs->commits);\n> revs->commits = reversed;\n> revs->reverse = 0;\n> @@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)\n> return c;\n> }\n> \n> - c = get_revision_internal(revs);\n> + if (revs->max_count_stage)\n> + c = pop_commit(&revs->commits);\n> + else\n> + c = get_revision_internal(revs);\n> +\n> if (c && revs->graph)\n> graph_update(revs->graph, c);\n> if (!c) {\n> diff --git a/revision.h b/revision.h\n> index 584f1338b5..e157463cb1 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -309,6 +309,8 @@ struct rev_info {\n> /* special limits */\n> int skip_count;\n> int max_count;\n> + unsigned int max_count_type:1;\n> + unsigned int max_count_stage:1;\n> timestamp_t max_age;\n> timestamp_t max_age_as_filter;\n> timestamp_t min_age;\n> diff --git a/t/t4202-log.sh b/t/t4202-log.sh\n> index 05cee9e41b..668c231cf1 100755\n> --- a/t/t4202-log.sh\n> +++ b/t/t4202-log.sh\n> @@ -1882,6 +1882,20 @@ test_expect_success 'log --graph with --name-status' '\n> test_cmp_graph --name-status tangle..reach\n> '\n> \n> +test_expect_success 'log --max-count-oldest=3 --oneline' '\n> + test_when_finished rm expect &&\n> + git log --oneline | tail -n3 >expect &&\n> + git log --oneline --max-count-oldest=3 >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n> + test_when_finished rm expect &&\n> + git log --oneline | tail -n3 | tac >expect &&\n> + git log --oneline --max-count-oldest=3 --reverse >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> cat >expect <<-\\EOF\n> * reach\n> |\n"},{"id":"542956","messageId":"af_SX9mQPLxolg4k@exploit","threadId":"65511","inReplyTo":"xmqqjytcdeys.fsf@gitster.g","subject":"Re: [PATCH v5] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-10T00:36:38Z","receivedAt":"2026-05-10T00:36:49Z","isPatch":true,"body":"On Sat, May 09, 2026 at 08:01:15PM +0900, Junio C Hamano wrote:\n> Mirko Faina writes:\n> \n> > --max-count is a commit limiting option sets a maximum amount of commits\n> > to be shown. If a user wants to see only the first N commits of the\n> > history (the oldest commits) they'd have to combine --max-count with\n> > --skip. This is not very user-friendly.\n> > ...\n> > +test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n> > +\ttest_when_finished rm expect &&\n> > +\tgit log --oneline | tail -n3 | tac >expect &&\n> > +\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n> > +\ttest_cmp expect actual\n> > +'\n> \n> \"tac\" is not portable, and breaks macOS CI jobs.\n> \n> https://github.com/git/git/actions/runs/25591146540/job/75128929633#step:4:2058\n> \n> Wouldn't\n> \n> git log --oneline --reverse | head -n3 >expect\n> \n> be equivalent?\n\nYes, will do.\n\nThank you\n"},{"id":"542957","messageId":"af_Td41Mga0Z-MMv@exploit","threadId":"65511","inReplyTo":"2409449.ElGaqSPkdT@piment-oiseau","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-10T00:41:18Z","receivedAt":"2026-05-10T00:41:21Z","isPatch":true,"body":"On Sat, May 09, 2026 at 02:46:26PM +0200, Jean-Noël AVILA wrote:\n> On Tuesday, 5 May 2026 23:54:56 CEST Mirko Faina wrote:\n> > --max-count is a commit limiting option sets a maximum amount of commits\n> > to be shown. If a user wants to see only the first N commits of the\n> > history (the oldest commits) they'd have to do something like\n> > \n> > git log $(git rev-list HEAD | tail -n N | head -n 1)\n> > \n> > This is not very user-friendly.\n> > \n> > Teach get_revision() the --max-count-oldest option.\n> > \n> > Signed-off-by: Mirko Faina \n> > ---\n> > Since v5 I've reworded the commit message and rewrote the docs for\n> > --max-count-oldest to be clearer on its functionality.\n> > \n> > Documentation/rev-list-options.adoc | 5 ++\n> > revision.c | 77 +++++++++++++++++++++++++++--\n> > revision.h | 2 +\n> > t/t4202-log.sh | 14 ++++++\n> > 4 files changed, 95 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/Documentation/rev-list-options.adoc\n> > b/Documentation/rev-list-options.adoc index 2d195a1474..9f857cabcc 100644\n> > --- a/Documentation/rev-list-options.adoc\n> > +++ b/Documentation/rev-list-options.adoc\n> > @@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.\n> > `--max-count=`::\n> > Limit the output to __ commits.\n> > \n> > +`--max-count-oldest=`::\n> > + Just like `--max-count=`, it limits the output to __\n> > + commits. But instead of limiting to the first __ commits it\n> > + limits to the last __ commits.\n> > +\n> \n> Putting aside the discussion of --max-count= vs --max-count-\n> oldest=, I do not think that defining --max-count-old with respect with \n> --max-count is legible. It would be better to refine the definition of --max-\n> count (i.e. \"Limit the output to the __ first commits\") and just \n> define --max-count-oldest on its own in the same manner. Referring to another \n> entry is only practicable when it avoids repeating a long explanation. \n> Otherwise, each entry's explanation should be as self-contained as possible.\n\nWill do in v7.\n\n> > `--skip=`::\n> > Skip __ commits before starting to show the commit output.\n> > \n> > diff --git a/revision.c b/revision.c\n> > index 599b3a66c3..3aaa77ced5 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info \n> *revs, int\n> > argc, const char **arg }\n> > \n> > if ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n> > + if (revs->max_count_type == 1)\n> > + die(_(\"can't use --max-count with --max-count-oldest\"));\n> > revs->max_count = parse_count(optarg);\n> > revs->no_walk = 0;\n> > + revs->max_count_type = 0;\n> > return argcount;\n> > + } else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, \n> &optarg))) {\n> > + if (revs->max_count_type == 0 && revs->max_count != -1)\n> > + die(_(\"can't use --max-count with --max-count-oldest\"));\n> > + if (revs->skip_count > 0)\n> > + die(_(\"con't use --max-count-oldest with --skip\"));\n> \n> Typo here (con't → can't). In any case, please prefer \n> die_for_incompatible_opt2, to uniformize the messages and limit the number of \n> translation strings.\n\nWill do.\n\n> Adding a test for these incompatibilities would be great too.\n\nYes, should've done that sooner. Will do.\n\n> > + revs->max_count = parse_count(optarg);\n> > + revs->no_walk = 0;\n> > + revs->max_count_type = 1;\n> > + revs->max_count_stage = 0;\n> > } else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n> > + if (revs->max_count_type == 1)\n> > + die(_(\"con't use --max-count-oldest with --skip\"));\n> \n> ditto\n\nWill do.\n\n> > revs->skip_count = parse_count(optarg);\n> > return argcount;\n> > } else if ((*arg == '-') && isdigit(arg[1])) {\n> > @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct \n> rev_info\n> > *revs) return c;\n> > }\n> > \n\nThank you\n"},{"id":"542958","messageId":"af_UdLh68wcDURf9@exploit","threadId":"65511","inReplyTo":"xmqqcxz4b8mc.fsf@gitster.g","subject":"Re: [PATCH v6] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-10T00:48:14Z","receivedAt":"2026-05-10T00:48:18Z","isPatch":true,"body":"On Sun, May 10, 2026 at 06:01:15AM +0900, Junio C Hamano wrote:\n> Saving this message to a file and grepping for a tab finds nothing,\n> which indicates that the patch seems to be unsalvageably whitespace\n> broken, given that most of the context lines should use tabs for\n> indent.\n> \n> What did you do differently this time? The previous rounds did not\n> have this problem.\n\nAh yes, I remember retab-bing with expandtab while writing stuff after\nthe commit message. At the time I must've not realized that it retabbed\nthe content of the patch as well (and I didn't even end up needing to\nretab). Sorry about that.\n\nI won't resend v6 as it will be discarded for v7 anyway.\n\nThank you\n"},{"id":"543428","messageId":"463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space","threadId":"65511","inReplyTo":"ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space","subject":"[PATCH v7] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-15T23:29:55Z","receivedAt":"2026-05-15T23:30:28Z","isPatch":true,"body":"--max-count is a commit limiting option sets a maximum amount of commits\nto be shown. If a user wants to see only the first N commits of the\nhistory (the oldest commits) they'd have to do something like\n\n git log $(git rev-list HEAD | tail -n N | head -n 1)\n\nThis is not very user-friendly.\n\nTeach get_revision() the --max-count-oldest option.\n\nSigned-off-by: Mirko Faina \n---\nSince v6 I've simplified the docs, replaced die messages with\ndie_for_incompatible_opt2 and fixed graph output when used with\n--boundary.\n\n Documentation/rev-list-options.adoc | 5 +-\n revision.c | 103 +++++++++++++++++++++++++++-\n revision.h | 2 +\n t/t4202-log.sh | 33 +++++++++\n 4 files changed, 139 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..e8c88d0f1c 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -16,7 +16,10 @@ ordering and formatting options, such as `--reverse`.\n `-`::\n `-n `::\n `--max-count=`::\n-\tLimit the output to __ commits.\n+\tLimit the output to the first __ commits that would be shown.\n+\n+`--max-count-oldest=`::\n+\tLimit the output to the last __ commits that would be shown.\n \n `--skip=`::\n \tSkip __ commits before starting to show the commit output.\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..7fc79049b2 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2339,10 +2339,28 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t}\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 0;\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\tif (revs->skip_count > 0)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\trevs->max_count = parse_count(optarg);\n+\t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 1;\n+\t\trevs->max_count_stage = 0;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->skip_count = parse_count(optarg);\n \t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -4521,15 +4539,83 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_oldest_commits(struct rev_info *revs,\n+\t\t\t\t struct commit_list **queue)\n+{\n+\tstruct commit *c;\n+\tint max_count = revs->max_count;\n+\tint queuei_count = 0;\n+\tint queueo_count = 0;\n+\tstruct commit_list *queueo = NULL;\n+\tstruct commit_list *queuei = NULL;\n+\tstruct commit_list *reversed_queue = NULL;\n+\n+\trevs->max_count = -1;\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\t/*\n+\t\t * We need to reset SHOWN status otherwise --graph breaks.\n+\t\t * It is fine to do, get_revision_internal() doesn't consider\n+\t\t * children commits as they have been already processed and the\n+\t\t * traversal happens only child to parent.\n+\t\t *\n+\t\t * We do this because the --graph machinery relies on the status\n+\t\t * of the parents to decide how the printing will happen.\n+\t\t *\n+\t\t * We can't simply replace this instruction with a\n+\t\t * graph_update() as it doesn't do the actualy printing, we'd\n+\t\t * have to remove any commit that goes over the\n+\t\t * --max-count-oldest limit from revs->graph.\n+\t\t */\n+\t\tc->object.flags &= ~(SHOWN | CHILD_SHOWN);\n+\t\tcommit_list_insert(c, &queuei);\n+\t\tqueuei_count++;\n+\t\twhile (queuei_count + queueo_count > max_count) {\n+\t\t\tif (!queueo_count) {\n+\t\t\t\twhile (queuei_count > 0) {\n+\t\t\t\t\tc = pop_commit(&queuei);\n+\t\t\t\t\tqueuei_count--;\n+\t\t\t\t\tcommit_list_insert(c, &queueo);\n+\t\t\t\t\tqueueo_count++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tpop_commit(&queueo);\n+\t\t\tqueueo_count--;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\twhile ((c = pop_commit(&queuei)))\n+\t\tcommit_list_insert(c, &queueo);\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\n+\twhile ((c = pop_commit(&reversed_queue)))\n+\t\tcommit_list_insert(c, queue);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tstruct commit_list *queue = NULL;\n+\tstruct commit_list *p;\n+\n+\tif (revs->max_count_type == 1 && !revs->max_count_stage) {\n+\t\tretrieve_oldest_commits(revs, &queue);\n+\t\tcommit_list_free(revs->commits);\n+\t\trevs->commits = queue;\n+\t\trevs->max_count_stage = 1;\n+\t}\n \n \tif (revs->reverse) {\n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->max_count_type == 1)\n+\t\t\twhile ((c = pop_commit(&revs->commits)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\telse\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse = 0;\n@@ -4543,7 +4629,18 @@ struct commit *get_revision(struct rev_info *revs)\n \t\treturn c;\n \t}\n \n-\tc = get_revision_internal(revs);\n+\tif (revs->max_count_stage) {\n+\t\tc = pop_commit(&revs->commits);\n+\t\tif (c) {\n+\t\t\tc->object.flags |= SHOWN;\n+\t\t\tif (!(c->object.flags & BOUNDARY))\n+\t\t\t\tfor (p = c->parents; p; p = p->next)\n+\t\t\t\t\tp->item->object.flags |= CHILD_SHOWN;\n+\t\t}\n+\t} else {\n+\t\tc = get_revision_internal(revs);\n+\t}\n+\n \tif (c && revs->graph)\n \t\tgraph_update(revs->graph, c);\n \tif (!c) {\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..e157463cb1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -309,6 +309,8 @@ struct rev_info {\n \t/* special limits */\n \tint skip_count;\n \tint max_count;\n+\tunsigned int max_count_type:1;\n+\tunsigned int max_count_stage:1;\n \ttimestamp_t max_age;\n \ttimestamp_t max_age_as_filter;\n \ttimestamp_t min_age;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..8f2471e7e4 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,39 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+test_expect_success 'log --max-count-oldest=3 --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline | tail -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline --reverse | head -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest with --max-count' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --max-count=3 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest with --skip' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --skip=1 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '\n+\ttest_when_finished rm expect actual &&\n+\tgit log --graph --boundary >expect &&\n+\tgit log --max-count-oldest=1000 --graph --boundary >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"543574","messageId":"agu1rZe0BiNowmBT@exploit","threadId":"65511","inReplyTo":"8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space","subject":"Re: [PATCH v8] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-19T01:04:06Z","receivedAt":"2026-05-19T01:04:10Z","isPatch":true,"body":"On Tue, May 19, 2026 at 02:55:22AM +0200, Mirko Faina wrote:\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to do something like\n> \n> git log $(git rev-list HEAD | tail -n N | head -n 1)\n> \n> This is not very user-friendly.\n> \n> Teach get_revision() the --max-count-oldest option.\n> \n> Signed-off-by: Mirko Faina \n> ---\n> Documentation/rev-list-options.adoc | 5 +-\n> revision.c | 111 +++++++++++++++++++++++++++-\n> revision.h | 2 +\n> t/t4202-log.sh | 41 ++++++++++\n> 4 files changed, 155 insertions(+), 4 deletions(-)\n\nSorry, forgot to write down what changed since v7. There was an issue\nwith the counting as --max-count-oldest counted boundary commits too.\nThat is simply solved by only adding on non boundaries.\n\nThat left another issue, there are now some \"orphaned\" boundaries when\nprinting the graph. In addition to that, because of how the graph\nmachinery works, the graph is now trying to include the parents of the\norphaned boundaries. To fix this we just flip the CHILD_SHOWN flag on\nthe parents of the commit we're discarding.\n\nHopefully this is the last version.\n"},{"id":"543575","messageId":"8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space","threadId":"65511","inReplyTo":"463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space","subject":"[PATCH v8] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-19T00:55:22Z","receivedAt":"2026-05-19T01:05:15Z","isPatch":true,"body":"--max-count is a commit limiting option sets a maximum amount of commits\nto be shown. If a user wants to see only the first N commits of the\nhistory (the oldest commits) they'd have to do something like\n\n git log $(git rev-list HEAD | tail -n N | head -n 1)\n\nThis is not very user-friendly.\n\nTeach get_revision() the --max-count-oldest option.\n\nSigned-off-by: Mirko Faina \n---\n Documentation/rev-list-options.adoc | 5 +-\n revision.c | 111 +++++++++++++++++++++++++++-\n revision.h | 2 +\n t/t4202-log.sh | 41 ++++++++++\n 4 files changed, 155 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..e8c88d0f1c 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -16,7 +16,10 @@ ordering and formatting options, such as `--reverse`.\n `-`::\n `-n `::\n `--max-count=`::\n-\tLimit the output to __ commits.\n+\tLimit the output to the first __ commits that would be shown.\n+\n+`--max-count-oldest=`::\n+\tLimit the output to the last __ commits that would be shown.\n \n `--skip=`::\n \tSkip __ commits before starting to show the commit output.\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..5d53db3152 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2339,10 +2339,28 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t}\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 0;\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\tif (revs->skip_count > 0)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\trevs->max_count = parse_count(optarg);\n+\t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 1;\n+\t\trevs->max_count_stage = 0;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->skip_count = parse_count(optarg);\n \t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -4521,15 +4539,91 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_oldest_commits(struct rev_info *revs,\n+\t\t\t\t struct commit_list **queue)\n+{\n+\tstruct commit *c;\n+\tint max_count = revs->max_count;\n+\tint queuei_count = 0;\n+\tint queueo_count = 0;\n+\tstruct commit_list *queueo = NULL;\n+\tstruct commit_list *queuei = NULL;\n+\tstruct commit_list *reversed_queue = NULL;\n+\tstruct commit_list *p;\n+\n+\trevs->max_count = -1;\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\t/*\n+\t\t * We need to reset SHOWN status otherwise --graph breaks.\n+\t\t * It is fine to do, get_revision_internal() doesn't consider\n+\t\t * children commits as they have been already processed and the\n+\t\t * traversal happens only child to parent.\n+\t\t *\n+\t\t * We do this because the --graph machinery relies on the status\n+\t\t * of the parents to decide how the printing will happen.\n+\t\t *\n+\t\t * We can't simply replace this instruction with a\n+\t\t * graph_update() as it doesn't do the actualy printing, we'd\n+\t\t * have to remove any commit that goes over the\n+\t\t * --max-count-oldest limit from revs->graph.\n+\t\t */\n+\t\tc->object.flags &= ~(SHOWN | CHILD_SHOWN);\n+\t\tcommit_list_insert(c, &queuei);\n+\t\tif (!(c->object.flags & BOUNDARY))\n+\t\t\tqueuei_count++;\n+\t\twhile (queuei_count + queueo_count > max_count) {\n+\t\t\tif (!queueo_count) {\n+\t\t\t\twhile ((c = pop_commit(&queuei))) {\n+\t\t\t\t\tcommit_list_insert(c, &queueo);\n+\t\t\t\t\tqueueo_count++;\n+\t\t\t\t}\n+\t\t\t\tqueuei_count = 0;\n+\t\t\t}\n+\t\t\tc = pop_commit(&queueo);\n+\t\t\tqueueo_count--;\n+\t\t\t/* We need to do this otherwise we'll discard the\n+\t\t\t * commits that go over the --max-count-oldest limit but\n+\t\t\t * not their respective boundaries. This matters only if\n+\t\t\t * we're discarding the commit right before the boundary.\n+\t\t\t */\n+\t\t\tfor (p = c->parents; p; p = p->next)\n+\t\t\t\tp->item->object.flags &= ~CHILD_SHOWN;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\twhile ((c = pop_commit(&queuei)))\n+\t\tcommit_list_insert(c, &queueo);\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\n+\twhile ((c = pop_commit(&reversed_queue)))\n+\t\tcommit_list_insert(c, queue);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tstruct commit_list *queue = NULL;\n+\tstruct commit_list *p;\n+\n+\tif (revs->max_count_type == 1 && !revs->max_count_stage) {\n+\t\tretrieve_oldest_commits(revs, &queue);\n+\t\tcommit_list_free(revs->commits);\n+\t\trevs->commits = queue;\n+\t\trevs->max_count_stage = 1;\n+\t}\n \n \tif (revs->reverse) {\n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->max_count_type == 1)\n+\t\t\twhile ((c = pop_commit(&revs->commits)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\telse\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse = 0;\n@@ -4543,7 +4637,18 @@ struct commit *get_revision(struct rev_info *revs)\n \t\treturn c;\n \t}\n \n-\tc = get_revision_internal(revs);\n+\tif (revs->max_count_stage) {\n+\t\tc = pop_commit(&revs->commits);\n+\t\tif (c) {\n+\t\t\tc->object.flags |= SHOWN;\n+\t\t\tif (!(c->object.flags & BOUNDARY))\n+\t\t\t\tfor (p = c->parents; p; p = p->next)\n+\t\t\t\t\tp->item->object.flags |= CHILD_SHOWN;\n+\t\t}\n+\t} else {\n+\t\tc = get_revision_internal(revs);\n+\t}\n+\n \tif (c && revs->graph)\n \t\tgraph_update(revs->graph, c);\n \tif (!c) {\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..e157463cb1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -309,6 +309,8 @@ struct rev_info {\n \t/* special limits */\n \tint skip_count;\n \tint max_count;\n+\tunsigned int max_count_type:1;\n+\tunsigned int max_count_stage:1;\n \ttimestamp_t max_age;\n \ttimestamp_t max_age_as_filter;\n \ttimestamp_t min_age;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..c3c1b862d3 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,47 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+test_expect_success 'log --max-count-oldest=3 --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline | tail -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline --reverse | head -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest with --max-count' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --max-count=3 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest with --skip' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --skip=1 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '\n+\ttest_when_finished rm expect actual &&\n+\tgit log --graph --boundary >expect &&\n+\tgit log --max-count-oldest=1000 --graph --boundary >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --oneline --graph --boundary --max-count-oldest=1' '\n+\ttest_when_finished rm actual &&\n+\techo 2 >expect &&\n+\tgit log --oneline --graph --boundary --max-count-oldest=1 HEAD~1..HEAD \\\n+\t| wc -l >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0\n\n"},{"id":"543594","messageId":"xmqqse7n7w2i.fsf@gitster.g","threadId":"65511","inReplyTo":"463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space","subject":"Re: [PATCH v7] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-19T06:27:49Z","receivedAt":"2026-05-19T06:27:52Z","isPatch":true,"body":"Mirko Faina writes:\n\n> +\t\t * graph_update() as it doesn't do the actualy printing, we'd\n\n\"actually\"?\n\n"},{"id":"543637","messageId":"agxBvWQnZiRJZwNC@exploit","threadId":"65511","inReplyTo":"xmqqse7n7w2i.fsf@gitster.g","subject":"Re: [PATCH v7] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-19T10:57:47Z","receivedAt":"2026-05-19T10:57:52Z","isPatch":true,"body":"On Tue, May 19, 2026 at 03:27:49PM +0900, Junio C Hamano wrote:\n> Mirko Faina writes:\n> \n> > +\t\t * graph_update() as it doesn't do the actualy printing, we'd\n> \n> \"actually\"?\n\nWhoops, it should be \"actual\".\n\nThank you\n"},{"id":"543731","messageId":"xmqq7boy4o05.fsf@gitster.g","threadId":"65511","inReplyTo":"8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space","subject":"Re: [PATCH v8] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-20T06:02:34Z","receivedAt":"2026-05-20T06:02:37Z","isPatch":true,"body":"Mirko Faina writes:\n\n> --max-count is a commit limiting option sets a maximum amount of commits\n> to be shown. If a user wants to see only the first N commits of the\n> history (the oldest commits) they'd have to do something like\n>\n> git log $(git rev-list HEAD | tail -n N | head -n 1)\n>\n> This is not very user-friendly.\n>\n> Teach get_revision() the --max-count-oldest option.\n>\n> Signed-off-by: Mirko Faina \n> ---\n\nThis breaks CI\n\n https://github.com/git/git/actions/runs/26138986677/job/76880268854#step:4:2072\n\nSquash something like this to fix.\n\n--- >8 ---\nSubject: [PATCH] SQUASH??? test portability and other fixes\n\n* \"test_when_finished\" should use \"rm -f\", not an error-detecting\n \"rm\", as the execution may not have reached to the point to create\n the \"actual\" file it is removing.\n\n* Do not hide exit status of \"git log\" by piping its output into\n another process.\n\n* Do not expect output of \"wc -l\" is portable. macOS puts extra\n whitespaces in front, while GNU/Linux does not.\n---\n t/t4202-log.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex c3c1b862d3..75edb0eb38 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1916,11 +1916,10 @@ test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '\n '\n \n test_expect_success 'log --oneline --graph --boundary --max-count-oldest=1' '\n-\ttest_when_finished rm actual &&\n-\techo 2 >expect &&\n-\tgit log --oneline --graph --boundary --max-count-oldest=1 HEAD~1..HEAD \\\n-\t| wc -l >actual &&\n-\ttest_cmp expect actual\n+\ttest_when_finished rm -f actual &&\n+\tgit log --oneline --graph --boundary --max-count-oldest=1 \\\n+\t\tHEAD~1..HEAD >actual &&\n+\ttest_line_count = 2 actual\n '\n \n cat >expect <<-\\EOF\n-- \n2.54.0-398-ga4b2d32071\n\n"},{"id":"543746","messageId":"ag3kJ_xKY6584De4@exploit","threadId":"65511","inReplyTo":"xmqq7boy4o05.fsf@gitster.g","subject":"Re: [PATCH v8] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-05-20T16:42:36Z","receivedAt":"2026-05-20T16:42:46Z","isPatch":true,"body":"On Wed, May 20, 2026 at 03:02:34PM +0900, Junio C Hamano wrote:\n> Mirko Faina writes:\n> \n> > --max-count is a commit limiting option sets a maximum amount of commits\n> > to be shown. If a user wants to see only the first N commits of the\n> > history (the oldest commits) they'd have to do something like\n> >\n> > git log $(git rev-list HEAD | tail -n N | head -n 1)\n> >\n> > This is not very user-friendly.\n> >\n> > Teach get_revision() the --max-count-oldest option.\n> >\n> > Signed-off-by: Mirko Faina \n> > ---\n> \n> This breaks CI\n> \n> https://github.com/git/git/actions/runs/26138986677/job/76880268854#step:4:2072\n> \n> Squash something like this to fix.\n> \n> --- >8 ---\n> Subject: [PATCH] SQUASH??? test portability and other fixes\n> \n> * \"test_when_finished\" should use \"rm -f\", not an error-detecting\n> \"rm\", as the execution may not have reached to the point to create\n> the \"actual\" file it is removing.\n> \n> * Do not hide exit status of \"git log\" by piping its output into\n> another process.\n> \n> * Do not expect output of \"wc -l\" is portable. macOS puts extra\n> whitespaces in front, while GNU/Linux does not.\n> ---\n> t/t4202-log.sh | 9 ++++-----\n> 1 file changed, 4 insertions(+), 5 deletions(-)\n\nSorry about that. And thank you for the fix.\n"},{"id":"544428","messageId":"xmqq4ijm3p2x.fsf@gitster.g","threadId":"65511","inReplyTo":"ag3kJ_xKY6584De4@exploit","subject":"[PATCH v9] revision.c: implement --max-count-oldest","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-01T21:53:10Z","receivedAt":"2026-06-01T21:53:13Z","isPatch":true,"body":"Mirko Faina writes:\n\n> On Wed, May 20, 2026 at 03:02:34PM +0900, Junio C Hamano wrote:\n>> \n>> This breaks CI\n>> \n>> https://github.com/git/git/actions/runs/26138986677/job/76880268854#step:4:2072\n>> \n>> Squash something like this to fix.\n>> ...\n>\n> Sorry about that. And thank you for the fix.\n\nIt has been a while, and we saw no further comments by other\nreviewers.\n\nPerhaps we should declare a victory and mark the topic for 'next'.\n\n------ >8 ------\nFrom: Mirko Faina \nDate: Tue, 19 May 2026 02:55:22 +0200\n\n\"--max-count\" is a commit limiting option and sets a maximum amount\nof commits to be shown. If a user wants to see only the first N\ncommits of the history (the oldest commits) they'd have to do\nsomething like\n\n git log $(git rev-list HEAD | tail -n N | head -n 1)\n\nThis is not very user-friendly.\n\nTeach get_revision() the --max-count-oldest option.\n\nSigned-off-by: Mirko Faina \n[jc: fixed up t4202 ]\nSigned-off-by: Junio C Hamano \n---\n Documentation/rev-list-options.adoc | 5 +-\n revision.c | 111 +++++++++++++++++++++++++++-\n revision.h | 2 +\n t/t4202-log.sh | 40 ++++++++++\n 4 files changed, 154 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 2d195a1474..e8c88d0f1c 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -16,7 +16,10 @@ ordering and formatting options, such as `--reverse`.\n `-`::\n `-n `::\n `--max-count=`::\n-\tLimit the output to __ commits.\n+\tLimit the output to the first __ commits that would be shown.\n+\n+`--max-count-oldest=`::\n+\tLimit the output to the last __ commits that would be shown.\n \n `--skip=`::\n \tSkip __ commits before starting to show the commit output.\ndiff --git a/revision.c b/revision.c\nindex 599b3a66c3..5d53db3152 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2339,10 +2339,28 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t}\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->max_count = parse_count(optarg);\n \t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 0;\n \t\treturn argcount;\n+\t} else if ((argcount = parse_long_opt(\"max-count-oldest\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 0 && revs->max_count != -1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--max-count\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\tif (revs->skip_count > 0)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n+\t\trevs->max_count = parse_count(optarg);\n+\t\trevs->no_walk = 0;\n+\t\trevs->max_count_type = 1;\n+\t\trevs->max_count_stage = 0;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n+\t\tif (revs->max_count_type == 1)\n+\t\t\tdie_for_incompatible_opt2(1, \"--skip\", 1,\n+\t\t\t\t\t\t \"--max-count-oldest\");\n \t\trevs->skip_count = parse_count(optarg);\n \t\treturn argcount;\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n@@ -4521,15 +4539,91 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \treturn c;\n }\n \n+static void retrieve_oldest_commits(struct rev_info *revs,\n+\t\t\t\t struct commit_list **queue)\n+{\n+\tstruct commit *c;\n+\tint max_count = revs->max_count;\n+\tint queuei_count = 0;\n+\tint queueo_count = 0;\n+\tstruct commit_list *queueo = NULL;\n+\tstruct commit_list *queuei = NULL;\n+\tstruct commit_list *reversed_queue = NULL;\n+\tstruct commit_list *p;\n+\n+\trevs->max_count = -1;\n+\twhile ((c = get_revision_internal(revs))) {\n+\t\t/*\n+\t\t * We need to reset SHOWN status otherwise --graph breaks.\n+\t\t * It is fine to do, get_revision_internal() doesn't consider\n+\t\t * children commits as they have been already processed and the\n+\t\t * traversal happens only child to parent.\n+\t\t *\n+\t\t * We do this because the --graph machinery relies on the status\n+\t\t * of the parents to decide how the printing will happen.\n+\t\t *\n+\t\t * We can't simply replace this instruction with a\n+\t\t * graph_update() as it doesn't do the actualy printing, we'd\n+\t\t * have to remove any commit that goes over the\n+\t\t * --max-count-oldest limit from revs->graph.\n+\t\t */\n+\t\tc->object.flags &= ~(SHOWN | CHILD_SHOWN);\n+\t\tcommit_list_insert(c, &queuei);\n+\t\tif (!(c->object.flags & BOUNDARY))\n+\t\t\tqueuei_count++;\n+\t\twhile (queuei_count + queueo_count > max_count) {\n+\t\t\tif (!queueo_count) {\n+\t\t\t\twhile ((c = pop_commit(&queuei))) {\n+\t\t\t\t\tcommit_list_insert(c, &queueo);\n+\t\t\t\t\tqueueo_count++;\n+\t\t\t\t}\n+\t\t\t\tqueuei_count = 0;\n+\t\t\t}\n+\t\t\tc = pop_commit(&queueo);\n+\t\t\tqueueo_count--;\n+\t\t\t/* We need to do this otherwise we'll discard the\n+\t\t\t * commits that go over the --max-count-oldest limit but\n+\t\t\t * not their respective boundaries. This matters only if\n+\t\t\t * we're discarding the commit right before the boundary.\n+\t\t\t */\n+\t\t\tfor (p = c->parents; p; p = p->next)\n+\t\t\t\tp->item->object.flags &= ~CHILD_SHOWN;\n+\t\t}\n+\t}\n+\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\twhile ((c = pop_commit(&queuei)))\n+\t\tcommit_list_insert(c, &queueo);\n+\twhile ((c = pop_commit(&queueo)))\n+\t\tcommit_list_insert(c, &reversed_queue);\n+\n+\twhile ((c = pop_commit(&reversed_queue)))\n+\t\tcommit_list_insert(c, queue);\n+}\n+\n struct commit *get_revision(struct rev_info *revs)\n {\n \tstruct commit *c;\n \tstruct commit_list *reversed;\n+\tstruct commit_list *queue = NULL;\n+\tstruct commit_list *p;\n+\n+\tif (revs->max_count_type == 1 && !revs->max_count_stage) {\n+\t\tretrieve_oldest_commits(revs, &queue);\n+\t\tcommit_list_free(revs->commits);\n+\t\trevs->commits = queue;\n+\t\trevs->max_count_stage = 1;\n+\t}\n \n \tif (revs->reverse) {\n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs)))\n-\t\t\tcommit_list_insert(c, &reversed);\n+\t\tif (revs->max_count_type == 1)\n+\t\t\twhile ((c = pop_commit(&revs->commits)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n+\t\telse\n+\t\t\twhile ((c = get_revision_internal(revs)))\n+\t\t\t\tcommit_list_insert(c, &reversed);\n \t\tcommit_list_free(revs->commits);\n \t\trevs->commits = reversed;\n \t\trevs->reverse = 0;\n@@ -4543,7 +4637,18 @@ struct commit *get_revision(struct rev_info *revs)\n \t\treturn c;\n \t}\n \n-\tc = get_revision_internal(revs);\n+\tif (revs->max_count_stage) {\n+\t\tc = pop_commit(&revs->commits);\n+\t\tif (c) {\n+\t\t\tc->object.flags |= SHOWN;\n+\t\t\tif (!(c->object.flags & BOUNDARY))\n+\t\t\t\tfor (p = c->parents; p; p = p->next)\n+\t\t\t\t\tp->item->object.flags |= CHILD_SHOWN;\n+\t\t}\n+\t} else {\n+\t\tc = get_revision_internal(revs);\n+\t}\n+\n \tif (c && revs->graph)\n \t\tgraph_update(revs->graph, c);\n \tif (!c) {\ndiff --git a/revision.h b/revision.h\nindex 584f1338b5..e157463cb1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -309,6 +309,8 @@ struct rev_info {\n \t/* special limits */\n \tint skip_count;\n \tint max_count;\n+\tunsigned int max_count_type:1;\n+\tunsigned int max_count_stage:1;\n \ttimestamp_t max_age;\n \ttimestamp_t max_age_as_filter;\n \ttimestamp_t min_age;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 05cee9e41b..75edb0eb38 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -1882,6 +1882,46 @@ test_expect_success 'log --graph with --name-status' '\n \ttest_cmp_graph --name-status tangle..reach\n '\n \n+test_expect_success 'log --max-count-oldest=3 --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline | tail -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '\n+\ttest_when_finished rm expect &&\n+\tgit log --oneline --reverse | head -n3 >expect &&\n+\tgit log --oneline --max-count-oldest=3 --reverse >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --max-count-oldest with --max-count' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --max-count=3 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest with --skip' '\n+\ttest_when_finished rm stderr &&\n+\ttest_must_fail git log --max-count-oldest=3 --skip=1 2>stderr &&\n+\ttest_grep \"cannot be used together\" stderr\n+'\n+\n+test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '\n+\ttest_when_finished rm expect actual &&\n+\tgit log --graph --boundary >expect &&\n+\tgit log --max-count-oldest=1000 --graph --boundary >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --oneline --graph --boundary --max-count-oldest=1' '\n+\ttest_when_finished rm -f actual &&\n+\tgit log --oneline --graph --boundary --max-count-oldest=1 \\\n+\t\tHEAD~1..HEAD >actual &&\n+\ttest_line_count = 2 actual\n+'\n+\n cat >expect <<-\\EOF\n * reach\n |\n-- \n2.54.0-514-g9d901a57fc\n\n"},{"id":"544546","messageId":"ah8VHKk83Az44A-t@exploit","threadId":"65511","inReplyTo":"xmqq4ijm3p2x.fsf@gitster.g","subject":"Re: [PATCH v9] revision.c: implement --max-count-oldest","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-06-02T17:42:31Z","receivedAt":"2026-06-02T17:50:49Z","isPatch":true,"body":"On Tue, Jun 02, 2026 at 06:53:10AM +0900, Junio C Hamano wrote:\n> It has been a while, and we saw no further comments by other\n> reviewers.\n> \n> Perhaps we should declare a victory and mark the topic for 'next'.\n\nYes, if no one objects it might be time for 'next'.\n"}]}