Re: [Geany-Devel] New plugin loader mechanisms

2015-04-04 Thread Thomas Martitz

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

2015-04-04 Thread Thomas Martitz

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

2015-04-04 Thread Thomas Martitz

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