Re: [PATCH] revision walker: Fix --boundary when limited

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 6, 2007, 01:44 UTC
Message-ID: <Pine.LNX.4.63.0703060242130.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vd53nupg9.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 5 Mar 2007, Junio C Hamano wrote:

Show 18 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> +	if (revs->reverse) {
> >> +		l = NULL;
> >> +		while ((c = get_revision_1(revs)))
> >> +			commit_list_insert(c, &l);
> >> +		revs->commits = l;
> >> +		revs->reverse = 0;
> >>  	}
> >
> > Clever!
> 
> It is not clever, but just is stupid and WRONG.  It just shows
> how little I care about --reverse ;-).
> 
> revision_1() is to get the full list without non limit limiters,
> so the above loop would not even deplete the max_count but
> literally grabs everything.

Oops. I missed the _1(), _and_ the missing "revs->reverse = 0"...

Show 7 quoted lines

> >> +	for (l = c->parents; l; l = l->next) {
> >
> > AFAICT this changes behaviour: c->parents were possibly rewritten.
> 
> Well, the behaviour of max with boundary in 'master' did the same thing, 
> as what was in revs->commits are rewritten parents of commits we already 
> returned, didn't it?

I missed that, too. Maybe I should get more familiar with the revision walker first, before continuing to ask for ridicule.

Ciao, Dscho