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

[Feld Boris]

On 16/04/2018 19:49, Feld Boris wrote:
> 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?
We sent the V6 version with the renamed test file (we hope it's the 
right name) so you can take the second updated changeset.
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel