Re: [PATCH] Lose perl dependency. (fwd)

From

Junio C Hamano <junkio@cox.net>

Date

Jan 20, 2007, 18:31 UTC

Message-ID

<7vwt3h7dp6.fsf@assigned-by-dhcp.cox.net>

In-Reply-To

<Pine.LNX.4.63.0701201025070.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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?

	$ 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.

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)