Re: [PATCH 3 of 3 V5] revset: skip legacy lookup for revspec wrapped in 'revset(…)'

[Feld Boris]

On 16/04/2018 14:24, Yuya Nishihara wrote:
> On Mon, 16 Apr 2018 08:58:31 +0200, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.feld at octobus.net>
>> # Date 1523369212 -7200
>> #      Tue Apr 10 16:06:52 2018 +0200
>> # Node ID 109ca88347d7b531d4b48370efcbc4d6e850cf92
>> # Parent  f363552ced37ae028bbcf2cba1f02ac623385f54
>> # EXP-Topic noname
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 109ca88347d7
>> revset: skip legacy lookup for revspec wrapped in 'revset(…)'
>> @@ -2191,7 +2194,12 @@ def matchany(ui, specs, lookup=None, loc
>>           raise error.ParseError(_("empty query"))
>>       parsedspecs = []
>>       for s in specs:
>> -        parsedspecs.append(revsetlang.parse(s, lookup))
>> +        lookupthis = lookup
>> +        stripped = s.strip()
>> +        if (stripped.startswith(prefixrevset)
>> +                and stripped.endswith(postfixrevset)):
>> +            lookupthis = None
>> +        parsedspecs.append(revsetlang.parse(s, lookupthis))
> Is it okay to move this hack to revsetlang._parsewith?
>
> @@ -482,6 +485,8 @@ def _parsewith(spec, lookup=None, symini
>         ...
>       ParseError: ('invalid token', 4)
>       """
> +    if spec.startswith('revset(') and spec.endswith(')'):
> +        lookup = None
>       p = parser.parser(elements)
>       tree, pos = p.parse(tokenize(spec, lookup=lookup,
>                                    syminitletters=syminitletters))
>
> I don't think revset.match*() is the right place to do parsing stuff, and
> we'll need a tokenizer to make it more correctly handle variants such as
> ' revset ( ... )' or '... and revset(...)'.
You're are right, moving it lower in the stack makes sense. Would it be 
possible to implement it even lower in revsetlang.tokenize?

We tried preparing a V6 to move the code but we didn't find the queued 
version of the first changeset. Were you waiting for us to move the code 
yourself?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel