Le 26/03/2015 00:16, Thomas Martitz a écrit :
Am 20.03.2015 um 19:45 schrieb Dimitar Zhekov:
Thinking about it, if the plugin can't run because it's missing resource files required for its operation, then I think it should be treaded like incompatible plugins.
There seem like two different things to me.
The first thing a loader needs to do is query each plugin for compatibility. Incompatible plugins should not be listed at all, and it'll be good if the loader emits status messages for them in the Status tab. As an alternative, they may be listed disabled, with a message why the module can't be used.
Now, do we really want the plugins to run arbitrary resource checking code, and display their own error messages, only because they are queried (or registered, as you put it), each time the list is build?
They [incompatible plugins and plugins that are unable to function due to missing runtime deps] are different things, but the resulting action should be the same (hide it from the PM dialog, possibly give the user a hint). Hiding from the PM dialog should IMO be done before it even showed up, to avoid creating expectations that cannot be met and negative user surprise.
Yes plugins should be able to run arbitrary checking code because that's necessary for proxy plugins. Whether Geany can provide APIs to report results to a the user in a consistent manner is on another piece of paper, one that's IMO slightly out of scope for my proposal.
I'm thinking if the plugin loaded successfully, then it should be operational too.
I can check if scope.glade exists, but is it valid XML, compatible with the current version of gtk_builder? The only way to be 100% sure is to actually load it. And them immediately unload it, because Scope is being queried only, not activated. And then load it again on init...
My opinion is that if the XML isn't compatible to the current GtkBuilder version then scope shouldn't even show up in the PM dialog, because there is no way it can be functional. This is not fundamentally different to a plugin being incompatible to Geany's ABI version. So yes, that implies validating the XML in geany_plugin_register() (although loading seems excessive to me, because the XML should be considered distributed with the plugin and not to be edited by the user; at some point you have to make some assumption to keep problems managable).
If this seems so inefficient you could try to rescue the state from geany_plugin_register() to init. However keep in mind that init can be called without prior call to geany_load_module(), if the user toggles plugins in the PM dialog. And that init might not be called at all, so plugins shouldn't be wasteful with resources in geany_load_module() if might never be needed.
But nobody can guarantee that it exists and is valid on init. What if the Plugin manager is open, scope.glade is removed or altered, and then "[ ] Scope Debugger" is checked? It's low probability, of course, but how am I supposed to react - just crash? If init remains void, then it would be no better than the current void plugin_init(), and I'll simply check anything in load - why bother, if I *still* need to manually disable scope on initialization failure?
What do you do currently?
Besides, I didn't mean to make init() any better than the current plugin_init() w.r.t. to its semantics. The only difference is more/other parameters. Geany does not handle failing init() currently, and I don't want to change that now because I think that's out of scope for my proposal.
Well, while I can understand not extending the scope of the changes, as it introduces *new* API, it's a good time to improve things without breaking any compatibility :)
[…] I also tend to think that a failing init is misdesigned.
What do others think about this point?
Well, while I agree a plugin shouldn't fail to initialize (when the user clicks on it), I tend to agree with Dimitar: there are some things that can fail, and that you can only know whether or not they do by actually trying. Checking many things when querying the module seem wrong to me, because if we start to make e.g. extensive I/O there, it gets slow for no reason. And it also seem wrong to run anything more than strictly needed when the plugin hasn't even been activated.
E.g., I don't want to bother about performance with 250 plugins installed beside how long Geany takes to scan the SOs.
And in the end, I agree that if init() can still fail to do its job (which will be the case, unless all failable initialization is done in load_module() only), then it's better to have a way to notify the user something failed, rather than pretending it worked but having a non-functional plugin.
All this said, a counter argument is that many plugins will load external UI for configure(), and there we have no way to fail either.
So anyway, I think I'd be in favor of a failable init(), maybe like
gboolean (*init) (..., GError **error);
if we want to be able to report messages and the like. Or just let each plugin do the same msgwing_add() or whatever, but a common integrated thing might be worth it.
Regards, Colomban