Florian Lagg wrote:
> Why I implemented it this way:
> * there is no need for configuration in RC
> * there is no need for an extra hook registration in the plugins
> * because you may have a better way to do it?
>
> You will find the patch-file and a complete 7zip file including the whole
> roundcubemail folder here:
> http://www.lagg.at/temp
> The patch file is also included in this mail.
>
> As I am not the greatest php programmer it would be great if someone can
> review it.
- $this->prop['plugin_dir'] = $this->prop['plugin_dir'] ?
unslashify($this->prop['temp_dir']) : INSTALL_PATH . 'plugins';
+ $this->prop['plugin_dir'] = $this->prop['plugin_dir'] ?
unslashify($this->prop['plugin_dir']) : INSTALL_PATH . 'plugins';
The basic idea is good but here are some suggestions:
1) The plugin class, once instantiated, should register it's hooks at the
plugins repository (rcube_plugins class). No implicit calling of an object
method. Otherwise hook names are restricted to naming conventions.
2) Same as in 1) for the rcube_plugins class. The application should
trigger a plugin-hook by calling one single method (like 'trigger_hook()')
and pass the name of the hook as the first argument.
3) Hooks need data and (maybe) return modified data. Therefore I suggest to
pack all data of a plugin-hook into a single argument which is an array
(or object). The plugin method then returns the complete array (object)
with modified data. Triggering a hook would then look like this:
$credentials = $plugins->trigger_hook('before-login',
array('user' => $user, 'pass' => $pwd));
4) Please read the http://trac.roundcube.net/wiki/Dev_Guidelines,
especially the naming conventions.
We also have a wiki on http://trac.roundcube.net. Feel free to create pages
and upload patches there.
Thanks for your work so far!
Thomas
_______________________________________________
List info: http://lists.roundcube.net/dev/
