[PATCH 1 of 6 events v3] util: add event handling mechanism

[Mads Kiilerich]

On 09/28/2014 11:01 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1411933407 25200
> #      Sun Sep 28 12:43:27 2014 -0700
> # Node ID d513c232aeba16137b291456536adee0d88a040f
> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
> util: add event handling mechanism
>
> The patch adds a generic event handling mechanism to Mercurial. From a
> high level, you create a class with methods corresponding to event
> names. You can then register functions/callbacks against events that
> get called when that event fires.
>
> As will be demonstrated in subsequent patches, event handling can be
> considered an internal hooks mechanism and will provide a better
> alternative to wrapping or monkeypatching.
>
> The intent of the events system is to give extensions a more
> well-defined point for code insertion.

That sounds a bit like aspect-oriented programming, while events can be 
many other things. Why is not called AOP and why is it called events?

> Events will provide a better mechanism for code insertion. Events solve
> the discovery problem by providing a well-defined (like hooks) set of
> places for supported code insertion. Events are also easier to code,
> as extension authors don't need to worry about monkeypatching: just
> write a function and register it.

That might be true, but this patch series do not provide any evidence of 
what improvements it actually could bring ... or that it is a good 
mechanism for this kind of improvements. Actual use of this mechanism in 
the built-in extensions would help a lot to validate it. Without actual 
use it just adds quite a bit of complexity and no benefits.

> +class event(object):
> +    '''An event with its handlers.
> +
> +    An ``event`` is essentially a collection of functions that will be invoked
> +    when the event fires. ``event`` instances are typically created by defining
> +    methods on ``eventmanager`` instances.
> +
> +    Handler functions can be registered against an instance via the ``+=``
> +    operator. They can be unregistered via the ``-=`` operator.
> +
> +    Handler functions can be invoked by calling an ``event`` instance like
> +    it is a function.
> +
> +    Handlers are executed in the order they are registered.
> +
> +    The return value of handler functions is ignored.

I guess this imply that it should be "pure" functions that can't have 
side effects (or any effect at all?) unless the functions are curryed 
with some context? But they are allowed to raise exceptions? Is it 
accepted or frowned upon to modify the parameters?

Please consider saying something about this intended use here in the 
docstring. Actual use of it will of course also give some examples.

> +class eventmanager(object):
> +    '''A collection of events.
> +
> +    This class powers the internal events system. Instances of this class are
> +    typically attached to an object, but they can be standalone.
> +
> +    This class is an abstract base class. Actual event containers should
> +    subclass this class. Events are registered by defining methods on these
> +    subclasses. The methods should define the named arguments the event
> +    accepts and a docstring describing the purpose of the event. The custom
> +    __new__ implementation of this class will automagically convert each
> +    method to an ``event`` instance.
> +
> +    Methods lacking docstrings will result in an exception during class
> +    creation. This requirement serves to reinforce the well-defined intent
> +    of event APIs.

-1 to enforcing it just to enforce it. Mandatory docstrings is a policy 
- that should not be hardcoded.

> +    Arguments defined in the event/method definition are parsed for special
> +    meaning. If an argument shares a name with a named argument passed to the
> +    ``eventmanager`` constructor, the value passed to the ``eventmanager``
> +    constructor will be used as the default value for that argument in the
> +    event. In addition, if a default value is defined on the method/event,
> +    that default value will be used when calling events. Method-specific
> +    defaults override the "global" defaults from the ``eventmanager``
> +    constructor.
> +
> +    For example, say numerous events pass a repository instance into events.
> +    You can pass the repo as a named argument to the class constructor to
> +    save some typing at event call time. e.g.
> +
> +    >>> class repoevents(eventmanager):
> +    ...     def myevent(repo):
> +    ...         """Some repo event."""
> +    >>> def handler(repo):
> +    ...     print(repo['name'])
> +    >>> repo = {'name': 'my repo'}

I find the use of 'repo' in this example confusing. This dict is so far 
from what a 'repo' is. The example makes more sense to me if calling the 
dict 'data' or 'config' or having a different kind of 'context'.

/Mads