Re: [PATCH 0/4] Re: cherry-pick and 'log --no-walk' and ordering

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 13, 2012, 17:05 UTC
Message-ID: <7vzk5yen99.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAOeW2eHprw73+zqVbJRird1eE7ayU_KjCUSoieYsGi1rbL5QBQ@mail.gmail.com>

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

Show 13 quoted lines

> I also thought the sorting was just a bug. From what I understand by
> looking how the code has evolved, the sorting in the no-walk case was
> not intentional, but more of a consequence of the implementation. That
> patch you suggested was my first attempt and led me to find the broken
> cherry-pick test cases that I then fixed in patch 2/4. But, it clearly
> would break the test case in t4202 called 'git log --no-walk <commits>
> sorts by commit time'. So I started digging from there and found e.g.
>
> http://thread.gmane.org/gmane.comp.version-control.git/123205/focus=123216
>
> For convenience, I have pasted the commit message of the commit
> mentioned in that thread at the end of this email.  So we would be
> breaking at least Johannes's use case if we changed it.

Ok. Having a way to conveniently sort based on committer date is indeed handy, and losing it would be a regression.

Not that the accident that supports only on committer date is a nicely designed feature. The user may want to sort on author date instead, but there is no way to do so with --no-walk. So in that sense, Johannes's use case happens to work by accident.

> ... so is a migration desired? Or just
> change the default for --no-walk from "sorted" to "unsorted" in git
> 2.0?

I think the proper support for Johannes's case should give users more control on what to sort on, and that switch should not be tied to "--no-walk". After all, being able to sort commits in the result of limit_list() with various criteria would equally useful as being able to sort commits listed on the command line with --no-walk. Think about what "git shortlog A..B" does, for example. It is like first enumerating commits within the given range, and sorting the result using author as the primary and then timestamp as the secondary sort column.

So let's not even think about migration, and go in the direction of giving "--no-walk" two flavours, for now. Either it keeps the order commits were given from the command line, or it does the default sort using the timestamp. We can later add the --sort-on option that would work with or without --no-walk for people who want output that is differently sorted, but that is outside the scope of your series.

> By the way, git-log's documentation says "By default, the commits are
> shown in reverse chronological order.", which to some degree is in
> support of the current behavior.

That is talking about the presentation order of the result of limit_list(), predates --no-walk, and was not adjusted to the new world order when --no-walk was introduced, so I would not take it as a supporting argument.

But not regressing the current "you can see them sorted on the commit timestamp (this is merely an accident and not a designed feature, so you cannot choose to sort on other things)" behaviour is a reason enough not to disable sorting for the plain "--no-walk" option.

Thanks.