[PATCH 4 of 5 V2] revset: use delayregistrar to register predicate in extension easily

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Sun Jan 3 04:56:33 CST 2016


At Sat, 2 Jan 2016 17:56:59 +0900,
Yuya Nishihara wrote:
> 
> On Sat, 02 Jan 2016 01:28:28 +0900, FUJIWARA Katsunori wrote:
> > At Fri, 1 Jan 2016 17:53:10 +0900,
> > Yuya Nishihara wrote:
> > > "cmdtable" was wrong?
> > > 
> > > I like consistent extension APIs, so my idea was something like this:
> > > 
> > > 1. define table of revset symbols per extension
> > > 
> > >   # creates new table and associated decorator
> > >   revsetpredicate = registrar.revsetpredicate()
> > 
> > In this case, loading logic looks inside 'revsetpredicate' object to
> > get predicate functions to be loaded, doesn't it ? (= table-ness of
> > bridging object isn't important)
> 
> Right. The difference is who controls the loading logic.
> 
>   (a) @command          mercurial.dispatch loads cmdtable
>   (b) @revsetpredicate  hgext.xxx updates revset.symbols
> 
> We haven't (a) for revsets and templates, and your patch follows it,
> which is straightforward. I just wondered if (a) is preferable as an extension
> API.

Thank you for explanation.

> > >   revsetpredicate = registrar.revsetpredicate(revsetsymbols)
> > 
> > (BTW, would you also mean that revset decorator specific code should
> > be placed in registrar.py by sample code above ? I think that it
> > shouldn't so, IMHO)
> 
> Yes. If the registrar provides all decorators, an extension can avoid
> importing templatekw or templatefilters just because it defines new template
> keywords/filters. (though the revset module would be necessary to define new
> workable revset predicates.)

Yeah, I though that placing decorator in each corresponded module is
reasonable, because defining new workable function requires importing
corresponded module in many cases, to use utilities in it: revset,
fileset, some template keyword listing elements or so, for example.

But it is obvious that module importing should be avoided if possible,
as you described.


> > > 2. load them by dispatch._dispatch()
> > 
> > What about registration steps below, if explicit setup() on
> > revsetpredicate isn't suitable for consistent API ? This can prevents
> > dispatch (and registrar) from depending on revset, template XXXX, and
> > so on.
> >
> >   dispatch._dispatch()
> >   - extensions.loadall()
> >     - at loading extension
> >       - instantiate revset.predicate or so as 'revsetpredicate' (*1)
> >         - at loading revset.py
> >           - register revset specific hook (*2) to registrar.py
> >       - revset.predicate()
> >         - decorator stores information internally
> >     - uisetup of extension
> >     - extsetup of extension
> >   - invoke hooks registered in registrar.py on each extensions
> >     - invoke hook (*2)
> >       - load predicates from 'revsetpredicate' attribute (*1) of extension
> 
> registrar shouldn't import revset, but I think it's fine for dispatch to
> import revset. And import-time hook registration won't be necessary.

OK, I'll send another series to revise this series in such manner.

> Regards,
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list