Patchwork [STABLE] revset: fix keyword arguments to go through optimization process

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 7, 2016, 11:46 a.m.
Message ID <77a733ef85bfd554ac0f.1470570414@mimosa>
Download mbox | patch
Permalink /patch/16169/
State Accepted
Headers show

Comments

Yuya Nishihara - Aug. 7, 2016, 11:46 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1470549529 -32400
#      Sun Aug 07 14:58:49 2016 +0900
# Branch stable
# Node ID 77a733ef85bfd554ac0f5f1f9fa5a7088727ae7b
# Parent  b8f9cdca88077e97d4869320b9d18481fbe252ef
revset: fix keyword arguments to go through optimization process

Before, a keyvalue node was processed by the last catch-all condition of
_optimize(). Therefore, topo.firstbranch=expr would bypass tree rewriting
and would crash if an expr wasn't trivial.
Pierre-Yves David - Aug. 11, 2016, 7:40 p.m.
On 08/07/2016 01:46 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1470549529 -32400
> #      Sun Aug 07 14:58:49 2016 +0900
> # Branch stable
> # Node ID 77a733ef85bfd554ac0f5f1f9fa5a7088727ae7b
> # Parent  b8f9cdca88077e97d4869320b9d18481fbe252ef
> revset: fix keyword arguments to go through optimization process
>
> Before, a keyvalue node was processed by the last catch-all condition of
> _optimize(). Therefore, topo.firstbranch=expr would bypass tree rewriting
> and would crash if an expr wasn't trivial.

This seems correct to me, pushed.
Thanks. Does this mean you use topo sorting somewhere?
Yuya Nishihara - Aug. 12, 2016, 12:27 a.m.
On Thu, 11 Aug 2016 21:40:14 +0200, Pierre-Yves David wrote:
> On 08/07/2016 01:46 PM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1470549529 -32400
> > #      Sun Aug 07 14:58:49 2016 +0900
> > # Branch stable
> > # Node ID 77a733ef85bfd554ac0f5f1f9fa5a7088727ae7b
> > # Parent  b8f9cdca88077e97d4869320b9d18481fbe252ef
> > revset: fix keyword arguments to go through optimization process
> >
> > Before, a keyvalue node was processed by the last catch-all condition of
> > _optimize(). Therefore, topo.firstbranch=expr would bypass tree rewriting
> > and would crash if an expr wasn't trivial.
> 
> This seems correct to me, pushed.
> Thanks. Does this mean you use topo sorting somewhere?

No. I just found the bug while refactoring _optimize() to make my new revset
ordering series less sucky.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2423,6 +2423,9 @@  def _optimize(x, small):
     elif op == 'list':
         ws, ts = zip(*(_optimize(y, small) for y in x[1:]))
         return sum(ws), (op,) + ts
+    elif op == 'keyvalue':
+        w, t = _optimize(x[2], small)
+        return w, (op, x[1], t)
     elif op == 'func':
         f = getsymbol(x[1])
         wa, ta = _optimize(x[2], small)
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -470,6 +470,25 @@  keyword arguments
   hg: parse error: can't use a key-value pair in this context
   [255]
 
+ right-hand side should be optimized recursively
+
+  $ try --optimize 'foo=(not public())'
+  (keyvalue
+    ('symbol', 'foo')
+    (group
+      (not
+        (func
+          ('symbol', 'public')
+          None))))
+  * optimized:
+  (keyvalue
+    ('symbol', 'foo')
+    (func
+      ('symbol', '_notpublic')
+      None))
+  hg: parse error: can't use a key-value pair in this context
+  [255]
+
 Test that symbols only get parsed as functions if there's an opening
 parenthesis.
 
@@ -1649,6 +1668,11 @@  use the topo.firstbranch option when top
   hg: parse error: topo.firstbranch can only be used when using the topo sort key
   [255]
 
+topo.firstbranch should accept any kind of expressions:
+
+  $ hg log -r 'sort(0, topo, topo.firstbranch=(book1))'
+  0 b12  m111 u112 111 10800
+
   $ cd ..
   $ cd repo