Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> I think there are two very valid ways. You determine what you
>> would spit out as if there is no --reverse, and then reverse the
>> result, or you do not limit with them to get everthing, reverse
>> the result and do the counting limit on that reversed list.
> ...
>> I do not think you would need to artificially make it limited like your
>> patch does if you go this route
>
> Why? To see the last commit (which should be output first), I _have_ to
> traverse them first, before reversing the order. I thought revs->limited
> does exactly that -- traverse all commits first. Am I mistaken?
I think you are talking about the second semantics; I was
talking about the first one. In other words, the one whose
semantics of:
$ git log --max-count=10 --skip=5 --reverse HEAD
is to first internally run
$ git log --max-count=10 --skip=5 HEAD
then reverse the resulting 10 commits and spit them out.
Now, "git log --max-count=10 --skip=5" does not need to call
limit_list(). It needs to traverse the usual date-sorted
revs->commits for fifteen rounds.
Looking at your patch again,...
@@ -1155,6 +1160,8 @@ void prepare_revision_walk(struct rev_info *revs)
sort_in_topological_order_fn(&revs->commits, revs->lifo,
revs->topo_setter,
revs->topo_getter);
+ if (revs->reverse)
+ revs->commits = reverse_commit_list(revs->commits);
}
static int rewrite_one(struct rev_info *revs, struct commit **pp)
This makes the code traverse and grab everything and then
reverse; the later get_revision() -> get_revision_1() loop skips
5, returns 10 and then finally stops. In other words, this
gives 10 old commits counting from the 6th oldest one in the
history.
If we prefer the first semantics, we do not have to traverse and
grab everything. That is what I was getting at.
That is, something like this, with your option parsing change
(modulo we _might_ want to explicitly mark some of the users
incompatible), addition of reverse field to struct rev_info,
moving reverse_commit_list() to a more public place, but without
making the reverse to imply limited traversal.
diff --git a/revision.c b/revision.c
index f2ddd95..161c4c0 100644
--- a/revision.c
+++ b/revision.c
@@ -1274,6 +1274,14 @@ struct commit *get_revision(struct rev_info *revs)
{
struct commit *c = NULL;
+ if (revs->reverse) {
+ /* we were asked to reverse, but haven't reversed the
+ * result, yet, so do it here once
+ */
+ revs->commits = reverse_commit_list(revs->commits);
+ revs->reverse = 0;
+ }
+
if (0 < revs->skip_count) {
while ((c = get_revision_1(revs)) != NULL) {
if (revs->skip_count-- <= 0)
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html