Merge lp:~nmb/bzr/894609-lazy-hook-names into lp:bzr

[Neil Martinsen-Burrell]

allow lazily registered hooks to have their names appear in "bzr hooks" (bug 894609)

This allows lazily registered hooks to have their names appear in "bzr hooks". It uses __name__ and __module__ of a callable to find the name when the hook was registered lazily. It doesn't work for all lazy hooks, particularly those registered with hooks.install_lazy_named_hook or those not defined at the top level of a module, but it is an improvement on the present situation.

[Martin Pool (mbp)]

That looks reasonable, thanks.

  vote approve

review: Approve

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/hooks.py'
--- bzrlib/hooks.py	2011-10-11 10:21:35 +0000
+++ bzrlib/hooks.py	2011-11-28 19:09:26 +0000
@@ -127,6 +127,7 @@
         """
         dict.__init__(self)
         self._callable_names = {}
+        self._lazy_callable_names = {}
         self._module = module
         self._member_name = member_name
 
@@ -196,7 +197,15 @@
         the code names are rarely meaningful for end users and this is not
         intended for debugging.
         """
-        return self._callable_names.get(a_callable, "No hook name")
+        name = self._callable_names.get(a_callable, None)
+        if name is None and a_callable is not None:
+            name = self._lazy_callable_names.get((a_callable.__module__,
+                                                  a_callable.__name__),
+                                                 None)
+        if name is None:
+            return 'No hook name'
+        return name
+
 
     def install_named_hook_lazy(self, hook_name, callable_module,
         callable_member, name):
@@ -221,6 +230,8 @@
                 self)
         else:
             hook_lazy(callable_module, callable_member, name)
+        if name is not None:
+            self.name_hook_lazy(callable_module, callable_member, name)
 
     def install_named_hook(self, hook_name, a_callable, name):
         """Install a_callable in to the hook hook_name, and label it name.
@@ -266,6 +277,10 @@
         """Associate name with a_callable to show users what is running."""
         self._callable_names[a_callable] = name
 
+    def name_hook_lazy(self, callable_module, callable_member, callable_name):
+        self._lazy_callable_names[(callable_module, callable_member)]= \
+            callable_name
+
 
 class HookPoint(object):
     """A single hook that clients can register to be called back when it fires.

=== modified file 'bzrlib/tests/blackbox/test_hooks.py'
--- bzrlib/tests/blackbox/test_hooks.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/blackbox/test_hooks.py	2011-11-28 19:09:26 +0000
@@ -19,6 +19,8 @@
 from bzrlib.branch import Branch
 from bzrlib.tests import TestCaseWithTransport
 
+def _foo_hook():
+    pass
 
 class TestHooks(TestCaseWithTransport):
 
@@ -51,3 +53,23 @@
 
     def test_hooks_no_branch(self):
         self.run_bzr('hooks')
+
+    def test_hooks_lazy_with_unnamed_hook(self):
+        self.make_branch('.')
+        def foo(): return
+        Branch.hooks.install_named_hook_lazy('set_rh',
+            'bzrlib.tests.blackbox.test_hooks',
+            '_foo_hook',
+            None)
+        out, err = self.run_bzr('hooks')
+        self._check_hooks_output(out, {'set_rh': ["No hook name"]})
+        
+    def test_hooks_lazy_with_named_hook(self):
+        self.make_branch('.')
+        def foo(): return
+        Branch.hooks.install_named_hook_lazy('set_rh',
+            'bzrlib.tests.blackbox.test_hooks',
+            '_foo_hook',
+            'hook has a name')
+        out, err = self.run_bzr('hooks')
+        self._check_hooks_output(out, {'set_rh': ["hook has a name"]})

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-11-28 15:59:39 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-11-28 19:09:26 +0000
@@ -69,6 +69,9 @@
   and gives a better error message if the time zone offset is not given.
   (Matt Giuca, #892657)
 
+* Provide names for lazily registered hooks. 
+  (Neil Martinsen-Burrell, #894609)
+
 * Resolve regression from colocated branch path handling, by ensuring that
   unreserved characters are unquoted in URLs. (Martin Packman, #842223)