[PATCH 2 of 5] registrar: introduce funcregistrarbase to replace funcregistrar

Yuya Nishihara yuya at tcha.org
Sat Jan 9 05:25:29 CST 2016


On Sat, 09 Jan 2016 19:46:34 +0900, FUJIWARA Katsunori wrote:
> At Thu, 7 Jan 2016 23:46:02 +0900,
> Yuya Nishihara wrote:
> > > +    def __init__(self):
> > > +        self.table = {}
> > 
> > Perhaps core modules would want to pass a table.
> > 
> >   # revset.py
> >   predicate = registrar.revsetpredicate(symbols)
> > 
> > If there's no revset.safesymbols, we can do it.
> 
> You would assume that passing table directly to decorator can avoid
> explicit invocation of extra loading function at the end of own
> module, wouldn't you ?  (e.g. templatekw, templatefilters or so)

Yes, it looks ugly to me to call extra setup function by the hosting module
itself.

> It is reasonable, but on the other hand, similarity between modules
> using this extra loading mechanism seems important, too, IMHO.

Eventually we can remove revset.safesymbols and we'll no longer need to
call loadpredicate(None, None, predicate).

> > > +    def parsefuncdecl(self, decl):
> > > +        """Parse function declaration and return the name of function in it
> > > +        """
> > > +        i = decl.find('(')
> > > +        if i > 0:
> > > +            return decl[:i]
> > > +        else:
> > > +            return decl
> > 
> > Is it intentional that i == 0 isn't counted as a function declaration?
> > "()" is invalid name anyway, but I'm just curious why you've chosen i > 0.
> 
> Treating "i == 0" as regular name puts "empty name" into table, and it
> is never fetched via current tokenizers (isn't it ? :-))
> 
> Therefore, this patch treats such declaration as "name for special
> internal purpose" (e.g. direct expanding tree for revset alias).

Ok, though I don't understand the possible use of '(..' name, I'm fine with
it.


More information about the Mercurial-devel mailing list