On 13/01/2019 10:53, Yuya Nishihara wrote:
> 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.
Just learned about `methods`, nice. This is much simpler.
>
>>>> +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.
Okay, going that route.
>
>>> 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
Great, I initially pursued a tree-based approached, but could not figure
out a good route. Thanks for the showing us the way. The result is both
cleaner and simpler \o/
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
