[PATCH 2 of 6] graphlog: turn getlogrevs() into a generator

[Matt Mackall]

On Sat, 2012-06-16 at 10:59 +0200, Patrick Mézard wrote:
> Le 23/05/12 21:44, Matt Mackall a écrit :
> > On Wed, 2012-05-23 at 09:44 +0200, Patrick Mézard wrote:
> >> Le 22/05/12 21:37, Matt Mackall a écrit :
> >>> On Tue, 2012-05-08 at 22:58 +0200, Patrick Mezard wrote:
> >>>> # HG changeset patch
> >>>> # User Patrick Mezard <patrick at mezard.eu>
> >>>> # Date 1336509824 -7200
> >>>> # Node ID 53db775e252c0f4f7add32b62e08096814d3ab7e
> >>>> # Parent  d30440da7b38c671266ae1734db3921475a63caa
> >>>> graphlog: turn getlogrevs() into a generator
> >>>
> >>> Queued these first two for default, thanks. Still mulling the rest, I'm
> >>> afraid.
> >>
> >> By the way, I am not really attached to the precompute thing. This is
> >> just the simplest/less invasive trick I found to "solve" the issue at
> >> hand. The fundamental issues are:
> >> - There is no execution context provided to revset functions, which makes caching and rewriting harder to implement.
> >> - Being made of tuples, it is pretty hard to annotate the AST.
> > 
> > Yeah, I think we'll need to do something here. But I think I'd rather
> > start down the path of implementing an incremental revset framework and
> > see where that gets us. It will almost certainly need a caching scheme
> > but it probably won't look much like this one.
> 
> Revisiting this topic I noticed you actually pushed this patch:
> 
>   graphlog: turn getlogrevs() into a generator
>   http://selenic.com/hg/rev/058e14da7044
> 
> Does it mean you agree with turning revset.match() output into a
> stateful filtering function which could be called several times rather
> than into a generator?

Maybe? There's probably a bit too much happening in this question.

> I said that:
> """
> I feel that calling the matcher multiple times is correct, at least with
> the revsets involved in getlogrevs() because they are:

Ok, I'm intentionally trying _not_ to view this issue from the rather
skewed perspective of getlogrevs(). I'm trying to view it from the
perspective of "what should a query interface look like?".

In an ideal world, yes, I can ask the question "is revision X in revset
R" by doing something like:

m = revset.match(repo.ui, R)
if m(repo, [X]):
	return True

That works today, actually.

If we want to ask the same question later about Y, that's slow, because
it goes and does all the same work again. But yes, I think it's
reasonable to have m internally cache results and/or lazily generate
results if possible.

Unfortunately, the API for that is rather broken: we have repo in the
match call and not in the constructor.

-- 
Mathematics is the supreme nostalgia of our time.