Patchwork [2,of,2] remotenames: follow-up on D3639 to make revset funcs take only one arg

login
register
mail settings
Submitter Augie Fackler
Date Oct. 5, 2018, 2:39 a.m.
Message ID <969132f98a74ecad2095.1538707164@imladris.local>
Download mbox | patch
Permalink /patch/35477/
State Accepted
Headers show

Comments

Augie Fackler - Oct. 5, 2018, 2:39 a.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1538703312 14400
#      Thu Oct 04 21:35:12 2018 -0400
# Node ID 969132f98a74ecad2095c9099556fe7c54d1c84f
# Parent  6638bd3f061b89b3b0bf432799da79e1fee5eb62
remotenames: follow-up on D3639 to make revset funcs take only one arg

Per the review discussion on D3639, we want this to just take one
argument. That ended up simplifying the code, so I'm sharing this as a
follow-up to that revision rather than editing in-flight.
Yuya Nishihara - Oct. 5, 2018, 11:29 a.m.
On Thu, 04 Oct 2018 22:39:24 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1538703312 14400
> #      Thu Oct 04 21:35:12 2018 -0400
> # Node ID 969132f98a74ecad2095c9099556fe7c54d1c84f
> # Parent  6638bd3f061b89b3b0bf432799da79e1fee5eb62
> remotenames: follow-up on D3639 to make revset funcs take only one arg

Queued, thanks.

> +                if not matcher(name):
> +                    continue
> +                revs.update(ns.nodes(repo, name))

Perhaps, it has to abort if the match pattern is a literal, and the
exact name does not exist. I'll send a follow up.
Pulkit Goyal - Oct. 5, 2018, 1:01 p.m.
On Fri, Oct 5, 2018 at 2:37 PM Yuya Nishihara <yuya@tcha.org> wrote:

> On Thu, 04 Oct 2018 22:39:24 -0400, Augie Fackler wrote:
> > # HG changeset patch
> > # User Augie Fackler <raf@durin42.com>
> > # Date 1538703312 14400
> > #      Thu Oct 04 21:35:12 2018 -0400
> > # Node ID 969132f98a74ecad2095c9099556fe7c54d1c84f
> > # Parent  6638bd3f061b89b3b0bf432799da79e1fee5eb62
> > remotenames: follow-up on D3639 to make revset funcs take only one arg
>

Thanks a lot for doing this!

Patch

diff --git a/hgext/remotenames.py b/hgext/remotenames.py
--- a/hgext/remotenames.py
+++ b/hgext/remotenames.py
@@ -347,28 +347,18 @@  def remotebrancheskw(context, mapping):
     return templateutil.compatlist(context, mapping, 'remotebranch',
                                    remotebranches, plural='remotebranches')
 
-def _revsetutil(repo, subset, x, rtypes, args):
+def _revsetutil(repo, subset, x, rtypes, matcher):
     """utility function to return a set of revs based on the rtypes"""
 
     revs = set()
     cl = repo.changelog
-    literals, matchers = args
-    # whether arguments were passed or not
-    argspassed = bool(literals or matchers)
     for rtype in rtypes:
         if rtype in repo.names:
             ns = repo.names[rtype]
             for name in ns.listnames(repo):
-                if argspassed:
-                    if name in literals:
-                        revs.update(ns.nodes(repo, name))
-                        continue
-                    for matcher in matchers:
-                        if matcher(name):
-                            revs.update(ns.nodes(repo, name))
-                            break
-                else:
-                    revs.update(ns.nodes(repo, name))
+                if not matcher(name):
+                    continue
+                revs.update(ns.nodes(repo, name))
 
     results = (cl.rev(n) for n in revs if cl.hasnode(n))
     return subset & smartset.baseset(sorted(results))
