Patchwork [4,of,4] bisect: move 'printresult' in the 'hbisect' module

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 3, 2016, 2:37 p.m.
Message ID <0aea0bb1821e36d03049.1475505440@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/16830/
State Accepted
Headers show

Comments

Pierre-Yves David - Oct. 3, 2016, 2:37 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1472005151 -7200
#      Wed Aug 24 04:19:11 2016 +0200
# Node ID 0aea0bb1821e36d03049dee5d6aaf8d6bfa87eeb
# Parent  2c643f89b68448619f6cf4326c58ef8cc686e51a
# EXP-Topic bisect
bisect: move 'printresult' in the 'hbisect' module

The logic is already extracted into a closure. We move it into the module
dedicated to bisect.

A minor change is applied: the creation of the 'displayer' is kept in the main
command function, it remove the needs to import 'cmdutil' in 'hbisect'. This
would create an import circle otherwise.
Ryan McElroy - Oct. 5, 2016, 3:59 p.m.
On 10/3/2016 3:37 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1472005151 -7200
> #      Wed Aug 24 04:19:11 2016 +0200
> # Node ID 0aea0bb1821e36d03049dee5d6aaf8d6bfa87eeb
> # Parent  2c643f89b68448619f6cf4326c58ef8cc686e51a
> # EXP-Topic bisect
> bisect: move 'printresult' in the 'hbisect' module
>
> The logic is already extracted into a closure. We move it into the module
> dedicated to bisect.

This series looks good to me as-is.
Augie Fackler - Oct. 8, 2016, 10:08 a.m.
On Wed, Oct 05, 2016 at 04:59:13PM +0100, Ryan McElroy wrote:
> On 10/3/2016 3:37 PM, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> > # Date 1472005151 -7200
> > #      Wed Aug 24 04:19:11 2016 +0200
> > # Node ID 0aea0bb1821e36d03049dee5d6aaf8d6bfa87eeb
> > # Parent  2c643f89b68448619f6cf4326c58ef8cc686e51a
> > # EXP-Topic bisect
> > bisect: move 'printresult' in the 'hbisect' module
> >
> > The logic is already extracted into a closure. We move it into the module
> > dedicated to bisect.
>
> This series looks good to me as-is.

Queued, thanks for the review.

> _______________________________________________
> 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
@@ -835,33 +835,6 @@  def bisect(ui, repo, rev=None, extra=Non
 
     Returns 0 on success.
     """
-    def print_result(nodes, good):
-        displayer = cmdutil.show_changeset(ui, repo, {})
-        if len(nodes) == 1:
-            # narrowed it down to a single revision
-            if good:
-                ui.write(_("The first good revision is:\n"))
-            else:
-                ui.write(_("The first bad revision is:\n"))
-            displayer.show(repo[nodes[0]])
-            extendnode = hbisect.extendrange(repo, state, nodes, good)
-            if extendnode is not None:
-                ui.write(_('Not all ancestors of this changeset have been'
-                           ' checked.\nUse bisect --extend to continue the '
-                           'bisection from\nthe common ancestor, %s.\n')
-                         % extendnode)
-        else:
-            # multiple possible revisions
-            if good:
-                ui.write(_("Due to skipped revisions, the first "
-                        "good revision could be any of:\n"))
-            else:
-                ui.write(_("Due to skipped revisions, the first "
-                        "bad revision could be any of:\n"))
-            for n in nodes:
-                displayer.show(repo[n])
-        displayer.close()
-
     def check_state(state, interactive=True):
         if not state['good'] or not state['bad']:
             if (good or bad or skip or reset) and interactive:
@@ -937,7 +910,8 @@  def bisect(ui, repo, rev=None, extra=Non
         finally:
             state['current'] = [node]
             hbisect.save_state(repo, state)
-        print_result(nodes, bgood)
+        displayer = cmdutil.show_changeset(ui, repo, {})
+        hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
         return
 
     # update state
@@ -976,7 +950,8 @@  def bisect(ui, repo, rev=None, extra=Non
         raise error.Abort(_("nothing to extend"))
 
     if changesets == 0:
-        print_result(nodes, good)
+        displayer = cmdutil.show_changeset(ui, repo, {})
+        hbisect.printresult(ui, repo, state, displayer, nodes, good)
     else:
         assert len(nodes) == 1 # only a single node can be tested next
         node = nodes[0]
diff --git a/mercurial/hbisect.py b/mercurial/hbisect.py
--- a/mercurial/hbisect.py
+++ b/mercurial/hbisect.py
@@ -279,3 +279,29 @@  def shortlabel(label):
         return label[0].upper()
 
     return None
+
+def printresult(ui, repo, state, displayer, nodes, good):
+    if len(nodes) == 1:
+        # narrowed it down to a single revision
+        if good:
+            ui.write(_("The first good revision is:\n"))
+        else:
+            ui.write(_("The first bad revision is:\n"))
+        displayer.show(repo[nodes[0]])
+        extendnode = extendrange(repo, state, nodes, good)
+        if extendnode is not None:
+            ui.write(_('Not all ancestors of this changeset have been'
+                       ' checked.\nUse bisect --extend to continue the '
+                       'bisection from\nthe common ancestor, %s.\n')
+                     % extendnode)
+    else:
+        # multiple possible revisions
+        if good:
+            ui.write(_("Due to skipped revisions, the first "
+                    "good revision could be any of:\n"))
+        else:
+            ui.write(_("Due to skipped revisions, the first "
+                    "bad revision could be any of:\n"))
+        for n in nodes:
+            displayer.show(repo[n])
+    displayer.close()