[Geany-Devel] New plugin loader mechanisms

[Colomban Wendling]

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