Patchwork [2,of,3,V2] hooks: replace if-try-finally with a "with" statement

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 28, 2015, 8:44 p.m.
Message ID <8ba3d6abebd87af2166b.1432845872@Iris>
Download mbox | patch
Permalink /patch/9341/
State Accepted
Headers show

Comments

Jordi Gutiérrez Hermoso - May 28, 2015, 8:44 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1432845724 14400
#      Thu May 28 16:42:04 2015 -0400
# Node ID 8ba3d6abebd87af2166b84936a45df850af86dad
# Parent  2727a8bbdce7d482bb3877f9d36df09483a4895b
hooks: replace if-try-finally with a "with" statement

This seems like a textbook case for the new demandimport.deactivated
context manager: check if something must be done, do it, and cleanup
at the end regardless of exceptions.

The diff isn't as bad as it seems. It's just all the whitespace
changes due to needing an extra level of indentation. It looks cleaner
with `hg diff -w`.

Patch

diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -35,32 +35,27 @@  def _pythonhook(ui, repo, name, hname, f
             if modpath and modfile:
                 sys.path = sys.path[:] + [modpath]
                 modname = modfile
-        demandimportenabled = demandimport.isenabled()
-        if demandimportenabled:
-            demandimport.disable()
-        try:
-            obj = __import__(modname)
-        except ImportError:
-            e1 = sys.exc_type, sys.exc_value, sys.exc_traceback
+        with demandimport.deactivated():
             try:
-                # extensions are loaded with hgext_ prefix
-                obj = __import__("hgext_%s" % modname)
+                obj = __import__(modname)
             except ImportError:
-                e2 = sys.exc_type, sys.exc_value, sys.exc_traceback
-                if ui.tracebackflag:
-                    ui.warn(_('exception from first failed import '
-                              'attempt:\n'))
-                ui.traceback(e1)
-                if ui.tracebackflag:
-                    ui.warn(_('exception from second failed import '
-                              'attempt:\n'))
-                ui.traceback(e2)
-                raise util.Abort(_('%s hook is invalid '
-                                   '(import of "%s" failed)') %
-                                 (hname, modname))
-        finally:
-            if demandimportenabled:
-                demandimport.enable()
+                e1 = sys.exc_type, sys.exc_value, sys.exc_traceback
+                try:
+                    # extensions are loaded with hgext_ prefix
+                    obj = __import__("hgext_%s" % modname)
+                except ImportError:
+                    e2 = sys.exc_type, sys.exc_value, sys.exc_traceback
+                    if ui.tracebackflag:
+                        ui.warn(_('exception from first failed import '
+                                  'attempt:\n'))
+                    ui.traceback(e1)
+                    if ui.tracebackflag:
+                        ui.warn(_('exception from second failed import '
+                                  'attempt:\n'))
+                    ui.traceback(e2)
+                    raise util.Abort(_('%s hook is invalid '
+                                       '(import of "%s" failed)') %
+                                     (hname, modname))
         sys.path = oldpaths
         try:
             for p in funcname.split('.')[1:]: