A.L.E.C wrote:

I think it would be good start for plugins API to get rid of hardcoded actions from index.php.

Absolutely!

My proposition is to create file for each action (filename = actionname) in tasks directories, and then we have:

foreach(array('plugins', 'steps') as $dir) if (file_exists('program/'.$dir.'/'.$RCMAIL->task.'/'.$RCMAIL->action.'.inc')) { @include_once('program/'.$dir.'/'.$RCMAIL->task.'/_init.inc'); // init actions (renamed func.inc)

@include_once('program/'.$dir.'/'.$RCMAIL->task.'/'.$RCMAIL->action.'.inc'); @include_once('program/'.$dir.'/'.$RCMAIL->task.'/_destroy.inc'); // post actions break; }

plugins directory it's just for possibility to overwrite built-in action with plugin action.

This works fine as long as you validate the $RCMAIL->task variable and it only works for a single plugin.

And instead of focusing on files I'd recommend creating a plugin directory that contains a well-defined "init.php" file which then registers calls to plugin handlers (that's how Squirrelmail does it - and it works quite well). Another way is to override classes, like Typo3 does, but this has the drawback that some constellations don't work together.

My recommendation would be something like this:

/myplugin/init.php: init_plugin('init', 'functionname'); init_plugin('showmail', 'functionname');

That way the plugin does not have to provide empty files - and using the @ is not a good coding practice - instead do a file_exists!

My 2 cents.

Mike

On Mon, Jun 9, 2008 at 12:43 PM, A.L.E.C alec@alec.pl wrote:

I think it would be good start for plugins API to get rid of hardcoded actions from index.php. So, we have such code:

// include task specific files if ($RCMAIL->task=='mail') { include_once('program/steps/mail/func.inc');

if ($RCMAIL->action=='show' || $RCMAIL->action=='preview' || $RCMAIL->action=='print') include('program/steps/mail/show.inc');

...

My proposition is to create file for each action (filename = actionname) in tasks directories, and then we have:

foreach(array('plugins', 'steps') as $dir) if (file_exists('program/'.$dir.'/'.$RCMAIL->task.'/'.$RCMAIL->action.'.inc')) { @include_once('program/'.$dir.'/'.$RCMAIL->task.'/_init.inc'); // init actions (renamed func.inc)

@include_once('program/'.$dir.'/'.$RCMAIL->task.'/'.$RCMAIL->action.'.inc'); @include_once('program/'.$dir.'/'.$RCMAIL->task.'/_destroy.inc'); // post actions break; }

Maybe we can work on a required set of "methods" and calls for all plugins to get rid off those [file, function]_exists() things. We could also register available methods/calls via an ini file (of course parsing of those should be cached) which exposes them.

I am all for dropping in plugins and running them - on the same side this very dynamic context also eats away on resources, so maybe we should have a plugin.ini after all which initializes them or lets roundcube know what is available.

I'd also like to seperate "core" feature set and plugins. Not everything (by definition)is a plugin, even if they are handled the same.

RE: hooks - interesting idea, I think that would make sense. We can utilize jquery to perform the frontend things for that. We'd probably need an "ajax" endpoint so jquery can give the endpoint a context (e.g. reading mail, writing mail, browsing addressbook, browsing folder, ...) and in return get available plugins for the hooks that are available in this context.

Also check out the wiki for some notes that Brennan and I took on a plugin api.

Till _______________________________________________ List info: http://lists.roundcube.net/dev/

On Mon, Jun 9, 2008 at 2:21 PM, Michael Baierl mail@mbaierl.com wrote:

till wrote:

...

Maybe we can work on a required set of "methods" and calls for all plugins to get rid off those [file, function]_exists() things. We

Still you need them as RCD should not fail in case something is missing. And it does not make sense to force the plugin developer to have to create a long template with all callbacks. You will always end up with such _exists() methods if you upgrade your Plugin API - you don't want the plugins to fail...

I don't see why we cannot demand plugins to come with a clean api. I am not talking about 30 methods, more like 4 or 5 that have to be there. And if they are empty, I am personally willing to waste those extra bytes. ;-)

In the end, we'll have to support all that stuff people come up with. We should plan on enforcing relatively strict guidelines on plugins. Not to offend anyone, but most projects don't do this and you end up with a large repository but the quality of the majority of the components is poor, they won't work after an major PHP upgrade, have no documentation, are not updated in case of a bug/exploit and so on.

So, I'd rather 10 plugins that are well done, then 30 which are not. :-)

...

could also register available methods/calls via an ini file (of course parsing of those should be cached) which exposes them.

Why INI? Use a PHP file instead. And still it will be slower then using function names and function_exists calls.

Why not? It's relatively clean, doesn't require deeper code knowledge to edit ("Why do I need to escape this string inside this definition?"). The trade-off here is speed, of course. Still, the parsing can be cached.

...

I am all for dropping in plugins and running them - on the same side this very dynamic context also eats away on resources, so maybe we should have a plugin.ini after all which initializes them or lets roundcube know what is available.

I've already coded some plugin system and did not see any performance hit. I know Typo3 and know that it can eat up a lot of memory in case of a lot of extensions are loaded, but still the plugin overhead itself is relatively small. If you care about file_stat calls (_exists) - just use one init.php per plugin which then loads all other required files.

Maybe you are right, but we already have lots of issues with memory_limit and so on. Yeah, we do have some issues with composing and parsing and the way the libs we use handle objects and (not) worry about their memory footprint. Yet, people also complain that RoundCube won't run with 12 MB RAM, etc.. I can see the next discussion.

I am sure that even the dynamic context [could, should] be cached as well. I just don't want RoundCube to take 20 secs to pull up a login form. ;-) In the end, imap is already not the fastest kid on the block.

Personally, I like to have things expicit - configuring things requires that you go into a config file at least once and enable it, and you have to go in to disable it. We could also extend that some time with an interface for the postmaster@ account (just brainstorming here ;-)).

Cheers, Till _______________________________________________ List info: http://lists.roundcube.net/dev/

On Mon, Jun 9, 2008 at 2:46 PM, Алексей Михеев amiheev@st-host.ru wrote:

On Mon, 09 Jun 2008 14:21:38 +0200 Michael Baierl mail@mbaierl.com wrote:

...

till wrote:

...

Maybe we can work on a required set of "methods" and calls for all plugins to get rid off those [file, function]_exists() things. We

Still you need them as RCD should not fail in case something is missing. And it does not make sense to force the plugin developer to have to create a long template with all callbacks. You will always end up with such _exists() methods if you upgrade your Plugin API - you don't want the plugins to fail...

Why would you ever need those nasty *_exists() and callbacks, why not create a base Plugin class and inherit actual plugins from it and let them provide all additional info (up to dependency tracking) by themselves? It's PHP5!

Exactly, write an abstract, or an interface, implement/extend. Done. Clean api. :-)

Thanks.

Till _______________________________________________ List info: http://lists.roundcube.net/dev/

A.L.E.C wrote:

I think it would be good start for plugins API to get rid of hardcoded actions from index.php. So, we have such code:

// include task specific files if ($RCMAIL->task=='mail') { include_once('program/steps/mail/func.inc');

if ($RCMAIL->action=='show' || $RCMAIL->action=='preview' || $RCMAIL->action=='print') include('program/steps/mail/show.inc');

Well, the hard-coded includes are not very nice and could be replaced by a controller class or just an array mapping tasks/actions to a certain file.

I don't like the approach to use request parameters like $RCMAIL->task and $RCMAIL->action directly in include paths. This could open some security vulnerabilities. Even file_exists() will succeed if you want to include some password files.

I'd suggest to use an object oriented approach to define and register plugins. How about this:

class my_rcube_plugin extends rcube_plugin {

function __construct() { $this->add_hook('login', array($this, 'mlogin')); $this->register_action('foo', array($this, 'myfoo')); $this->add_toolbar_button('compose', ...); $this->add_folder_item(...); }

function mylogin($p) { $p['username'] = 'foo'; $p['password'] = 'bar'; return $p; }

function myfoo() {

}

}

The base class rcube_plugin will implement all the internal functions and add the registered actions and hooks to the according controller.

Each plugin should have it's separate folder within the plugin directory and we define a fixed name (eg. init.php) where the class name and more details (like the task where it belongs to) are defined. If we require the class file to be named as the class (in our case it would be my_rcube_plugin.php) it will work with our auto-loader. For performance reasons we can cache the contents of all init.php files to prevent from traversing the whole plugin directory on each request.

I don't see a reason why the init-file shouldn't be written in PHP since people who write a plugin have to do this in PHP as well and thus have basic knowledge about this language.

~Thomas _______________________________________________ List info: http://lists.roundcube.net/dev/