[PATCH 4 of 6] parser: move alias declaration parser to common rule-set class

Yuya Nishihara yuya at tcha.org
Fri Apr 1 22:52:52 EDT 2016


On Fri, 1 Apr 2016 14:32:41 -0700, Pierre-Yves David wrote:
> On 04/01/2016 08:37 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 2a3e01c30ddc08a5ff331b8d9b30ac8c014090d6
> > # Parent  ae143c83b4c139422da83eab3c9c3ffb5825a6e8
> > 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 doctest is kept in revset._parsealiasdecl() because it requires concrete
> > syntax rules.
> >
> > diff --git a/mercurial/parser.py b/mercurial/parser.py
> > --- a/mercurial/parser.py
> > +++ b/mercurial/parser.py
> > @@ -253,3 +253,40 @@ class aliasrules(object):
> >           self._getlist = getlist
> >           self._symbolnode = symbolnode
> >           self._funcnode = funcnode
> > +
> > +    def _builddecl(self, decl):  
> 
> Why is this method private? It seems to be the main entry point for the 
> object right now. Is this temporary?

Yes, temporary. All private calls will be removed by the subsequent patches.

> > +        """Parse an alias declaration into ``(name, tree, args, errorstr)``
> > +
> > +        - ``name``: of declared alias (may be ``decl`` itself at error)
> > +        - ``tree``: parse result (or ``None`` at error)
> > +        - ``args``: list of argument names (or None for symbol declaration)
> > +        - ``errorstr``: detail about detected error (or None)
> > +        """
> > +        try:
> > +            tree = self._parsedecl(decl)  
> 
> So all the actual syntax parsing is done in self._parsedecl, but the
> analysis of that tree is done in this method. We should probably mention it.

Ok, will do in next version.

> Should we have a very simple doctest to validate this?

Hmm, I can split the original doctest to parsing and analysis tests. The latter
can be moved to this function.


More information about the Mercurial-devel mailing list