Patchwork [4,of,4] revset: factor out public optimize() function from recursion

login
register
mail settings
Submitter Yuya Nishihara
Date May 5, 2016, 7:57 a.m.
Message ID <ab27ea196841de780186.1462435055@mimosa>
Download mbox | patch
Permalink /patch/14907/
State Accepted
Headers show

Comments

Yuya Nishihara - May 5, 2016, 7:57 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1462158540 -32400
#      Mon May 02 12:09:00 2016 +0900
# Node ID ab27ea196841de780186db4d25ed7b7e1b42625e
# Parent  623d1c438bfe30c0937cb16f7a1873f22e6ae4a0
revset: factor out public optimize() function from recursion

New optimize() hides internal arguments and return values. This makes it easy
to add more parameters and return values to _optimize().
Sean Farley - May 5, 2016, 11:21 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1462158540 -32400
> #      Mon May 02 12:09:00 2016 +0900
> # Node ID ab27ea196841de780186db4d25ed7b7e1b42625e
> # Parent  623d1c438bfe30c0937cb16f7a1873f22e6ae4a0
> revset: factor out public optimize() function from recursion
>
> New optimize() hides internal arguments and return values. This makes it easy
> to add more parameters and return values to _optimize().

Sure, looks good to me.
Pierre-Yves David - May 9, 2016, 1:22 p.m.
Pushed, thanks

On 05/06/2016 01:21 AM, Sean Farley wrote:
> Yuya Nishihara <yuya@tcha.org> writes:
>
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1462158540 -32400
>> #      Mon May 02 12:09:00 2016 +0900
>> # Node ID ab27ea196841de780186db4d25ed7b7e1b42625e
>> # Parent  623d1c438bfe30c0937cb16f7a1873f22e6ae4a0
>> revset: factor out public optimize() function from recursion
>>
>> New optimize() hides internal arguments and return values. This makes it easy
>> to add more parameters and return values to _optimize().
> Sure, looks good to me.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3514,7 +3514,7 @@  def debugrevspec(ui, repo, expr, **opts)
         if newtree != tree:
             ui.note("* concatenated:\n", revset.prettyformat(newtree), "\n")
         if opts["optimize"]:
-            weight, optimizedtree = revset.optimize(newtree, True)
+            optimizedtree = revset.optimize(newtree)
             ui.note("* optimized:\n", revset.prettyformat(optimizedtree), "\n")
     func = revset.match(ui, expr, repo)
     revs = func(repo)
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2088,7 +2088,7 @@  def _matchonly(revs, bases):
         and getstring(bases[1][1], _('not a symbol')) == 'ancestors'):
         return ('list', revs[2], bases[1][2])
 
-def optimize(x, small):
+def _optimize(x, small):
     if x is None:
         return 0, x
 
@@ -2098,30 +2098,30 @@  def optimize(x, small):
 
     op = x[0]
     if op == 'minus':
-        return optimize(('and', x[1], ('not', x[2])), small)
+        return _optimize(('and', x[1], ('not', x[2])), small)
     elif op == 'only':
         t = ('func', ('symbol', 'only'), ('list', x[1], x[2]))
-        return optimize(t, small)
+        return _optimize(t, small)
     elif op == 'onlypost':
-        return optimize(('func', ('symbol', 'only'), x[1]), small)
+        return _optimize(('func', ('symbol', 'only'), x[1]), small)
     elif op == 'dagrangepre':
-        return optimize(('func', ('symbol', 'ancestors'), x[1]), small)
+        return _optimize(('func', ('symbol', 'ancestors'), x[1]), small)
     elif op == 'dagrangepost':
-        return optimize(('func', ('symbol', 'descendants'), x[1]), small)
+        return _optimize(('func', ('symbol', 'descendants'), x[1]), small)
     elif op == 'rangeall':
-        return optimize(('range', ('string', '0'), ('string', 'tip')), small)
+        return _optimize(('range', ('string', '0'), ('string', 'tip')), small)
     elif op == 'rangepre':
-        return optimize(('range', ('string', '0'), x[1]), small)
+        return _optimize(('range', ('string', '0'), x[1]), small)
     elif op == 'rangepost':
-        return optimize(('range', x[1], ('string', 'tip')), small)
+        return _optimize(('range', x[1], ('string', 'tip')), small)
     elif op == 'negate':
         s = getstring(x[1], _("can't negate that"))
