Patchwork [2,of,4] revset: build list of (key, reverse) pairs before sorting

login
register
mail settings
Submitter Yuya Nishihara
Date June 15, 2016, 1:42 p.m.
Message ID <29d03589403b37677ae2.1465998173@mimosa>
Download mbox | patch
Permalink /patch/15515/
State Accepted
Headers show

Comments

Yuya Nishihara - June 15, 2016, 1:42 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1465607740 -32400
#      Sat Jun 11 10:15:40 2016 +0900
# Node ID 29d03589403b37677ae254f51476e07805b23d62
# Parent  05e51f240485ea6a8490342deda2ce1f08c33bbf
revset: build list of (key, reverse) pairs before sorting

Prepares for extracting a function that only validates sort options.
Martijn Pieters - June 16, 2016, 1:27 p.m.
On 15 June 2016 at 14:42, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1465607740 -32400
> #      Sat Jun 11 10:15:40 2016 +0900
> # Node ID 29d03589403b37677ae254f51476e07805b23d62
> # Parent  05e51f240485ea6a8490342deda2ce1f08c33bbf
> revset: build list of (key, reverse) pairs before sorting
>
> Prepares for extracting a function that only validates sort options.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1871,18 +1871,27 @@ def sort(repo, subset, x):
>          # i18n: "sort" is a keyword
>          keys = getstring(args['keys'], _("sort spec must be a string"))
>
> +    keyflags = []
> +    for k in keys.split():
> +        fk = k
> +        reverse = (k[0] == '-')
> +        if reverse:
> +            k = k[1:]
> +        if k not in _sortkeyfuncs and k != 'topo':
> +            raise error.ParseError(_("unknown sort key %r") % fk)
> +        keyflags.append((k, reverse))
> +
>      s = args['set']
> -    keys = keys.split()
>      revs = getset(repo, subset, s)
>
> -    if len(keys) > 1 and any(k.lstrip('-') == 'topo' for k in keys):
> +    if len(keyflags) > 1 and any(k == 'topo' for k, reverse in keyflags):
>          # i18n: "topo" is a keyword
>          raise error.ParseError(_(
>              'topo sort order cannot be combined with other sort keys'))
>
>      firstbranch = ()
>      if 'topo.firstbranch' in args:
> -        if any(k.lstrip('-') == 'topo' for k in keys):
> +        if any(k == 'topo' for k, reverse in keyflags):
>              firstbranch = getset(repo, subset, args['topo.firstbranch'])
>          else:
>              # i18n: "topo" and "topo.firstbranch" are keywords
> @@ -1890,32 +1899,22 @@ def sort(repo, subset, x):
>                  'topo.firstbranch can only be used when using the topo sort '
>                  'key'))
>
> -    if not keys:
> +    if not keyflags:
>          return revs
> -    if keys == ["rev"]:
> -        revs.sort()
> +    if len(keyflags) == 1 and keyflags[0][0] == "rev":
> +        revs.sort(reverse=keyflags[0][1])
>          return revs
> -    elif keys == ["-rev"]:
> -        revs.sort(reverse=True)
> -        return revs
> -    elif keys[0] in ("topo", "-topo"):
> +    elif keyflags[0][0] == "topo":
>          revs = baseset(_toposort(revs, repo.changelog.parentrevs, firstbranch),
>                         istopo=True)
> -        if keys[0][0] == '-':
> +        if keyflags[0][1]:
>              revs.reverse()
>          return revs
>
>      # sort() is guaranteed to be stable
>      ctxs = [repo[r] for r in revs]
> -    for k in reversed(keys):
> -        fk = k
> -        reverse = (k[0] == '-')
> -        if reverse:
> -            k = k[1:]
> -        try:
> -            ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
> -        except KeyError:
> -            raise error.ParseError(_("unknown sort key %r") % fk)
> +    for k, reverse in reversed(keyflags):
> +        ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
>      return baseset([c.rev() for c in ctxs])
>
>  def _toposort(revs, parentsfunc, firstbranch=()):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Looks great to me. Thanks for cleaning this up!

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1871,18 +1871,27 @@  def sort(repo, subset, x):
         # i18n: "sort" is a keyword
         keys = getstring(args['keys'], _("sort spec must be a string"))
 
+    keyflags = []
+    for k in keys.split():
+        fk = k
+        reverse = (k[0] == '-')
+        if reverse:
+            k = k[1:]
+        if k not in _sortkeyfuncs and k != 'topo':
+            raise error.ParseError(_("unknown sort key %r") % fk)
+        keyflags.append((k, reverse))
+
     s = args['set']
-    keys = keys.split()
     revs = getset(repo, subset, s)
 
-    if len(keys) > 1 and any(k.lstrip('-') == 'topo' for k in keys):
+    if len(keyflags) > 1 and any(k == 'topo' for k, reverse in keyflags):
         # i18n: "topo" is a keyword
         raise error.ParseError(_(
             'topo sort order cannot be combined with other sort keys'))
 
     firstbranch = ()
     if 'topo.firstbranch' in args:
-        if any(k.lstrip('-') == 'topo' for k in keys):
+        if any(k == 'topo' for k, reverse in keyflags):
             firstbranch = getset(repo, subset, args['topo.firstbranch'])
         else:
             # i18n: "topo" and "topo.firstbranch" are keywords
@@ -1890,32 +1899,22 @@  def sort(repo, subset, x):
                 'topo.firstbranch can only be used when using the topo sort '
                 'key'))
 
-    if not keys:
+    if not keyflags:
         return revs
-    if keys == ["rev"]:
-        revs.sort()
+    if len(keyflags) == 1 and keyflags[0][0] == "rev":
+        revs.sort(reverse=keyflags[0][1])
         return revs
-    elif keys == ["-rev"]:
-        revs.sort(reverse=True)
-        return revs
-    elif keys[0] in ("topo", "-topo"):
+    elif keyflags[0][0] == "topo":
         revs = baseset(_toposort(revs, repo.changelog.parentrevs, firstbranch),
                        istopo=True)
-        if keys[0][0] == '-':
+        if keyflags[0][1]:
             revs.reverse()
         return revs
 
     # sort() is guaranteed to be stable
     ctxs = [repo[r] for r in revs]
-    for k in reversed(keys):
-        fk = k
-        reverse = (k[0] == '-')
-        if reverse:
-            k = k[1:]
-        try:
-            ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
-        except KeyError:
-            raise error.ParseError(_("unknown sort key %r") % fk)
+    for k, reverse in reversed(keyflags):
+        ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
     return baseset([c.rev() for c in ctxs])
 
 def _toposort(revs, parentsfunc, firstbranch=()):