Proxy plugins #629

kugel- · 2015-08-26T22:40:13Z

This is the patch set that implements proxy plugins as per mailing list thread a few months ago.

Please see commit a006f71 ("plugins: add geany_plugin_register_proxy() to the plugin API") which contains a detailed description via API documentation.

Short summary: Plugins can register as proxy plugins. They register a list of file extensions and a few callbacks. Then, when Geany scans for normal plugins it will also look for these new file types, and call the proxy plugin's callbacks instead of the built-in shared library loader. The proxy will ultimately call geany_plugin_register() on behalf of the sub-plugin.

To the user, these sub-plugins are indistinguishable except the Plugin Manager indicates the relationship to the proxy via a TreeView. Within Geany, they are treated as first-class citizens.

Please review (and ideally merge).

kugel- · 2015-08-27T09:53:39Z

I don't know where to PR'ify but here's a converted GeanyPy acting as a real-word showcase: https://github.com/kugel-/geany-plugins/tree/geanypy-pluxy

kugel- · 2015-09-02T15:28:31Z

@b4n @elextr @codebrainz sorry to annoy you but I really really want this for 1.26 so give it a look pretty please

codebrainz · 2015-09-03T02:58:13Z

@kugel- this weekend is a long-weekend here, if you ping me on Saturday (I'm about half a day behind your timezone), I will try to sit down with this and at least give it some testing/reviewing.

It looks to have a build error for this newish function on all platforms, according to Travis.

b4n · 2015-09-04T14:43:50Z

@b4n @elextr @codebrainz sorry to annoy you but I really really want this for 1.26 so give it a look pretty please

I'm busy this WE, but I should be on it starting next Monday. Feel free to ping me continuously if I don't ;)

codebrainz · 2015-09-06T17:01:19Z

plugins/demoproxy.c

+ *      demoproxy.c - this file is part of Geany, a fast and lightweight IDE
+ *
+ *      Copyright 2007-2012 Enrico Tröger <enrico(dot)troeger(at)uvena(dot)de>
+ *      Copyright 2007-2012 Nick Treleaven <nick(dot)treleaven(at)btinternet(dot)com>


Since you wrote this new file, it would be more appropriate to put your copyright in there.

Agree this should be copyright @kugel-

elextr · 2015-09-07T02:20:39Z

@kugel- all travis builds show an implicitly defined function. Did you miss an include file?

b4n · 2015-09-07T16:18:54Z

Here's what GCC tells me when I build this:

../../src/plugins.c: In function 'proxied_count_inc':
../../src/plugins.c:140:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
../../src/plugins.c: In function 'proxied_count_dec':
../../src/plugins.c:149:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^
../../src/plugins.c: In function 'plugin_load_so':
../../src/plugins.c:590:14: warning: unused variable 'p_geany_data' [-Wunused-variable]
  GeanyData **p_geany_data;
              ^
../../src/plugins.c: In function 'plugin_new':
../../src/plugins.c:648:11: warning: unused variable 'module' [-Wunused-variable]
  GModule *module;
           ^
../../src/plugins.c: In function 'unregister_proxy':
../../src/plugins.c:897:110: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  foreach_ptr_array(p, i, supported_plugin_list)
                                                                                                              ^
../../src/plugins.c: In function 'is_plugin':
../../src/plugins.c:1022:114: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
  foreach_ptr_array(proxy, i, supported_plugin_list)
                                                                                                                  ^
../../src/plugins.c: In function 'load_plugins_from_path':
../../src/plugins.c:1086:17: warning: unused variable 'tmp' [-Wunused-variable]
  gchar *fname, *tmp;
                 ^
../../src/plugins.c: In function 'pm_plugin_toggled':
../../src/plugins.c:1432:29: warning: unused variable 'valid' [-Wunused-variable]
  gboolean old_state, state, valid;
                             ^
../../src/plugins.c: In function 'pm_treeview_query_tooltip':
../../src/plugins.c:1583:24: warning: initialization discards 'const' qualifier from pointer target type
   gchar *uncheck_str = "\n<i>Other plugins depend on this. Disable them first to allow deactivation.</i>\n";
                        ^
../../src/plugins.c:1590:3: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
   markup = g_strdup_printf(tmp, can_uncheck ? "" : uncheck_str);
   ^
../../src/plugins.c: In function 'pm_prepare_treeview':
../../src/plugins.c:1738:9: warning: unused variable 'list' [-Wunused-variable]
  GList *list;
         ^
../../src/plugins.c:1737:14: warning: unused variable 'iter' [-Wunused-variable]
  GtkTreeIter iter;
              ^
../../src/plugins.c: At top level:
../../src/plugins.c:78:22: warning: 'builtin_extensions' defined but not used [-Wunused-variable]
 static const gchar * builtin_extensions[] = { G_MODULE_SUFFIX, NULL };
                      ^

../../plugins/demoproxy.c: In function 'proxy_init':
../../plugins/demoproxy.c:68:3: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
   gchar *key = g_strdup_printf(fmt, i++);
   ^
../../plugins/demoproxy.c: In function 'proxy_help':
../../plugins/demoproxy.c:92:12: warning: unused variable 'file' [-Wunused-variable]
  GKeyFile *file;
            ^

b4n · 2015-09-07T19:39:37Z

I find the geany_plugin_register_proxy() API a bit odd: the proxy has to call this, yet fill the vfuncs/callback in the GeanyPlugin structure. This seem odd API-wise to me, and seem to lead to a seemingly arbitrary limitation of one proxy per native plugin.
Is there a particular reason why it's done that way?

kugel- · 2015-09-07T20:24:02Z

The separate API allows to do the registration in the plugin's init(), and have the file extensions deppend on plugin configuration. Also a API-wise separattion to geany_plugin_register() is needed to allow for nested proxies.

A proxy can be a proxy for many types. the only limitation is that only one set of ProxyFuncs is supported per proxy. But this limitation is not a problem, since the proxies can do their own dispatching. For geany it simplifies storing the pointers (avoids introducing a list of vtables, etc)

b4n · 2015-09-08T13:33:35Z

src/plugins.c

+	if (tmp == NULL)
+		return FALSE;
+	/* ensure the dot is really part of the filename */
+	else if (strchr(tmp, G_DIR_SEPARATOR) != NULL)


On Windows we probably also need to check /

Oh look. I used strchr(). I'm inconsistent with myself :/

Regarding the /, do we have to do this everywhere? I didn't notice an #ifdef hell about that yet

We don't deconstruct paths manually very often, and would generally use g_path_get_basename(). But we do have utils_tidy_path() that does it. I'm not certain we need it here, but I could easily imagine it could happen.

kugel- · 2015-09-21T15:20:00Z

