[MERGE] Add Hooks.install_named_hook
Aaron Bentley
aaron at aaronbentley.com
Sun Mar 9 20:42:40 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Daniel Mark Watkins wrote:
> Fixing another of Alexander's recent quibbles with hooks, this adds the
> Hooks.install_named_hook method, which does exactly what
> Hooks.install_hook does, but requires a 'name' parameter which
> describes the given callable.
>
> As far as I see it, there's no reason not to provide a name for a hook
> when installing it (and doing so is certainly preferable), so
> deprecating the old method seemed like a reasonable thing to do.
bb:resubmit
I agree with the rationale. But this patch introduces code duplication.
Why not have install_hook call install_named_hook with a None name?
Also, your exception handling scope could be tighter here:
> + try:
> + self[hook_name].append(a_callable)
> + if name is not None:
> + self.name_hook(a_callable, name)
> + except KeyError:
> + raise errors.UnknownHook(self.__class__.__name__, hook_name)
It's admittedly unlikely that self.name_hook would raise KeyError, but
if it did, this code would incorrectly raise UnknownHook.
In this case, you can change it to
try:
self[hook_name].append(a_callable)
except KeyError:
raise errors.UnknownHook(self.__class__.__name__, hook_name)
if name is not None:
self.name_hook(a_callable, name)
In other cases, you might want to do:
try:
self[hook_name].append(a_callable)
except KeyError:
raise errors.UnknownHook(self.__class__.__name__, hook_name)
else:
if name is not None:
self.name_hook(a_callable, name)
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFH1EvA0F+nu1YWqI0RAi1EAJ0R8kCLxWDxVTVb5nhSlx2a0esplgCfYw2M
25luf0otiSLoggit/gDtxhQ=
=bD9p
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list