On Sun, 13 Jan 2019 08:40:41 +0100, Boris FELD wrote:
> On 12/01/2019 06:00, Yuya Nishihara wrote:
> > On Fri, 11 Jan 2019 12:29:10 +0100, Boris Feld wrote:
> >> # HG changeset patch
> >> # User Boris Feld <boris.f...@octobus.net>
> >> # Date 1546605681 -3600
> >> # Fri Jan 04 13:41:21 2019 +0100
> >> # Node ID 73926c4ab24d6c01723ed050601b134bdc89562f
> >> # Parent 4a56fbdacff33c3985bbb84f2e19ddfbd48ed4fa
> >> # EXP-Topic revs-efficiency
> >> # Available At https://bitbucket.org/octobus/mercurial-devel/
> >> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> >> 73926c4ab24d
> >> revset: introduce an API that avoids `formatspec` input serialization
> > The idea looks good.
> >
> >> +class _inputrules(parser.basealiasrules):
> >> + """replace internal input reference by their actual value"""
> >> +
> >> + @classmethod
> >> + def _getalias(cls, inputs, tree):
> >> + if not isinstance(tree, tuple):
> >> + return None
> >> + if tree[0] != 'func':
> >> + return None
> >> + if getsymbol(tree[1]) != _internal_input:
> >> + return None
> >> + idx = int(getsymbol(tree[2]))
> >> + newtree = ('func',
> >> + ('symbol', internal_input_func),
> >> + ('smartset', inputs[idx])
> >> + )
> >> + return parser.alias(idx, None, None, newtree), None
> > Perhaps, this can be just ('smartset', ...) node like 'symbol'/'string'.
> > There's not point to convert it to a 'func' unless it can be expressed
> > as a revset string.
> >
> > And if we can probably ditch the specialized _inputrules. See below.
>
> We need to combine the smartset with the existing subset. Doing it
> within a function seems simpler and less impactful.
Well, there's no practical difference between ('func', ..., ('smartset',))
and ('smartset',).
For ('func', ..., ('smartset',)), we'll need two functions:
symbols[internal_input_func] and methods['smartset'].
symbols[internal_input_func] needs to check the argument type, and
methods['smartset'] would raise ParseError.
For ('smartset',), methods['smartset'] knows 'x' is a smartset, so we can
just return 'x & subset' / 'subset & x'. See stringset() for example.
> >> +def formatspecargs(expr, *args):
> >> + """same as formatspec, but preserve some expensive arguments"""
> >> + parsed = _parseargs(expr, args)
> >> + ret = []
> >> + inputs = []
> >> + for t, arg in parsed:
> >> + if t is None:
> >> + ret.append(arg)
> >> + if t == 'baseset':
> >> + key = '%s(%d)' % (_internal_input, len(inputs))
> >> + inputs.append(smartset.baseset(arg))
> >> + ret.append(key)
> > Maybe we can assign integer (e.g. '$0') for each parameter here, and we can
> > just use the _aliasrules.expand() to replace it. '$x' is invalid in user
> > expression, so it can be used as a reserved symbol.
>
> Don't we use '$0' for user alias?
Yes, but that doesn't matter since 'expr' here isn't an alias expression.
> Also, I'm not sure what's the value of using alias replacement here. Can
> you detail your reasoning?
To get rid of the custom _inputrule class.
> The alias logic seems pretty tied to having string (that it parses) as
> replacement, that is why I used a dedicated object.
So here we have string to parse, b''.join(ret), and replacements, inputs.
If b''.join(ret) contained '$0' instead of 'internal(0)', and if inputs
were dict of {'$0': alias('$0', ..., ('smartset', inputs[0]))}, we can just
feed them to _aliasrules.
aliases = build_dict_of_inputs(inputs)
tree = _parsewith(b''.join(ret), syminitletters=_aliassyminitletters)
tree = _aliasrules.expand(aliases, tree)
That's the idea.
> > If this function returned a parsed tree, we can hide all the implementation
> > details in it, and we can probably remove the inputs argument from
> > revset.matchany().
>
> But revset/matchany does not take a parsed tree, does it?
A parsed tree can be directly passed in to revset.makematcher(). Since
repo.revs() is an internal API, there aren't much we have to copy from
matchany():
tree = foldconcat(tree)
tree = analyze(tree)
tree = optimize(tree)
return tree
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
