[dokuwiki] Re: events

  • From: Chris Smith <chris@xxxxxxxxxxxxx>
  • To: dokuwiki@xxxxxxxxxxxxx
  • Date: Sun, 16 Apr 2006 09:49:04 +0100

Ben Coburn wrote:

I was already starting to think about an "action hooks" extension so I'm very glad you wrote this. I have had time to read through the code now, so here are some thoughts on it.

1) I think advise() should become advise_before() and advise_after(). Some plugins that rely on "advisory hooks" may need to know that they are running before or after the event that caused the "advisory hook". For example, assume there is an "advisory hook" that a page is deleted. Some plugins may need to access the page content *before* it is deleted, while others could run before or after the page is removed. This ensures that the plugin author knows exactly when the hook will be executed relative to it's event.

For clarification. A trigger event (currently) consists of three signals.

<event name>_before
<event name>
<event name>_after

The first two are guaranteed to happen before. I was thinking of dropping <event name>_before as it is duplicating <event name> and this would avoid confusion of mistakingly thinking <event name>_before could be used with an event when in fact it is not being sent.

2) How about adding a file (inc/events_list.php) to the source that is a big php comment documenting all the current hooks. Having a wiki page documenting the hooks is good but will it be in sync with the stable release, darcs, etc? Having a concise list of available hooks revisioned in darcs will allow programmers to know exactly what hooks are available in *any* version of the dokuwiki source that they happen to have. Example...
/*
name: EVENT_NAME
action: expanded description of the event that activates this hook
type: advise or trigger
data: description of data sent in the event
location: function name and source file from which the event is activated
comment: comments on possible usage, etc.
*/


I think a wiki page with this information should be enough. Having things in wiki also makes it much easier for others who don't have access to the codebase to improve the documentation. Having the information out of step in two places I don't think would be a good thing.
3) Advisory events need to be completely advisory. Advisory events should not allow plugins to use preventDefault(). Code that activates advisory events should not check _default afterwards... (That is described as a private variable in the object anyway!) The cleanest solution is to split the "event" class into an Advisory_Event class and a Trigger_Event class. This will help enforce the meaning of advisory and trigger events.

Actually they don't need to check _default - its the value returned by ->advise(). I've now altered the code for 'ACTION_TEMPLATE' to do this.

Maybe I have documented things incorrectly - too close to the implementation rather than the intention. The main purpose of advise() is to allow the event signalling code to manage the event itself. This may be easier than refactoring the calling script to make a trigger. Also where an event doesn't have a default action (or there are no meaningful results/effects from the default action) there is no sense in having an "_after" event.

You can sort of see that from the implementation of trigger(). It uses advise() to signal the events.

How about I redo the documentation and not claim two types of events.

"There are events. Events may have a default action. If they do have a default action, that can be prevented and where there are results from the action those results are passed to <event>_after handlers. If there is no default action or the default action does not produce any meaningful results, no<event>_after will be signalled. Check the documentation for the event to determine whether or not it signals an <event>_after."

The use of advise() or trigger() then becomes something for the author of the event to consider when implementing the event. It is immaterial to event handlers - which was my original intention. Add to that removal of <event>_before and the system should be less confusing too.

Finally, maybe it would make sense to rename advise(), signal().

Cheers,

Chris


-- DokuWiki mailing list - more info at http://wiki.splitbrain.org/wiki:mailinglist

Other related posts: