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

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 26, 2015, 8:35 p.m.
Message ID <f80655710170e55cd8bd.1432672501@Iris>
Download mbox | patch
Permalink /patch/9287/
State Superseded
Commit 2cfb0bbf83a10646938d4a255fac576896985584
Headers show

Comments

Jordi Gutiérrez Hermoso - May 26, 2015, 8:35 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1432672027 14400
#      Tue May 26 16:27:07 2015 -0400
# Node ID f80655710170e55cd8bd36a23802a4ad85ac73aa
# Parent  ac165f67eab4ab8157f73ab229e80883d49fabe0
hooks: replace if-try-finally with a "with" statement

This seems like a textbook case for the new demandimport.importnow
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.importnow():
             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:]: