Submitter | Katsunori FUJIWARA |
---|---|
Date | Jan. 5, 2016, 11:48 a.m. |
Message ID | <de07b46cc5de1baa86d4.1451994515@feefifofum> |
Download | mbox | patch |
Permalink | /patch/12527/ |
State | Changes Requested |
Delegated to: | Yuya Nishihara |
Headers | show |
Comments
On Tue, 05 Jan 2016 20:48:35 +0900, FUJIWARA Katsunori wrote: > # HG changeset patch > # User FUJIWARA Katsunori <foozy@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. > + 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. > + 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 > + 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.
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@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@lares.dti.ne.jp
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.
Patch
diff --git a/mercurial/registrar.py b/mercurial/registrar.py --- a/mercurial/registrar.py +++ b/mercurial/registrar.py @@ -126,3 +126,85 @@ class delayregistrar(object): while self._list: func, decorator = self._list.pop(0) decorator(func) + +class funcregistrarbase(object): + """Base of decorator to register a fuction for specific purpose + + This decorator stores decorated functions into own dict 'table'. + + The least derived class can be defined by overriding 'formatdoc', + for example:: + + class keyword(funcregistrarbase): + formatdoc = ":%s: %s" + + This should be used as below: + + keyword = registrar.keyword() + + @keyword('bar') + def barfunc(*args, **kwargs): + '''Explanation of bar keyword .... + ''' + pass + + In this case: + + - 'barfunc' is stored as 'bar' in 'table' of an instance 'keyword' above + - 'barfunc.__doc__' becomes ":bar: Explanation of bar keyword" + """ + def __init__(self): + self.table = {} + + def __call__(self, decl, *args, **kwargs): + return lambda func: self.doregister(func, decl, *args, **kwargs) + + 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) + + self.table[name] = func + self.extrasetup(name, func, *args, **kwargs) + + return func + + 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 + + def getname(self, decl): + """Return the name of the registered function from decl + + Derived class should override this, if it allows more + descriptive 'decl' string than just a name. + """ + return decl + + def formatdoc(self, decl, doc): + """Return formatted document of the registered function for help + + 'doc' is '__doc__.strip()' of the registered function. + + If this is overridden by non-callable object in derived class, + such value is treated as "format string" and used to format + document by 'self.formatdoc % (decl, doc)' for convenience. + """ + raise NotImplementedError() + + def extrasetup(self, name, func): + """Execute exra setup for registered function, if needed + """ + pass