Hi,
I'm answering both to the initial mail and this one here, so expect mixed citations. This will avoid raising the same points twice :)
First, stuff extracted from the first mail. Disclaimer: Sorry if some remarks look slightly off-formulated, but while I altered them in the light of the last changes, I didn't have the courage of rewrite all sentences from the ground up when they were still (mostly) relevant.
gboolean geany_plugin_register(GeanyPlugin *, gint api, gint abi, PluginHooks *(see below), gpointer)
The plugin defines a single global function, geany_load_module(GeanyPlugin *, GModule *). This is the only function that geany learns about using g_module_symbol(). And the only thing this function ought to do is to call geany_plugin_register(). This does 4 things
- Provide the plugin handle to the plugin very early
- Perform abi and abi checks, so this is finally done inside geany.
Added bonus is that geany knows the requested api and can possibly apply backcompat workarounds on a per plugin basis (instead of globally), warn about very old plugins or even deny loading them. 3) Register the remaining hooks and callbacks (see below) 4) Associate a userdata pointer to the plugin, geany will pass this pointer back to future calls into the plugin (good for proxies)
In the future geany_plugin_register should be able to be used to register plugins recursivly, by passing the appropriate GeanyPlugin pointer, i.e. plugin A should be able to call geany_plugin_register(plugin_B, ...) to realize pluxies.
How does the proxy plugin create plugin_B? plugin_A is given by Geany, but how is plugin_B created?
Actually, when I started reading the mail I thought you'd have something like this (which is basically like Matthew's proposal):
``` struct RealPlugin /* good name would be better */ { /* we may need this indeed, set by register_plugin() */ GeanyData *geany_data; /* plugin metadata */ const gchar *name; const gchar *author; const gchar *version; /* ... */ /* plugin vfuncs */ gboolean (*init) (RealPlugin *self); void (*uninit) (RealPlugin *self); GtkWidget (*configure) (RealPlugin *self, GtkDialog *dlg); /* ... */ }; ```
and that a plugin's geany_load_module() would do something like
``` geany_load_module(...) { static struct RealPlugin self = { N_("foo"), N_("john doe"), "42", ... my_plugin_init, my_plugin_uninit, my_plugin_configure, ... }; geany_plugin_register(... &self ...); } ```
e.g. give Geany a structure representing the plugin, and that's all. Note that with this apporach, you can even add "class-style" custom data to a plugin simply by having a struct larger than RealPlugin:
``` struct MyRealPlugin { RealPlugin parent; /* offset 0 has the Geany struct, so it's binary compatible */ /* plugin-specific fields here */ int the_game; }; ```
Maybe proxy plugins need a data argument, but maybe not: they should be able to pack whatever they need in the struct they register for the "sub"-plugins. Not extending the structure from plugins might make easier extending the structure on our side, without needing padding though.
But anyway whether it's an extra data* argument (or member) or an extended struct doesn't change much my point, which would be that all the plugin description would be given to Geany as one single thing.
With your new proposal that gets rid of set_info() the "how does a proxied plugin give its info" (mostly) moot, but there still is the question why having separate argument holding pointless stuff (GeanyPlugin) while all could be packed in a new and better structure.
E.g. my point could maybe be summarized with "why split things up while they actually represent the same thing?".
And BTW, remember that we need the (translated) plugin infos in all cases, so it should be no difference giving it straight ahead -- even proxied plugins.
[…] Additionally the existing PluginCallbacks pointer registered here for all plugins that want statically defined signal handlers.
I'm really not sure if we should bother keeping this. Is there any benefit over plugin_signal_connect()?
[…] Please also note that each hook receives the GeanyPlugin pointer and the userdata pointer, these are the same passed to geany_plugin_register.
Why pass the GeanyPlugin? I mean, what useful info is in this? there is the PluginInfo, which seem hardly useful, and the GeanyPluginPrivate, which is totally opaque, so totally useless. OK you add the GeanyData too which is useful (but could be part of the RealPlugin thing just as well).
Again, here I'd have expected something like my_plugin_init(self, ...), as 1) it follows the common "pass the self/this as first parameter" scheme, and it's actually the only thing that seem to matter. Extra args are of course useful when they are needed, like GtkDialog* for configure.
With this new mechanism, proxy plugins are more than doable. geanypy should benifit as well.
Well, with your proposal, I don't get how a proxy plugins will actually register the plugins it proxies.
I would have imagined a proxy plugin would simply do something like this:
``` struct ProxyPlugin { RealPlugin parent; GList *plugins; }
struct SubPlugin { RealPlugin parent; ProxyPlugin *proxy; /* the proxy parent, if needed */ /* ... whatever state for each proxied plugin */ InterpreterData *data; }
static gboolean sub_init(SubPlugin *sub) { /* whatever to actually start the proxied plugi and alike */ interpreter_init_or_something(sub->data) interpreter_call(sub->data, "plugin_init"); }
static void sub_uninit(SubPlugin *sub) { /* stuff to shut the sub-plugin down */ interpeter_uninint(sub->data); }
proxy_plugin_init(ProxyPlugin *self) { self->plugins = NULL; foreach (proxied_plugins as pp) { struct SubPlugin *sub = g_malloc0(sizeof *sub); sub->init = sub_init; sub->uninit = sub_uninit; ...
geany_plugin_register(sub); self->plugins = g_list_append(self->plugins, sub); } }
proxy_plugin_uninit(ProxyPlugin *self) { foreach (self->plugins as sub) { geany_plugin_unregister(sub); free_sub_plugin(sub); } g_list_free(self->plugins); }
geany_load_module(...) { static struct ProxyPlugin self = { ... proxy_plugin_init, proxy_plugin_uninit, ... .list = NULL };
geany_plugin_register(&self); } ```
or something around these lines. But with the current status, I can't seem to understand how the proxy plugin could forge things like GeanyPlugin and the like?
Le 28/03/2015 23:56, Thomas Martitz a écrit :
[…] geany_load_module(), which is called by Geany via pointer obtained from g_module_symbol(). In this function the plugin should perform the following steps:
- Test against the passed geany_api_version if it can function under the
current Geany.
- Test for any additional runtime dependencies
- Call geany_plugin_register() appropate and test the return value
If any of the above steps fail FALSE should be returned, else TRUE.
Why return gboolean? is this just meant to allow geany to say the current "incompatible plugin, pleas rebuild"? If so, shouldn't this rather be a result of a failing geany_plugin_register()?
I don't get why geany_load_module() should fail, IIUC it should just not call geany_plugin_register() if it can't work, shouldn't it?
This proposal does not implement pluxies by itself, however I have done the work to implement pluxies on top of the new loader. I will post another mail about this when we finalized the new loader.
That might be where I don't get it. As I see it, the only missing bit from my slightly altered proposal is a geany_plugin_unregister() so a proxy plugin can remove its sub-plugins when it gets deactivated. Maybe I miss something, but that seem to be the only bit that would be required for a proxy plugin a normal plugin wouldn't need.
Regards, Colomban