Re: [PATCH] revision.c: implement --reverse=before for walks

From: Mirko Faina <mroik@delayed.space>
Date: Apr 18, 2026, 18:42 UTC
Message-ID: <aePNILp-yB_8gfiY@exploit>
In-Reply-To: <fbea5f1c-946b-400e-a9a2-2c6d7b088d46@malon.dev>

On Sun, Apr 19, 2026 at 02:20:41AM +0800, Tian Yuchen wrote:

> Hi Mirco,

Unfortunate miss spell T_T

Show 28 quoted lines

> On 4/19/26 00:47, Mirko Faina wrote:
> > In a revision walk `--reverse` can only be applied after any commit
> > limiting option. This makes getting a limited amount of commits from the
> > tail impossible. E.g.
> > 
> >      git log --reverse --max-count=3
> > 
> > Some would expect this to give back the first 3 commits of the project.
> > Instead it returns the last 3 but in reversed order.
> > 
> > Teach `get_revision()` to accpet an argument `(after|before)` from the
> > CLI, and apply the reversal before or after the commit limiting options
> > based on this argument. If no argument is provided default to the
> > current behaviour, applying `--reverse` after the commit limiting
> > options.
> 
> Reasoning looks good to me.
> 
> Nit: I think we could gather more feedback on the naming. If I were a user
> unfamiliar with how Git works, the most natural and intuitive operation for
> me would be: "Show me the three *oldest* commits" (to be more precise,
> *oldest* in the sense of topological order, rather than the commit date or
> author date order. It’s really annoying. ), rather than "reverse the entire
> list and then select the three most recent ones". I think the confusion
> arises because users do not (and should not) know that '--max-count' only
> returns the most recent commits; consequently, they might wonder: "Well, the
> 'after' and 'before' parameters do make a difference, but why?". I believe
> it is best not to lead users to this point.

The rest of the flags do apply during traversal, and since git goes through the whole history the relative order should be preserved, so depending on the flags that are passed the user can order topologically or chronologically (tho some flags are forbidden together).

I'm up to change the naming tho (and if someone has better phrasing for the docs please tell).

Show 20 quoted lines

> > @@ -4525,19 +4543,35 @@ struct commit *get_revision(struct rev_info *revs)
> >   {
> >   	struct commit *c;
> >   	struct commit_list *reversed;
> > +	int max_count = revs->max_count;
> > +
> > +	if (revs->reverse && !revs->reverse_output_stage) {
> > +		if (revs->reverse == 3) {
> > +			BUG("allowed values for reverse are 0, 1 and 2");
> > +			revs->reverse = 1;
> > +		}
> > +
> > +		if (revs->reverse == 2)
> > +			revs->max_count = -1;
> 
> I think the space complexity here could be reduced a little. After all,
> since we’re only retrieving a few commits, there’s no need to load the
> entire reversed commit history into memory.
> 
> Perhaps we could maintain a window (or perhaps max heap) of finite length?

Unfortunately since the underlying data structure is a linked list we have to traverse the whole tree to get the first one from the tail. The way get_revision() loads the next commit is through process_parents(). Even if we were able to start from the tail we wouldn't have any reference to the children.

I suspect reducing space complexity would require to change a lot of inner workings of git to make the history traversable both ways.

Thank you