[MERGE] Add Hooks.install_named_hook

[Aaron Bentley]

-----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-----