Patchwork [1,of,4] revset: introduce an internal `_rev` predicate for '%d' usage

login
register
mail settings
Submitter Boris Feld
Date Jan. 18, 2019, 3:53 p.m.
Message ID <52d20e4fe2e3049eec58.1547826796@localhost.localdomain>
Download mbox | patch
Permalink /patch/37875/
State Accepted
Headers show

Comments

Boris Feld - Jan. 18, 2019, 3:53 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1547817707 -3600
#      Fri Jan 18 14:21:47 2019 +0100
# Node ID 52d20e4fe2e3049eec58140fdcadb43528729b3d
# Parent  4fab8a7d2d72b4d50d1d647a015d33f64c1e2e4d
# EXP-Topic intlist
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 52d20e4fe2e3
revset: introduce an internal `_rev` predicate for '%d' usage

In 24a1f67bb75a, we aligned "%d" behavior on "%ld" one, invalid revisions got
silently ignored. However, soon after in 8aca89a694d4 and 26b0a7514f01, a side
effect changed the behavior of "%ld" to no longer silently filter invalid
revisions.

After discussion on the mailing list, it was decided to align on the new %ld
behavior:

https://www.mercurial-scm.org/pipermail/mercurial-devel/2019-January/127291.html


This changeset introduce a '_rev()' predicated that keep the benefit from
24a1f67bb75a while enforcing a more strict checking on the inputs.
Yuya Nishihara - Jan. 19, 2019, 11:28 a.m.
On Fri, 18 Jan 2019 16:53:16 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1547817707 -3600
> #      Fri Jan 18 14:21:47 2019 +0100
> # Node ID 52d20e4fe2e3049eec58140fdcadb43528729b3d
> # Parent  4fab8a7d2d72b4d50d1d647a015d33f64c1e2e4d
> # EXP-Topic intlist
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 52d20e4fe2e3
> revset: introduce an internal `_rev` predicate for '%d' usage

Queued this for stable, thanks.

IIUC, the behavior of "%ld" was unchanged though it's inconsistent. So I think
the other 3 patches can be deferred for 5.0 cycle.

> +@predicate('_rev(number)', safe=True)
> +def _rev(repo, subset, x):
> +    # internal version of "rev(x)" that raise error if "x" is invalid
> +    # i18n: "rev" is a keyword
> +    l = getargs(x, 1, 1, _("_rev requires one argument"))

Unified "_rev" in error messages to "rev" as we'll probably want to rewrite
rev() by using _rev().

> +    try:
> +        # i18n: "rev" is a keyword
> +        l = int(getstring(l[0], _("rev requires a number")))
> +    except (TypeError, ValueError):
> +        # i18n: "rev" is a keyword
> +        raise error.ParseError(_("rev expects a number"))
> +    repo.changelog.node(l) # check that the rev exists

We'll need to catch WdirUnsupported here if we change _intlist in that way.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1765,6 +1765,20 @@  def rev(repo, subset, x):
         return baseset()
     return subset & baseset([l])
 
+@predicate('_rev(number)', safe=True)
+def _rev(repo, subset, x):
+    # internal version of "rev(x)" that raise error if "x" is invalid
+    # i18n: "rev" is a keyword
+    l = getargs(x, 1, 1, _("_rev requires one argument"))
+    try:
+        # i18n: "rev" is a keyword
+        l = int(getstring(l[0], _("rev requires a number")))
+    except (TypeError, ValueError):
+        # i18n: "rev" is a keyword
+        raise error.ParseError(_("rev expects a number"))
+    repo.changelog.node(l) # check that the rev exists
+    return subset & baseset([l])
+
 @predicate('revset(set)', safe=True, takeorder=True)
 def revsetpredicate(repo, subset, x, order):
     """Strictly interpret the content as a revset.
diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -585,7 +585,7 @@  def _quote(s):
 
 def _formatargtype(c, arg):
     if c == 'd':
-        return 'rev(%d)' % int(arg)
+        return '_rev(%d)' % int(arg)
     elif c == 's':
         return _quote(arg)
     elif c == 'r':
@@ -663,9 +663,9 @@  def formatspec(expr, *args):
     >>> formatspec(b'%r:: and %lr', b'10 or 11', (b"this()", b"that()"))
     '(10 or 11):: and ((this()) or (that()))'
     >>> formatspec(b'%d:: and not %d::', 10, 20)
-    'rev(10):: and not rev(20)::'
+    '_rev(10):: and not _rev(20)::'
     >>> formatspec(b'%ld or %ld', [], [1])
-    "_list('') or rev(1)"
+    "_list('') or _rev(1)"
     >>> formatspec(b'keyword(%s)', b'foo\\xe9')
     "keyword('foo\\\\xe9')"
     >>> b = lambda: b'default'