On 11/4/05, Timothy Appnel <[EMAIL PROTECTED]> wrote:
> I've chatted with Mark offline about what it would take to merge
> Class::Trigger and the callback functionality in CGI::App. Before I
> start submitting and code patches to that end I wanted to run what I
> was thinking by the list.

Before writing the callback patches for CGIApp, I actually looked at
Class::Trigger to see if it would do what I wanted.  There were two
main reasons why I didn't go with it:

- Class::Trigger didn't do everything we needed (like you list below)
- The functionality we needed was quite trivial and I thought that it would be
less impact on CGIApp to write it myself (proposing a patch that
creates a new dependancy on the code makes it much harder to get your
patch accepted).

However, if we can get the missing functionality into Class::Trigger,
then I would be all for replacing the callbacks with Class::Trigger. 
I like code reuse, and use tonnes of modules in my own apps, so one
more dependancy doen't bother me.

> * Methods names. We say callback they say trigger.
>
> The method signatures are the same though so this can easily be
> remedied in CGI::App with an alias.
>
> * Hooks in CGI::App are case insensitive while Class::Trigger are case
> sensitive.
>
> I can suggest this change. This could also be remedied by overriding
> logic if necessary otherwise.
>
> * add_callback will accept a scalar that maps to a method name while
> add_trigger accepts code references only.
>
> Once again I can suggest this change, however it can be just as easily
> implemented in CA with a bit of overriding.

This was originally done because of the way runmodes were implemented.
 It makes it possible to override callback functions in subclasses
(which is necesary for the old style 'cgiapp_prerun' type methods).

> * Hook registration.
>
> Class::Trigger only explicitly supports hooks being added when
> Class::Trigger's use is declared. CGI::App's new_hook method allows
> for hooks to be declared (essentially) at any given time.
>
> Rather then add a method to Class::Trigger I would propose that the
> new_hook method in CGI::App handle working with Class::Trigger's class
> data in declaring new hooks. Or should I propose a new_trigger method
> be added to Class::Trigger? I could go either way here really.

This was a big reason for me not using Class::Trigger in the first
place.  With a standard CGIApp application, there are usually several
modules involved, and in a plugin, it doesn't make much sense to place
a 'use Class::Trigger' in the plugin to create new hooks.  Especially
since those hooks need to be part of the CGIApp application module,
and inherited by its subclasses.

> * Triggers/callbacks fetching.
>
> This is the most substantial difference. Class::Trigger fetches either
> object OR that object's class. CGI::Application fetches the object AND
> the entire class hierarchy of that object.

This is another necesary feature (IMHO).  Having callbacks/triggers
that are inherited is a very powerful feature of the plugin system.

> This logic can be easily added of course, but inserting that logic
> without penalizing other Class::Trigger users isn't as straight
> forward. What I'm considering is a "deep" fetch method that could be
> optionally run to execute triggers/callbacks as CGI::App does with a
> boolean flag. Since Class::Trigger is a mix-in  I'm thinking the way
> to go with that boolean flag is as a class::data accessor. In
> Class::Trigger that would like something like....
>
> sub __fetch_triggers {
>     my $proto = shift;
>     return ((ref $proto and $proto->{__triggers}) ||
>         $proto->__triggers) unless $proto->__deep_exec;
>     # __deep_exec would default to false (undefined)
>     # CGI::App like deep execution code here.
>     # walk object and class hierarchy assembling the
>     # return array.
> }
>
> Thoughts on this?

Something like that will probably make it easier for the owner of
Class::Trigger to accept the patch.  One thing that might be
interesting here is to provide a 'stop class'.  In other words, what
if $proto->__deep_exec return a class name, and the search only goes
up the class hierarchy until it reaches that class.  Then you could
give 'UNIVERSAL' if you wanted all classes.

If you can convince the owner of Class::Trigger to make all the
changes we would require, then I am all for the changes to
CGI::Application.

Cheers,

Cees

ps One more thing that we haven't revisited yet (and is also not
supported in Class::Trigger either) is callback ordering.  We use a
FIFO (first in first out) approach, since we could not agree on (or
come up with) a good ordering mechanism.  I still think it might be
nice to have the ability to control the execution order of all the
callbacks registered at one hook location (even though I haven't
absolutely needed it yet).

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/[email protected]/
              http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to