Patchwork [1,of,4] revset: fix crash on empty sort key

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

Comments

Yuya Nishihara - June 15, 2016, 1:42 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1465990644 -32400
#      Wed Jun 15 20:37:24 2016 +0900
# Node ID 05e51f240485ea6a8490342deda2ce1f08c33bbf
# Parent  8bf84295e59ba0c98e64fd7ccf3f809e4da5b237
revset: fix crash on empty sort key

Make it noop as before 2188f170f5b6. We could change it to an error, but
allowing empty key makes some sense for scripting that builds a key string
programmatically.
Jun Wu - June 15, 2016, 1:51 p.m.
Looks good to me.

Excerpts from Yuya Nishihara's message of 2016-06-15 22:42:52 +0900:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1465990644 -32400
> #      Wed Jun 15 20:37:24 2016 +0900
> # Node ID 05e51f240485ea6a8490342deda2ce1f08c33bbf
> # Parent  8bf84295e59ba0c98e64fd7ccf3f809e4da5b237
> revset: fix crash on empty sort key
> 
> Make it noop as before 2188f170f5b6. We could change it to an error, but
> allowing empty key makes some sense for scripting that builds a key string
> programmatically.
> 
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1890,6 +1890,8 @@ def sort(repo, subset, x):
>                  'topo.firstbranch can only be used when using the topo sort '
>                  'key'))
>  
> +    if not keys:
> +        return revs
>      if keys == ["rev"]:
>          revs.sort()
>          return revs
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -962,6 +962,13 @@ test sorting two sorted collections in d
>    6
>    2
>  
> +test empty sort key which is noop
> +
> +  $ log 'sort(0 + 2 + 1, "")'
> +  0
> +  2
> +  1
> +
>  test invalid sort keys
>  
>    $ log 'sort(all(), -invalid)'

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1890,6 +1890,8 @@  def sort(repo, subset, x):
                 'topo.firstbranch can only be used when using the topo sort '
                 'key'))
 
+    if not keys:
+        return revs
     if keys == ["rev"]:
         revs.sort()
         return revs
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -962,6 +962,13 @@  test sorting two sorted collections in d
   6
   2
 
+test empty sort key which is noop
+
+  $ log 'sort(0 + 2 + 1, "")'
+  0
+  2
+  1
+
 test invalid sort keys
 
   $ log 'sort(all(), -invalid)'