[dokuwiki] Re: events
- From: Ben Coburn <btcoburn@xxxxxxxxxxxxx>
- To: dokuwiki@xxxxxxxxxxxxx
- Date: Mon, 17 Apr 2006 13:21:42 -0700
On Apr 16, 2006, at 1:49 AM, Chris Smith wrote:
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.
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.
First let me restate how you intend this to work so that we both know
we are speaking about the same thing.
The event system provides one kind of event. This event provides the
same contract to event consumers (plugin authors) regardless of how the
event is initiated in the Dokuwiki code. The event can be initiated in
two ways. The advise method is called in sequence with the code block
that executes the (default) event. The trigger method is called in
place of the (default) event with a callback to the (default) event.
Now back to looking at the code.... The contract that the event system
provides to event consumers (plugin authors) is currently not
consistent. (This is largely why I had thought you intended there to be
two kinds of events.) Here are some differences between the current
behavior when advise() or trigger() are used to initiate events.
With advise:
- only <event name> is used
- may ignore preventDefault() without warning
With trigger:
- uses <event name>_before
- uses <event name>
- sometimes uses <event name>_after
- guarantees that preventDefault() is honored
From here on my comments are intended to produce a consistent event
system based on one event type as you suggest. I think I will keep
numbering points from where I left off, in case this makes the
discussion easier to follow....
4) Events are temporal. I think an <event name>_before and an <event
name>_after need to be sent (respectively) before and after the source
of each event. The event <event name> comes either before or after the
source of the event so it is the same as either <event name>_before or
<event name>_after. Therefor <event name> is redundant and should not
be used.
It is important to always send <event name>_before and <event
name>_after because order may matter to some plugins (event consumers)
-- even if there is no "result" value in <event name>_after. Take the
hypothetical example of 'PAGE_CREATED' and 'PAGE_DELETED' events that
provide only the namespace path and page name as event data. Some
plugins will want to lookup the page content while it still exists and
so will *need* to use 'PAGE_CREATED_after' and 'PAGE_DELETED_before'
events. Other plugins will rely only on the data provided in the event
and so can use either the <event name>_before or <event name>_after
events. The important point is that <event name>_before and <event
name>_after events always need to be available (initiated) respectively
before and after the source of the event even if most event consumers
(plugins) will only need one of them.
This is why I commented (in point #1) that advise should be
advise_before() and advise_after() so that it can be used in this
manner.
// example advise that honors preventDefault()
$evt = new event('EXAMPLE_A',$tmp=array());
if ($evt->advise_before(true)) {
/// code block of default event ///
}
$evt->advise_after();
// example advise that ignores preventDefault()
$evt = new event('EXAMPLE_B',$tmp=array());
$evt->advise_before();
/// code block of "default event" (may be empty) ///
$evt->advise_after();
5) Event consumers (plugins) need to be clearly told if
preventDefault() has any meaning for the event that they are receiving.
The advise_before() method should take an optional argument that sets
an event property such as 'canPreventDefault'. This property is set to
'true' when a default event exists and the code will honor
preventDefault(), otherwise it is set to 'false'. Here are the default
values I would propose:
advise_before( $enablePreventDefault = false )
Honoring preventDefault requires an extra if statement so this
value should have to be set explicitly to remind the programmer. See
the 'EXAMPLE_A' event code above.
advise_after( )
The preventDefault method has no meaning here so
'canPreventDefault' will be set to 'false' by this method.
trigger( $defaultCallback = NULL )
The trigger method internally honors preventDefault. The
'canPreventDefault' property of the event should only be set to 'false'
if there is no default event to prevent. (If defaultCallback is null
'canPreventDefault' will be set to 'false'. See point #6 below.)
6) The callback parameter used by trigger() should move from the
constructor [event()] to the trigger() method. Allowing an event that
is going to be initiated with the advise methods to have a callback
(_action) property is meaningless and just confuses the code.
This way
function event($name, &$data, $fn=NULL) and function trigger() becomes
function event($name, &$data) and function trigger($fn=NULL).
7) Miscellaneous. The 'event' and 'event_handler' classes should be
named with a "unique" prefix so that they are less likely to collide
with class in other event handling systems. The names 'Doku_Event' and
'Doku_Event_Handler' are probably reasonable. Event names should be
case insensitive, even if the convention is to name events in all
uppercase.
Hopefully this is a step closer to a consistent, cleanly defined, and
clearly executed event handling system for Dokuwiki.
Regards, Ben Coburn
-------------------
silicodon.net
-------------------
--
DokuWiki mailing list - more info at
http://wiki.splitbrain.org/wiki:mailinglist
Other related posts:
