Patchwork [2,of,2] revset: define table of sort() key functions

login
register
mail settings
Submitter Yuya Nishihara
Date May 25, 2016, 2:37 p.m.
Message ID <edbf799691811456b2fc.1464187075@mimosa>
Download mbox | patch
Permalink /patch/15196/
State Accepted
Headers show

Comments

Yuya Nishihara - May 25, 2016, 2:37 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1463223120 -32400
#      Sat May 14 19:52:00 2016 +0900
# Node ID edbf799691811456b2fc052750221672d4ee3e43
# Parent  63b2515cfc673094cc085ef929aeb34ace042e74
revset: define table of sort() key functions

This should be more readable than big "if" branch.
Martijn Pieters - May 25, 2016, 5:48 p.m.
On 25 May 2016 at 07:37, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1463223120 -32400
> #      Sat May 14 19:52:00 2016 +0900
> # Node ID edbf799691811456b2fc052750221672d4ee3e43
> # Parent  63b2515cfc673094cc085ef929aeb34ace042e74
> revset: define table of sort() key functions
>
> This should be more readable than big "if" branch.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1834,6 +1834,15 @@ def roots(repo, subset, x):
>          return True
>      return subset & s.filter(filter, condrepr='<roots>')
>
> +_sortkeyfuncs = {
> +    'rev': lambda c: c.rev(),
> +    'branch': lambda c: c.branch(),
> +    'desc': lambda c: c.description(),
> +    'user': lambda c: c.user(),
> +    'author': lambda c: c.user(),
> +    'date': lambda c: c.date()[0],
> +}
> +
>  @predicate('sort(set[, [-]key...])', safe=True)
>  def sort(repo, subset, x):
>      """Sort set by keys. The default sort order is ascending, specify a key
> @@ -1870,17 +1879,9 @@ def sort(repo, subset, x):
>          reverse = (k[0] == '-')
>          if reverse:
>              k = k[1:]
> -        if k == 'rev':
> -            ctxs.sort(key=lambda c: c.rev(), reverse=reverse)
> -        elif k == 'branch':
> -            ctxs.sort(key=lambda c: c.branch(), reverse=reverse)
> -        elif k == 'desc':
> -            ctxs.sort(key=lambda c: c.description(), reverse=reverse)
> -        elif k in 'user author':
> -            ctxs.sort(key=lambda c: c.user(), reverse=reverse)
> -        elif k == 'date':
> -            ctxs.sort(key=lambda c: c.date()[0], reverse=reverse)
> -        else:
> +        try:
> +            ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
> +        except KeyError:
>              raise error.ParseError(_("unknown sort key %r") % fk)
>      return baseset([c.rev() for c in ctxs])
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

+1 from me; I was considering this exact change.
Sean Farley - May 25, 2016, 9:50 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1463223120 -32400
> #      Sat May 14 19:52:00 2016 +0900
> # Node ID edbf799691811456b2fc052750221672d4ee3e43
> # Parent  63b2515cfc673094cc085ef929aeb34ace042e74
> revset: define table of sort() key functions
>
> This should be more readable than big "if" branch.

This also looks good to me.
Augie Fackler - May 29, 2016, 9:02 p.m.
On Wed, May 25, 2016 at 11:37:55PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1463223120 -32400
> #      Sat May 14 19:52:00 2016 +0900
> # Node ID edbf799691811456b2fc052750221672d4ee3e43
> # Parent  63b2515cfc673094cc085ef929aeb34ace042e74
> revset: define table of sort() key functions

Queued these, thanks.

>
> This should be more readable than big "if" branch.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1834,6 +1834,15 @@ def roots(repo, subset, x):
>          return True
>      return subset & s.filter(filter, condrepr='<roots>')
>
> +_sortkeyfuncs = {
> +    'rev': lambda c: c.rev(),
> +    'branch': lambda c: c.branch(),
> +    'desc': lambda c: c.description(),
> +    'user': lambda c: c.user(),
> +    'author': lambda c: c.user(),
> +    'date': lambda c: c.date()[0],
> +}
> +
>  @predicate('sort(set[, [-]key...])', safe=True)
>  def sort(repo, subset, x):
>      """Sort set by keys. The default sort order is ascending, specify a key
> @@ -1870,17 +1879,9 @@ def sort(repo, subset, x):
>          reverse = (k[0] == '-')
>          if reverse:
>              k = k[1:]
> -        if k == 'rev':
> -            ctxs.sort(key=lambda c: c.rev(), reverse=reverse)
> -        elif k == 'branch':
> -            ctxs.sort(key=lambda c: c.branch(), reverse=reverse)
> -        elif k == 'desc':
> -            ctxs.sort(key=lambda c: c.description(), reverse=reverse)
> -        elif k in 'user author':
> -            ctxs.sort(key=lambda c: c.user(), reverse=reverse)
> -        elif k == 'date':
> -            ctxs.sort(key=lambda c: c.date()[0], reverse=reverse)
> -        else:
> +        try:
> +            ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
> +        except KeyError:
>              raise error.ParseError(_("unknown sort key %r") % fk)
>      return baseset([c.rev() for c in ctxs])
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1834,6 +1834,15 @@  def roots(repo, subset, x):
         return True
     return subset & s.filter(filter, condrepr='<roots>')
 
+_sortkeyfuncs = {
+    'rev': lambda c: c.rev(),
+    'branch': lambda c: c.branch(),
+    'desc': lambda c: c.description(),
+    'user': lambda c: c.user(),
+    'author': lambda c: c.user(),
+    'date': lambda c: c.date()[0],
+}
+
 @predicate('sort(set[, [-]key...])', safe=True)
 def sort(repo, subset, x):
     """Sort set by keys. The default sort order is ascending, specify a key
@@ -1870,17 +1879,9 @@  def sort(repo, subset, x):
         reverse = (k[0] == '-')
         if reverse:
             k = k[1:]
-        if k == 'rev':
-            ctxs.sort(key=lambda c: c.rev(), reverse=reverse)
-        elif k == 'branch':
-            ctxs.sort(key=lambda c: c.branch(), reverse=reverse)
-        elif k == 'desc':
-            ctxs.sort(key=lambda c: c.description(), reverse=reverse)
-        elif k in 'user author':
-            ctxs.sort(key=lambda c: c.user(), reverse=reverse)
-        elif k == 'date':
-            ctxs.sort(key=lambda c: c.date()[0], reverse=reverse)
-        else:
+        try:
+            ctxs.sort(key=_sortkeyfuncs[k], reverse=reverse)
+        except KeyError:
             raise error.ParseError(_("unknown sort key %r") % fk)
     return baseset([c.rev() for c in ctxs])