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

[FUJIWARA Katsunori]

At Thu, 7 Jan 2016 23:46:02 +0900,
Yuya Nishihara wrote:
> 
> On Tue, 05 Jan 2016 20:48:35 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1451994203 -32400
> > #      Tue Jan 05 20:43:23 2016 +0900
> > # Node ID de07b46cc5de1baa86d40492941fd56c849311c3
> > # Parent  e526fa5df73b5263d164bd339251481050755720
> > registrar: introduce funcregistrarbase to replace funcregistrar
> 
> > 'extrasetup()' is invoked with *args and **kwargs arguments in
> > 'doregister()', but it is defined without them, because this can
> > reject unknown optional argument at decoration easily, if derived
> > class doesn't override 'extrasetup()' with any optional arguments.
> 
> Neat.
> 
> > +class funcregistrarbase(object):
> 
> Let's mark this class and its internal callbacks as private to make sure
> that the derived classes should be defined in the registrar.

I'll do so in revised series.

> > +    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)

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


> > +    def doregister(self, func, decl, *args, **kwargs):
> > +        name = self.getname(decl)
> > +
> > +        if func.__doc__ and not util.safehasattr(func, '_origdoc'):
> > +            doc = func.__doc__.strip()
> > +            func._origdoc = doc
> > +            if callable(self.formatdoc):
> > +                func.__doc__ = self.formatdoc(decl, doc)
> > +            else:
> > +                # convenient shortcut for simple format
> > +                func.__doc__ = self.formatdoc % (decl, doc)
> 
> I'm a little shocked that formatdoc() is overloaded as a format string.
> I would rather do
> 
>   def _formatdoc(self, decl, doc):
>       return self._doctemplate % (decl, doc)  # override me if needed

I'll do so in revised series.

> > +    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).

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