Re: [Geany-Devel] New plugin loader mechanisms
Am 29.03.2015 um 18:19 schrieb Colomban Wendling: 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. Okay, I can add the return value now. But can we agree on ignoring the return value for now and work out what Geany should do best in a separate thread/changeset. I just don't like to mix this UI/UX change (which is an effort on its own because Geany obviously does not handle failing init() at all) with the new loader. But with GError sounds a bit overkill to me, isn't gboolean or gint return code sufficient? Best regards. ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
Am 30.03.2015 um 23:07 schrieb Thomas Martitz: Am 30.03.2015 um 14:57 schrieb Colomban Wendling: If `pdata` is provided in geany_plugin_register(), how does it get released? If it has to be a pointer to static data it's a bit limiting, and forces use of (static) globals, which is one thing that this new API tries to avoid, doesn't it? We could also ask for a GDestroyNotify, but that's an additional arg we might not need. And also, providing the pdata in geany_load_module() might mean memory allocation, allocation that might never be used if the plugin isn't activated. OTOH, if `pdata` is a member of GeanyPlugin, it can be allocated in init() and freed in cleanup() without having to tell Geany how it has to free it. I see what you mean. Indeed, this could be a problem. I wanted the pdata parameter such that it is friendly to language bindings. This doesn't work if the user_data is hidden down in some other structs passed as parameter. For example, I wanted to make it possible to use vala member functions classes directly as plugin hooks. And this works, however there is indeed a leak. I have a prototype, see below. Considering this use case I would rather take the GDestroyNotify than hiding down pdata in another param. Passing it to geany_plugin_register() also allows for using a member function for the init() hook already. What do you think? Replying to self after having thought about it a bit more. Since I do want to maintain the ability to set the data in geany_load_module(), or rather just before the plugin's init() (to enable the use case cited below), but also at the same time still allow it to be also set in the plugin's init() (some plugins don't want expensive allocs before init()) AND definitely pass the pointer as a separate parameter, the only think I came up with is a separate API function to set the data, including the destroy notify. So I have now: geany_plugin_register(GeanyPlugin *, gint api, gint min_api, gint abi, GeanyPluginHooks *); /* pdata param is removed */ geany_plugin_set_data(GeanyPlugin *, gpointer pdata, GDestroyNotify free_func); geany_plugin_set_data() can be called before the plugin's init, or within, or any later. It'll set the data that is passed to all the hooks (NULL is passed if geany_plugin_set_data() was never called). The GDestroyNotify is called before the plugin is unloaded. Most importantly it is also called if geany_plugin_set_data() was called before init() and the plugin did not became activated at all. I have successfully prototyped that the below Vala example can work without leaks using the separate API function: -p.register(Geany.API_VERSION, 224, Geany.ABI_VERSION, -ref hooks, new Tester() /* LEAK */); +p.register(Geany.API_VERSION, 224, Geany.ABI_VERSION, ref hooks); +p.set_data(new Tester() /* no leak, g_object_unref is automatically passed as destroy notify */); Is that sound? Best regards Appendix: The prototype is like this: using Geany; class Tester { public void init(Plugin p) { } public void help(Plugin p) { /* shows a dialog */ } public void cleanup(Plugin p) { } } private PluginHooks hooks; public bool geany_load_module(Plugin p, GLib.Module mod, int geany_api_ver) { hooks.init = Tester.init; hooks.help = Tester.help; hooks.cleanup = Tester.cleanup; p.register(Geany.API_VERSION, 224, Geany.ABI_VERSION, ref hooks, new Tester() /* LEAK */); mod.make_resident(); /* ... */ return true; } ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] My non-C plugin roadmap
Am 30.03.2015 um 15:48 schrieb Colomban Wendling: Le 30/03/2015 00:17, Thomas Martitz a écrit : Am 29.03.2015 um 19:17 schrieb Colomban Wendling: Le 29/03/2015 00:23, Thomas Martitz a écrit : - New API functions to allow plugins to act as proxy plugins (pluxies). […] That's the part I'm really fuzzy about. I really don't see why we need this specific layer […] […] As with git master, Geany's core loader scans the plugin folder on startup and on opening the PM dialog. For each recognized plugin file it allocates a GeanyPluginPrivate and calls geany_load_module(). […] with my new loader (no pluxies) it goes like this, and this is *very* similar to git master. […] OK, fair enough indeed. And well, proxy plugins are special enough to warrant their own API if it's useful anyway, so okay. Yea, since the plan is to integrate the sub-plugins in the same UI as standard plugins, *some* kind of API is needed anyway. And I think my approach gives great flexibility to the pluxies while maintaining Geany (and the user) as the "supervisor". And this by only adding a single API function. Now, with pluxies, it is completely the same except for: 2* for each $file in $path, Geany calls is_plugin($file) which matches additional file extensions (as provided by pluxies), it also calls the probe() hook to resolve ambiguous files (e.g. .so files, they can be core or libpeas plugins) As raised on IRC, one small question: do we need a file extension if we have probe()? I don't mind much, but I would imagine probe() could filter extensions itself and simply return the appropriate value. This would also potentially allow for extensionless plugins. But that's a small detail, and apart feeling it a little redundant I don't mind either way. I think it's useful for a number of reasons: - It's less duplicated code in the pluxies because virtually all of them need to have one or more file extension checks (even if it's just less than 10 lines) - We could potentially give the user a more unified UI that sums up loaded pluxies - Knowing the file extension in the core could become handy in the feature, like if we ever want to auto-activate pluxies for certain extensions (e.g. for pluxies that we ship with Geany itself), or reporting conflicting pluxies to the user. We wouldn't have to introduce a new API then - Although very minor: we can safe the indirect function call into the pluxy if we already know the negative result We could support extension-less sub-plugins either way (e.g. by treating the ""/NUL extension specially) so we're not limiting ourself. However it's true that it's not *strictly* needed. I hope you better understand my concept now. […] Yep, I do, thanks for these very good clarifications :) You're welcome :) Best regards ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel