[PATCH 3 of 5 V2] ancestor: add a class to generate ancestors lazily

[Siddharth Agarwal]

On 12/17/2012 10:31 AM, Pierre-Yves David wrote:
> Looks promising.
>
> But I'm not very happy to see that as "yes another different function".
>
> Isn't it possible to alter the original ancestors function for the same purpose?

I discussed this with Pierre and we came to a solution for this.

> Can't we apply this kind of of speedup to the revset version too an use
> it every where?

That will be much more work than this.

> You are starting from <revs> not tip isn't it ? 

Yes, fixed.

> increvs a bit misleading. I read it a "incremental revs". I would name 
> it "inclusive". I would also use a default to True to simplify signature. 

Fixed (with default False though because that's how revlog.ancestors was).

> <stoprev> is never used in the code actual user code. Is that not 
> possible.

This is now moot.

> A changelog argument would be better here in my opinion. You should 
> highlight why you are using a heapq instead of a deque here. 

Fixed and fixed.

> could be -visit[0] > target every part of visit already have been 
> checked agains target.

Fixed.

> (switching the two conditional sound like a possible micro 
> optimistation.) 

Fixed.

> add a comment stating that pushing parent is mandatory if we want 
> futur call to containts to be correct

Done.

> I would use a break instead of a boolean check
>
> while visit and -visit[0] > target:
>      for parent in parentrevs(-heappop(visit))
>          if parent < stoprev or parent in seen:
>              continue
>          heappush(visit, -parent)
>          seen.add(parent)
>          if parent == target:
>              break
> else:
>      return False
> return True
>
> (yop, python, while else…)
>

This code is wrong because it can drop parents on the floor while 
exiting. It is possible to fix this with two for loops over the parents 
instead of one, but at that point, why not just use a flag?