Patchwork revset: don't error out if tokens parse as existing symbols

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 2, 2015, 4:50 a.m.
Message ID <c2616419662f8696d0f1.1430542258@Iris>
Download mbox | patch
Permalink /patch/8846/
State Superseded
Headers show

Comments

Jordi Gutiérrez Hermoso - May 2, 2015, 4:50 a.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1430352121 14400
#      Wed Apr 29 20:02:01 2015 -0400
# Node ID c2616419662f8696d0f14bb418d171667aa3da49
# Parent  e9edd53770fb77a9787a3e6592a3bf0a29c1bd80
revset: don't error out if tokens parse as existing symbols

It makes perfect sense for tokens to parse as existing revset symbols
(revset functions), and doesn't break anything, since parsing symbols
as functions works correctly in the presence of parens. For example,
if "only" is a bookmark, this used to error out,

   hg log -r "only(only, @)"

which shouldn't, as the inner "only" is unambiguously not a function.
Yuya Nishihara - May 2, 2015, 10:51 a.m.
On Sat, 02 May 2015 00:50:58 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1430352121 14400
> #      Wed Apr 29 20:02:01 2015 -0400
> # Node ID c2616419662f8696d0f14bb418d171667aa3da49
> # Parent  e9edd53770fb77a9787a3e6592a3bf0a29c1bd80
> revset: don't error out if tokens parse as existing symbols
> 
> It makes perfect sense for tokens to parse as existing revset symbols
> (revset functions), and doesn't break anything, since parsing symbols
> as functions works correctly in the presence of parens. For example,
> if "only" is a bookmark, this used to error out,
> 
>    hg log -r "only(only, @)"
> 
> which shouldn't, as the inner "only" is unambiguously not a function.

This patch will help to remove old-style parser at some time.

> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -335,8 +335,6 @@ def stringset(repo, subset, x):
>      return baseset()
>  
>  def symbolset(repo, subset, x):
> -    if x in symbols:
> -        raise error.ParseError(_("can't use %s here") % x)
>      return stringset(repo, subset, x)

I think symbolset() can be removed. fileset.py has no such function and
both 'string' and 'symbol' are mapped to stringset().

> @@ -291,6 +291,11 @@ quoting needed
>    $ log '"date"'
>    abort: unknown revision 'date'!
>    [255]
> +  $ hg book date -r 4
> +  $ log 'date'
> +  4

It doesn't test the revset function because old-style revision can be caught
by scmutil.revrange().

Regards,
Jordi Gutiérrez Hermoso - May 2, 2015, 5:59 p.m.
On Sat, 2015-05-02 at 19:51 +0900, Yuya Nishihara wrote:
> On Sat, 02 May 2015 00:50:58 -0400, Jordi Gutiérrez Hermoso wrote:
> > @@ -291,6 +291,11 @@ quoting needed
> >    $ log '"date"'
> >    abort: unknown revision 'date'!
> >    [255]
> > +  $ hg book date -r 4
> > +  $ log 'date'
> > +  4
> 
> It doesn't test the revset function because old-style revision can be caught
> by scmutil.revrange().

I beg your pardon?

Here the revset is "date", not "4", unless I am much mistaken. This
test certainly failed before this patch, so I am almost sure it's
going through the symbolset function.

As to your other concern whether symbolset can be removed, maybe, but
it's still being called by the test above
Yuya Nishihara - May 3, 2015, 4:35 a.m.
On Sat, 02 May 2015 13:59:04 -0400, Jordi Gutiérrez Hermoso wrote:
> On Sat, 2015-05-02 at 19:51 +0900, Yuya Nishihara wrote:
> > On Sat, 02 May 2015 00:50:58 -0400, Jordi Gutiérrez Hermoso wrote:
> > > @@ -291,6 +291,11 @@ quoting needed
> > >    $ log '"date"'
> > >    abort: unknown revision 'date'!
> > >    [255]
> > > +  $ hg book date -r 4
> > > +  $ log 'date'
> > > +  4
> > 
> > It doesn't test the revset function because old-style revision can be caught
> > by scmutil.revrange().
> 
> I beg your pardon?
> 
> Here the revset is "date", not "4", unless I am much mistaken. This
> test certainly failed before this patch, so I am almost sure it's
> going through the symbolset function.

See
http://selenic.com/repo/hg/file/e9edd53770fb/mercurial/scmutil.py#l757

% hg log -r date
hg: parse error: can't use date here
% hg book date
% hg log -r date
changeset:   -1:000000000000
bookmark:    date
tag:         tip
user:        
date:        Thu Jan 01 00:00:00 1970 +0000
% hg debugrevspec date
hg: parse error: can't use date here
Jordi Gutiérrez Hermoso - May 3, 2015, 4:23 p.m.
On Sun, 2015-05-03 at 13:35 +0900, Yuya Nishihara wrote:
> See
> http://selenic.com/repo/hg/file/e9edd53770fb/mercurial/scmutil.py#l757
> 
> % hg log -r date
> hg: parse error: can't use date here
> % hg book date
> % hg log -r date
> changeset:   -1:000000000000
> bookmark:    date
> tag:         tip
> user:        
> date:        Thu Jan 01 00:00:00 1970 +0000
> % hg debugrevspec date
> hg: parse error: can't use date here

Thank you, I see my error now. I have prepared a revised change.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -335,8 +335,6 @@  def stringset(repo, subset, x):
     return baseset()
 
 def symbolset(repo, subset, x):
-    if x in symbols:
-        raise error.ParseError(_("can't use %s here") % x)
     return stringset(repo, subset, x)
 
 def rangeset(repo, subset, x, y):
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -280,7 +280,7 @@  quoting needed
   hg: parse error: date requires a string
   [255]
   $ log 'date'
-  hg: parse error: can't use date here
+  abort: unknown revision 'date'!
   [255]
   $ log 'date('
   hg: parse error at 5: not a prefix: end
@@ -291,6 +291,11 @@  quoting needed
   $ log '"date"'
   abort: unknown revision 'date'!
   [255]
+  $ hg book date -r 4
+  $ log 'date'
+  4
+  $ log '"date"'
+  4
   $ log 'date(2005) and 1::'
   4