-        return optimize(('string', '-' + s), small)
+        return _optimize(('string', '-' + s), small)
     elif op in 'string symbol negate':
         return smallbonus, x # single revisions are small
     elif op == 'and':
-        wa, ta = optimize(x[1], True)
-        wb, tb = optimize(x[2], True)
+        wa, ta = _optimize(x[1], True)
+        wb, tb = _optimize(x[2], True)
         w = min(wa, wb)
 
         # (::x and not ::y)/(not ::y and ::x) have a fast path
@@ -2147,12 +2147,12 @@  def optimize(x, small):
             else:
                 s = '\0'.join(t[1] for w, t in ss)
                 y = ('func', ('symbol', '_list'), ('string', s))
-                w, t = optimize(y, False)
+                w, t = _optimize(y, False)
             ws.append(w)
             ts.append(t)
             del ss[:]
         for y in x[1:]:
-            w, t = optimize(y, False)
+            w, t = _optimize(y, False)
             if t is not None and (t[0] == 'string' or t[0] == 'symbol'):
                 ss.append((w, t))
                 continue
@@ -2170,34 +2170,34 @@  def optimize(x, small):
         # Optimize not public() to _notpublic() because we have a fast version
         if x[1] == ('func', ('symbol', 'public'), None):
             newsym = ('func', ('symbol', '_notpublic'), None)
-            o = optimize(newsym, not small)
+            o = _optimize(newsym, not small)
             return o[0], o[1]
         else:
-            o = optimize(x[1], not small)
+            o = _optimize(x[1], not small)
             return o[0], (op, o[1])
     elif op == 'parentpost':
-        o = optimize(x[1], small)
+        o = _optimize(x[1], small)
         return o[0], (op, o[1])
     elif op == 'group':
-        return optimize(x[1], small)
+        return _optimize(x[1], small)
     elif op in 'dagrange range parent ancestorspec':
         if op == 'parent':
             # x^:y means (x^) : y, not x ^ (:y)
             post = ('parentpost', x[1])
             if x[2][0] == 'dagrangepre':
-                return optimize(('dagrange', post, x[2][1]), small)
+                return _optimize(('dagrange', post, x[2][1]), small)
             elif x[2][0] == 'rangepre':
-                return optimize(('range', post, x[2][1]), small)
-
-        wa, ta = optimize(x[1], small)
-        wb, tb = optimize(x[2], small)
+                return _optimize(('range', post, x[2][1]), small)
+
+        wa, ta = _optimize(x[1], small)
+        wb, tb = _optimize(x[2], small)
         return wa + wb, (op, ta, tb)
     elif op == 'list':
-        ws, ts = zip(*(optimize(y, small) for y in x[1:]))
+        ws, ts = zip(*(_optimize(y, small) for y in x[1:]))
         return sum(ws), (op,) + ts
     elif op == 'func':
         f = getstring(x[1], _("not a symbol"))
-        wa, ta = optimize(x[2], small)
+        wa, ta = _optimize(x[2], small)
         if f in ("author branch closed date desc file grep keyword "
                  "outgoing user"):
             w = 10 # slow
@@ -2216,6 +2216,10 @@  def optimize(x, small):
         return w + wa, (op, x[1], ta)
     return 1, x
 
+def optimize(tree):
+    _weight, newtree = _optimize(tree, small=True)
+    return newtree
+
 # the set of valid characters for the initial letter of symbols in
 # alias declarations and definitions
 _aliassyminitletters = set(c for c in [chr(i) for i in xrange(256)]
@@ -2331,7 +2335,7 @@  def _makematcher(ui, tree, repo):
     if ui:
         tree = expandaliases(ui, tree, showwarning=ui.warn)
     tree = foldconcat(tree)
-    weight, tree = optimize(tree, True)
+    tree = optimize(tree)
     posttreebuilthook(tree, repo)
     def mfunc(repo, subset=None):
         if subset is None:
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -751,7 +751,7 @@  def revsingle(repo, revspec, default='.'
 
 def _pairspec(revspec):
     tree = revset.parse(revspec)
-    tree = revset.optimize(tree, True)[1]  # fix up "x^:y" -> "(x^):y"
+    tree = revset.optimize(tree)  # fix up "x^:y" -> "(x^):y"
     return tree and tree[0] in ('range', 'rangepre', 'rangepost', 'rangeall')
 
 def revpair(repo, revs):