@@ -376,48 +366,29 @@  def _revsetutil(repo, subset, x, rtypes,
 def _parseargs(x):
     """parses the argument passed in revsets
 
-    returns (literals, matchers) where,
-        literals is a set of literals passed by user
-        matchers is a list of matcher objects for patterns passed by user
+    Returns a matcher for the passed pattern.
     """
-
-    # set of paths passed as literals
-    literals = set()
-    # list of matcher to match the patterns passed as names
-    matchers = []
-
-    if not x:
-        return literals, matchers
-
-    args = set()
-    lx = revsetlang.getlist(x)
-    err = _('the argument must be a string')
-    for entry in lx:
-        args.add(revsetlang.getstring(entry, err))
-    for p in args:
-        kind, pattern, matcher = stringutil.stringmatcher(p)
-        if kind == 'literal':
-            literals.add(pattern)
-        else:
-            matchers.append(matcher)
-    return literals, matchers
-
-@revsetpredicate('remotenames([name, ...])')
+    args = revsetlang.getargs(x, 0, 1, _('only one argument accepted'))
+    for arg in args:
+        kind, pattern, matcher = stringutil.stringmatcher(
+            revsetlang.getstring(arg, _('argument must be a string')))
+        return matcher
+    return lambda a: True
+
+@revsetpredicate('remotenames([name])')
 def remotenamesrevset(repo, subset, x):
-    """All changesets which have a remotename on them. If paths are specified,
-    remotenames of those remote paths are only considered.
+    """All changesets which have a remotename on them. If `name` is
+    specified, only remotenames of matching remote paths are considered.
 
     Pattern matching is supported for `name`. See :hg:`help revisions.patterns`.
     """
-
-    args = _parseargs(x)
     return _revsetutil(repo, subset, x, ('remotebookmarks', 'remotebranches'),
-                       args)
+                       _parseargs(x))
 
-@revsetpredicate('remotebranches([name, ...])')
+@revsetpredicate('remotebranches([name])')
 def remotebranchesrevset(repo, subset, x):
-    """All changesets which are branch heads on remotes. If paths are specified,
-    only those remotes paths are considered.
+    """All changesets which are branch heads on remotes. If `name` is
+    specified, only remotenames of matching remote paths are considered.
 
     Pattern matching is supported for `name`. See :hg:`help revisions.patterns`.
     """
@@ -425,10 +396,10 @@  def remotebranchesrevset(repo, subset, x
     args = _parseargs(x)
     return _revsetutil(repo, subset, x, ('remotebranches',), args)
 
-@revsetpredicate('remotebookmarks([name, ...])')
+@revsetpredicate('remotebookmarks([name])')
 def remotebmarksrevset(repo, subset, x):
-    """All changesets which have bookmarks on remotes. If paths are specified,
-    only those remote paths are considered.
+    """All changesets which have bookmarks on remotes. If `name` is
+    specified, only remotenames of matching remote paths are considered.
 
     Pattern matching is supported for `name`. See :hg:`help revisions.patterns`.
     """
diff --git a/tests/test-logexchange.t b/tests/test-logexchange.t
--- a/tests/test-logexchange.t
+++ b/tests/test-logexchange.t
@@ -486,67 +486,19 @@  Testing for a single name which does not
 
   $ hg log -r 'remotenames("server3")' -GT "{rev}:{node|short} {remotenames}\n"
 
-Testing for multiple names where all of them exists
+Testing for multiple names, which is not supported.
 
   $ hg log -r 'remotenames("re:default", "re:server2")' -GT "{rev}:{node|short} {remotenames}\n"
-  o  10:bf433e48adea server2/default
-  |
-  | o  9:f34adec73c21 server2/wat
-  | |
-  | o  8:3e1487808078 default/foo default/wat
-  | :
-  @ :  7:ec2426147f0e default/default
-  | :
-  o :  6:87d6d6676308 default/bar server2/bar
-  :/
-  o  3:62615734edd5 server2/foo
-  |
-  ~
+  hg: parse error: only one argument accepted
+  [255]
 
   $ hg log -r 'remotebranches("default/wat", "server2/wat")' -GT "{rev}:{node|short} {remotebranches}\n"
-  o  9:f34adec73c21 server2/wat
-  |
-  o  8:3e1487808078 default/wat
-  |
-  ~
+  hg: parse error: only one argument accepted
+  [255]
 
   $ hg log -r 'remotebookmarks("default/foo", "server2/foo")' -GT "{rev}:{node|short} {remotebookmarks}\n"
-  o  8:3e1487808078 default/foo
-  :
-  o  3:62615734edd5 server2/foo
-  |
-  ~
-
-Testing for multipe names where some exists and some not
-
-  $ hg log -r 'remotenames(def, "re:server2")' -GT "{rev}:{node|short} {remotenames}\n"
-  o  10:bf433e48adea server2/default
-  :
-  : o  9:f34adec73c21 server2/wat
-  : :
-  o :  6:87d6d6676308 default/bar server2/bar
-  :/
-  o  3:62615734edd5 server2/foo
-  |
-  ~
-
-  $ hg log -r 'remotebranches("default/default", server)' -GT "{rev}:{node|short} {remotebranches}\n"
-  @  7:ec2426147f0e default/default
-  |
-  ~
-
-  $ hg log -r 'remotebookmarks("default/foo", serv)' -GT "{rev}:{node|short} {remotebookmarks}\n"
-  o  8:3e1487808078 default/foo
-  |
-  ~
-
-Where multiple names specified and None of them exists
-
-  $ hg log -r 'remotenames(def, serv2)' -GT "{rev}:{node|short} {remotenames}\n"
-
-  $ hg log -r 'remotebranches(defu, server)' -GT "{rev}:{node|short} {remotebranches}\n"
-
-  $ hg log -r 'remotebookmarks(delt, serv)' -GT "{rev}:{node|short} {remotebookmarks}\n"
+  hg: parse error: only one argument accepted
+  [255]
 
 Testing pattern matching
 
@@ -569,12 +521,3 @@  Testing pattern matching
   o  9:f34adec73c21 server2/wat
   |
   ~
-
-  $ hg log -r 'remotebookmarks("re:def", "re:.*2")' -GT "{rev}:{node|short} {remotebookmarks}\n"
-  o  8:3e1487808078 default/foo
-  :
-  : o  6:87d6d6676308 default/bar server2/bar
-  :/
-  o  3:62615734edd5 server2/foo
-  |
-  ~