[MERGE] Allow self documenting hooks.

Robert Collins robert.collins at canonical.com
Tue Mar 10 02:20:33 GMT 2009


On Tue, 2009-03-10 at 11:54 +1000, Martin Pool wrote:
> +    * Hooks can now be self documenting. ``bzrlib.hooks.Hooks.create_hook``
> +      is the entry point for this feature. (Robert Collins)
> 
> Having different classes called Foo and Foos is a bad idea because it
> makes it hard to talk about the two of them precisely.  That's
> probably better handled by renaming Hooks to HookSet or something.

Or we could call Hook Hookable or something like that.

> +                strings.append("An old-style hook. For documentation
> see the __init__ "
> +                    "method of '%s'\n" % (self.__class__.__name__,))
> 
> Why not just insert it?

Insert what?

> +    def hook(self, callback, callback_label):
> +        """Call this hook with callback, using callback_label to describe it.
> + 
> +        :param callback: The callable to use when this Hook fires.
> +        :param callback_label: A label to show in the UI while this callback is
> +            processing.
> +        """
> +        self._callbacks.append(callback)
> +        self._callback_names[callback] = callback_label
> +
> 
> I think the docstring is wrong: this doesn't actually call the hook,
> it rather registers the callback to be fired.

Oh the first line? Sure, what about
"Register a callback to be called when this <class> fires."

> +        strings.append("<bzrlib.hooks.Hook(")
> 
> It should be self.__class__.__name__ to allow it to be subclassed.

This is a weak point in python really, as self.__class__.__name__ is
'Hook', so we need quite a bit of fudging to get the same string :(.

I can do that, but I would expect folk subclassing to also change repr
regardless - because they need to show their additional data.

-Rob

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090310/67410910/attachment.pgp 


More information about the bazaar mailing list