[PATCH 3 of 7 V3 RFC] parser: move alias declaration parser to common rule-set class

[Yuya Nishihara]

On Sun, 10 Apr 2016 23:16:32 -0700, Pierre-Yves David wrote:
> On 04/04/2016 05:27 AM, Yuya Nishihara wrote:
> > On Sun, 3 Apr 2016 14:04:20 -0700, Pierre-Yves David wrote:  
> >> On 04/03/2016 02:48 AM, Yuya Nishihara wrote:  
> >>> # HG changeset patch
> >>> # User Yuya Nishihara <yuya at tcha.org>
> >>> # Date 1456736043 -32400
> >>> #      Mon Feb 29 17:54:03 2016 +0900
> >>> # Node ID aa54435b4a485541d3be7abbe5ed6fd584b43ee5
> >>> # Parent  37cbd68f6cd8760b146570dd8b547589539605fd
> >>> parser: move alias declaration parser to common rule-set class
> >>>
> >>> The original _parsealiasdecl() function is split into common _builddecl()
> >>> and revset-specific _parsealiasdecl(). And the original _parsealiasdecl()
> >>> call is temporarily replaced by rules._builddecl(), which should be eliminated
> >>> later.
> >>>
> >>> The doctests are mostly ported by using the dummy parse(), but the test for
> >>> 'foo bar' is kept in _parsealiasdecl() as it checks if "pos != len(decl)" is
> >>> working. Also, 'foo($1)' test is added to make sure the alias tokenizer can
> >>> handle '$1' symbol, which is the only reason why we need _parsealiasdecl().
> >>>     
> >> […]
> >>  
> >>> @@ -2373,6 +2316,7 @@ def _parsealiasdefn(defn, args):
> >>>    class _aliasrules(parser.basealiasrules):
> >>>        """Parsing and expansion rule set of revset aliases"""
> >>>        _section = _('revset alias')
> >>> +    _parsedecl = staticmethod(_parsealiasdecl)
> >>>        _getlist = staticmethod(getlist)
> >>>
> >>>    class revsetalias(object):  
> >> Why are we keeping the function outside of the class? (With a manual
> >> staticmethod promition and assignement). Shouln't we be able to directly
> >> declare it inside the class with a @static method decorator?  
> > Two reasons:
> >
> >   a) It's paired with _tokenizealias(). I could move both _parsealiasdecl()
> >      and _tokenizealias() to _aliasrules, but it would introduce unnecessary
> >      name "_tokenize" under _aliasrules, which I didn't want to.  
> 
> I do not see why this matters here. Having `parsealiasdecl` directly 
> declared as a static method will not prevent the use of that external 
> function `tokenizealias `. Does it? Did I missed something else?

No. I mean there isn't much motivation to move parsealiasdecl to
_aliasrules._parsedecl. It could, but that should be totally fine to keep
it as a free function.

> >   b) _parsealiasdecl() is only a parser that takes '$' as a symbol char, which
> >      can theoretically be used as one of parsing functions.  
> 
> I did not understand what you are trying to tell me here. Can you rephrase?

_parsealiasdecl() isn't strongly coupled with _aliasrules. It's more like
free parsing tools such as parse() or getlist().

Anyway, this is trivial matter, I don't think it's worth arguing about this.
If you prefer defining static function inside class, I'm okay to do that by
follow-up patch.