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

2018-04-17 Thread Yuya Nishihara 
On Mon, 16 Apr 2018 19:49:43 +0200, 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 
> >> # 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?

Maybe no. We'll need the tokenize() function to handle e.g. 'revset ( ... )'.

  if firsttwotokens(tokenize(s)) == ['revset', '(']:
  return tokenize(s)
  # otherwise, wrap lookup() to count parens and conditionally disable lookup?

> 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?

I've already fixed it locally, so will push soon.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

2018-04-17 Thread 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 
# 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@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


___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

2018-04-16 Thread 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 
# 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@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

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

2018-04-16 Thread Yuya Nishihara 
On Mon, 16 Apr 2018 08:58:31 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld 
> # 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(...)'.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel