Hi Owen, and in fact yours is a very good reasoning. I was just pointing that currently, all implementations of action_plugin_ui() in 'extras' are just what now is expected to be on action_plugin_ui_any(). So as long as all extras are implementing this action to change the ui (its own, or other's extras ui), to avoid conflict it makes more sense to me to leave this method as is, and include a 'custom' only self action. I used ID, but I was not thinking in uid at all, it could be the extra name, or whatever you want to.
If you are going to change this behavior (considering a new action '_any' affecting all plugins) then it could be also a good idea to think about the action's name. I don't mean to start an end-less discussion about it, but 'action_plugin_ui' is not exactly what I was expecting the first time I saw this action, so I'm not surprised that you said: "New plugin devs are usually surprised to find out this is not the case", in fact, the first surprise is that plugin_ui is only interacting with current's adminhandler (that seems not to be easy to replace) to handle a part of the admin interface, not all the plugin's user interfaces, so unless you already know Habari's internals, it is very dificult to notive that plugin_ui is only a part of the plugin UI, and only to be used in a specific part of the administration section. Just 2 cents, good critic however, I like Habari. Thanks Owen, I'm pretty sure that new commit has some performance improvements that you didn't mention and I'd like to bump for others. On Tue, Nov 2, 2010 at 4:13 AM, Owen Winkler <[email protected]> wrote: > On 11/1/2010 3:36 AM, Iñaki Lopez wrote: > >> Hi Owen a very good improvement, but still does not make sense why on >> one case you are using 'any' and in the other you are using a more >> specific approach. >> >> I'd be in favour or having two different apis (and this should also >> leave things working without breaking anything) in the form: >> >> // The usual one. >> public function action_plugin_ui( $plugin_id, $action ) { >> >> } >> >> And I'd have included the >> >> public function action_plugin_ID_ui($action) { >> } >> > > There's no need to include "ID" in the function name. You've already > defined a class for your plugin, and that should be enough to declare that > the method belongs only to that plugin. > > Also, what would "ID" consist of? Plugin ids are usually generated by > Habari and are only used for comparison; There is no guarantee that the > plugin id will be any particular string from install to install. Adding > some other parameter for the plugin to pull from (like in the info/xml, or > some other function) just makes it more complicated than it was already, or > at best, makes it no easier than it originally was when you had to compare > the plugin ids yourself. > > Adding "any" as the hook that responds to any plugin_ui call makes sense > because it explicitly states that it will respond to any request. > > As I mentioned in my original post, the expected behavior when observing > the the function in use is that the non-any hook will respond only for that > plugin. New plugin devs are usually surprised to find out this is not the > case. With this commit, we're simply enforcing what people already expect > to be true, what results by default in the least amount of code, and what > makes the non-standard behavior explicit. I think that's pretty good > reasoning, altogether. > > Thanks for your feedback. > > > Owen > > -- > To post to this group, send email to [email protected] > To unsubscribe from this group, send email to > [email protected] > For more options, visit this group at > http://groups.google.com/group/habari-dev > -- To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/habari-dev