@b4n @codebrainz @elextr I hopefully addressed all remarks, please review again (I'll take care of the travis issue later on)

kugel- · 2015-09-22T14:35:00Z

I forced pushed to some fixup commits since those haven't been commented on yet.

b4n · 2015-09-24T18:00:13Z

plugins/demoproxy.c

+ *
+ * Note: This is not installed by default, but (on *nix) you can install it as follows:
+ *
+ * Compile geany.Then copy or symlink the plugins/demoproxy.so and plugins/demoproxytest.px files


missing space between sentences

b4n · 2015-10-02T23:33:43Z

src/plugins.c

+static gpointer plugin_load_gmodule(GeanyPlugin *proxy, GeanyPlugin *subplugin, const gchar *fname, gpointer pdata)
+{
+	GModule *module;
+	GeanyData **p_geany_data;


b4n · 2015-10-02T23:54:08Z

Note to myself: don't forget to bump API when merging this!

kugel- · 2015-10-02T23:57:06Z

BTW if anyone wants to actually test this, there is the demoproxy (obviously) but also an adapted geanypy on my repo

https://github.com/kugel-/geany-plugins/tree/geanypy-pluxy

b4n · 2015-10-03T19:42:22Z

(I quickly tried to start squashing commits, and it'll be a little hard because many fixes alter next commit/fixes a bit too much for automatic merges to work. Doing it will take some time)

@kugel- thanks for the link, it'll be interesting to see a real usage :)

kugel- · 2015-10-03T19:48:08Z

You're welcome. I posted it already in the very early days of the PR :-)

The Squashing turns our non-trivial, I'll take care

kugel- · 2015-10-03T22:09:05Z

Here is the merged branch: https://github.com/kugel-/geany/tree/pluxy-merge

I added commits, which should be self-explainatory. Other than that, I fixed a @SInCE tag and a last "pluxies" occurence.

Tell me if I should push that -merge branch to this PR.

kugel- · 2015-10-03T22:24:47Z

I also added the above mengtioned as commits here

b4n · 2015-10-05T15:52:49Z

src/plugins.c

@@ -712,6 +805,40 @@ static void remove_sources(Plugin *plugin)
 }


+/* returns true if the plugin is native, i.e. not loaded through a proxy.
+ * This determines whether it's backed by a GModule */


this comment looks wrong

Currently they encapsulate loading and unloading of standard plugins. In the future plugins can provide such functions to load their types of plugins. Such a dummy proxy plugin is implemented now to load standard plugins so that these aren't going to be specially handled.

Being a GModule is actually a detail of standard plugins. Future proxy plugins might need different handles. Therefore replace the module field with a more generic pointer and encapsulate the GModule detail further. This pointer shall be returned from GeanyProxyFuncs::load and is passed back to GeanyProxyFuncs::unload, and isn't interpreted by Geany.

…re added During the loading of the active plugins they are also initialized (done at startup). As a result, these plugins could be pluxys and make more plugins available, some of which may be active as well. Because of this the loop has to be restarted if pluxies become available to also load active plugins that depend on the pluxy. The loop is only restarted at the end so only nested pluxys could possibly cause the loop to be run more than twice.

When a file extension alone is ambigious as to whether a potential plugin is really handled then the proxy should use the probe hook to find out. This can be especially helpful when two pluxies work on the same file extension. The proxy's probe() should return PROXY_IGNORED or PROXY_MATCHED accordingly. A special flag value, PROXY_NOLOAD, can be or'ed into PROXY_MATCHED to say that the file belongs to the proxy, but isn't directly loaded and should not be handled by any other proxy or geany itself. Example for PROXY_IGNORED: geanypy only supports python2 at the moment. So, scripts written for python3 aren't handled by it and should be skipped for the PM dialog. Or perhaps they are handled by another proxy that supports python3. Example for PROXY_NOLOAD: A pluxy registers for the metadata file extension (.plugin) where author etc is in. The actual implmentation is in a python script (.py). The .py file is tied to the .plugin and should not be processed by other pluxies. Thus, the pluxy also registers for the .py extension but returns PROXY_MATCHED|PROXY_NOLOAD for it (if it would return only PROXY_MATCHED the sub-plugin would show up twice in the PM dialog).

This function finally allows plugins to register themselves as a proxy for one or more file extensions. Lots of documentation is added to doc/plugins.dox, please refer to that for more details.

When enabling/disabling pluxys in the PM dialog the list of available plugins might change. If plugins before the pluxy go/come then the wrong plugin becomes selected (the selected row number stays the same). Re-apply the selection to the current one in the toggle callback to overcome this issue.

kugel- · 2015-10-05T20:19:10Z

@b4n pushed fixes. I updated https://github.com/kugel-/geany/tree/pluxy-merge accordingly.

b4n · 2015-10-06T13:33:13Z

LGTM, and merge branch is in sync. @codebrainz could you give it a quick test please?

This demo proxy does not actually do anything useful. It simply loads pseudo-plugins from an ini-style file. The point is that there will be a plugin in the PM dialog for each ini. Each ini-plugin also causes a menu item to be generated.

Geany now remembers how many plugins depend on a pluxy. It uses this information to disable the "Active" checkbox in the PM dialog. Additionally, the PM dialog displays plugins in a hierarchical manner, so that sub-plugins are shown next to pluxy. This is espcially handy since it makes the sub-plugin <-> pluxy relationship really obvious, and it's easier to spot which plugins need to be disabled before the pluxy can be disabled. This allows to remove code to re-select the plugin because the row (respective to the hierarchy level) does not change anymore.

g_ptr_array_insert() is too recent (2.40), but prepending is required. GQueue is a fine replacement with better old-glib support, at the expense of working with a doubly-linked list instead of plain array.

In the future we might want to enable calling it again to set new supported plugin types/extensions. This is not implemented yet, but in order to allow this in the future we have to prevent it now, otherwise we'd need to break the API.

b4n · 2015-10-06T13:50:39Z

Pre-rebase commits were:

0def4f5 plugins: introduce load and unload functions for plugins
d11ada3 plugins: generic load_data instead of module pointer in Plugin struct
bf7bbca plugins: refactor GtkListStore population code into separate function
7acda46 plugins: when loading active ones, loop until no more proxy plugins are added
533c4d6 plugins: introduce probe() for proxy plugins
a006f71 plugins: add geany_plugin_register_proxy() to the plugin API
ce9fe0b plugins: reselect when toggling the current plugin
0e9923e demopluxy: add a demo pluxy showcasing how to create a proxy plugin
e1e8349 plugins: improve PM dialog for proxy and sub-plugins
883ac8b Fixup 0def4f5 (plugins: introduce load and unload functions for plugins)
2e36527 Fixup d11ada3 (plugins: generic load_data instead of module pointer in Plugin struct)
dac823b Fixup bf7bbca (plugins: refactor GtkListStore population code into separate function)
290e3ab Fixup 7acda46 (plugins: when loading active ones, loop until no more proxy plugins are added)
50aecf7 Fixup a006f71 (plugins: add geany_plugin_register_proxy() to the plugin API)
04e5e52 Fixup ce9fe0b (plugins: reselect when toggling the current plugin)
9f67844 Fixup 0e9923e (demopluxy: add a demo pluxy showcasing how to create a proxy plugin)
31d94e7 Fixup e1e8349 (plugins: improve PM dialog for proxy and sub-plugins)
d148920 plugins: use GQueue to restore GLib compatibility
85a1674 Fixup 0def4f5 #2 (plugins: introduce load and unload functions for plugins)
2e443d1 Fixup 7acda46 #2 (plugins: when loading active ones, loop until no more proxy plugins are added)
b87c711 Fixup a006f71 #2 (plugins: add geany_plugin_register_proxy() to the plugin API)
f86877f Fixup ce9fe0b #2 (plugins: reselect when toggling the current plugin)
336f641 Fixup 0e9923e #2 (demopluxy: add a demo pluxy showcasing how to create a proxy plugin)
367f303 Fixup e1e8349 #2 (plugins: improve PM dialog for proxy and sub-plugins)
5a3872a Fixup d148920 (plugins: use GQueue to restore GLib compatibility)
6e1dfd4 Fixup a006f71 #3 (plugins: add geany_plugin_register_proxy() to the plugin API)
8e7de03 Fixup 0e9923e #3 (demopluxy: add a demo pluxy showcasing how to create a proxy plug
f90bc9c Fixup a006f71 #4 (plugins: add geany_plugin_register_proxy() to the plugin API)
7c329d0 Fixup 0e9923e #4 (demopluxy: add a demo pluxy showcasing how to create a proxy plugin)
1529158 Fixup d148920 #2 (plugins: use GQueue to restore GLib compatibility)
c54735e Fixup 0def4f5 #3 (plugins: introduce load and unload functions for plugins)
8a9c66a Fixup bf7bbca #2 (plugins: refactor GtkListStore population code into separate function)
d6978be Fixup 0e9923e #5 (demopluxy: add a demo pluxy showcasing how to create a proxy plugin)
38b4c03 Fixup 0def4f5 #4 (plugins: introduce load and unload functions for plugins)
be02a70 Fixup d11ada3 #2 (plugins: generic load_data instead of module pointer in Plugin struct)
c7ef4aa Fixup bf7bbca #3 (plugins: refactor GtkListStore population code into separate function)
13699be Fixup a006f71 #5 (plugins: add geany_plugin_register_proxy() to the plugin API)
df6569a Fixup a006f71 #6 (plugins: add geany_plugin_register_proxy() to the plugin API)
0252408 plugins: do not pass potentially destroyed data to unload callback
d5a06a7 plugins: enfore geany_plugin_register_proxy() can be called once
8161913 Fixup 0def4f5 #5 (plugins: introduce load and unload functions for plugins)
efc4f78 Fixup d11ada3 #3 (plugins: generic load_data instead of module pointer in Plugin struct)
90c905a Revert "plugins: do not pass potentially destroyed data to unload callback"

Add support for plugins acting as proxies for foreign plugins, promoting foreign plugins to first-class citizen.

codebrainz added this to the 1.26 milestone Sep 3, 2015

codebrainz added enhancement plugin API labels Sep 3, 2015

codebrainz self-assigned this Sep 3, 2015

codebrainz reviewed Sep 6, 2015
View reviewed changes

b4n reviewed Sep 8, 2015
View reviewed changes

kugel- force-pushed the pluxy branch from 5cda5b7 to d148920 Compare September 22, 2015 14:33

b4n reviewed Sep 24, 2015
View reviewed changes

b4n reviewed Oct 2, 2015
View reviewed changes

b4n reviewed Oct 5, 2015
View reviewed changes

kugel- added 7 commits October 5, 2015 22:09

plugins: refactor GtkListStore population code into separate function

203644a

plugins: add geany_plugin_register_proxy() to the plugin API

6e5ca69

This function finally allows plugins to register themselves as a proxy for one or more file extensions. Lots of documentation is added to doc/plugins.dox, please refer to that for more details.

kugel- and others added 5 commits October 6, 2015 15:40

plugins: use GQueue to restore GLib compatibility

6dfe5ce

g_ptr_array_insert() is too recent (2.40), but prepending is required. GQueue is a fine replacement with better old-glib support, at the expense of working with a doubly-linked list instead of plain array.

Bump plugin API version for proxy plugins support

d0f9446

kugel- force-pushed the pluxy branch from 90c905a to d0f9446 Compare October 6, 2015 13:46

b4n merged commit d0f9446 into geany:master Oct 6, 2015

b4n added a commit that referenced this pull request Oct 6, 2015

Merge pull request #629 from kugel-/pluxy

c6952c7

Add support for plugins acting as proxies for foreign plugins, promoting foreign plugins to first-class citizen.

b4n assigned b4n and unassigned codebrainz Oct 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proxy plugins #629

Proxy plugins #629

kugel- commented Aug 26, 2015

kugel- commented Aug 27, 2015

kugel- commented Sep 2, 2015

codebrainz commented Sep 3, 2015

b4n commented Sep 4, 2015

codebrainz Sep 6, 2015

elextr Sep 25, 2015

elextr commented Sep 7, 2015

b4n commented Sep 7, 2015

b4n commented Sep 7, 2015

kugel- commented Sep 7, 2015

b4n Sep 8, 2015

kugel- Sep 8, 2015

b4n Sep 8, 2015

kugel- commented Sep 21, 2015

kugel- commented Sep 22, 2015

b4n Sep 24, 2015

b4n Oct 2, 2015

b4n commented Oct 2, 2015

kugel- commented Oct 2, 2015

b4n commented Oct 3, 2015

kugel- commented Oct 3, 2015

kugel- commented Oct 3, 2015

kugel- commented Oct 3, 2015

b4n Oct 5, 2015

kugel- commented Oct 5, 2015

b4n commented Oct 6, 2015

b4n commented Oct 6, 2015

Proxy plugins #629

Proxy plugins #629

Conversation

kugel- commented Aug 26, 2015

kugel- commented Aug 27, 2015

kugel- commented Sep 2, 2015

codebrainz commented Sep 3, 2015

b4n commented Sep 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elextr commented Sep 7, 2015

b4n commented Sep 7, 2015

b4n commented Sep 7, 2015

kugel- commented Sep 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kugel- commented Sep 21, 2015

kugel- commented Sep 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b4n commented Oct 2, 2015

kugel- commented Oct 2, 2015

b4n commented Oct 3, 2015

kugel- commented Oct 3, 2015

kugel- commented Oct 3, 2015

kugel- commented Oct 3, 2015

Choose a reason for hiding this comment

kugel- commented Oct 5, 2015

b4n commented Oct 6, 2015

b4n commented Oct 6, 2015