> @@ -0,0 +1,218 @@
> +/*
> + * 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>
Agree this should be copyright @kugel-
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40393521
> + g_free((gchar *)plugin->info->author);
> +
> + g_key_file_free(data->file);
> + g_free(data);
> +}
> +@endcode
> +
> +Finally the demo_proxy's wrapper GeanyPluginFuncs. They are called for each possible sub-plugin and
> +therefore have to multiplex between each using the plugin-defined data pointer. Each is called by
> +Geany as if it were an ordinary, native plugin.
> +
> +proxy_init() actually reads the inferior's file using GKeyFile APIs. It prepares for the help
> +dialog and installs the menu items. proxy_help() is called when the user clicks the help button in
> +the Plugin Manager. Consequently, this fires up a suitable dialog, although with a dummy message.
> +proxy_cleanup() frees all memory allocated in proxy_init().
> +
It isn't clear if the ini file is just something this demo uses to define its sub-plugins, or if its required that all proxies use it, and if so why?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40393473
> +
> +The next step is to actually register as a proxy plugin. This is done in demoproxy_init().
> +As previously mentioned, it needs a list of accepted file extensions and a set of callback
> +functions.
> +
> +(a)code{.c}
> +static gboolean demoproxy_init(GeanyPlugin *plugin, gpointer pdata)
> +{
> + const gchar *extensions[] = { "ini", "px", NULL };
> +
> + plugin->proxy_funcs->probe = demoproxy_probe;
> + plugin->proxy_funcs->load = demoproxy_load;
> + plugin->proxy_funcs->unload = demoproxy_unload;
> +
> + return geany_plugin_register_proxy(plugin, extensions);
> +}
As discussed on IRC [here](http://irc.geany.org/logs/log_20150924.html), while I understand what @b4n means by the API is "funny" I don't think there is a better one that still meets at least some of simple, extendable, typesafe, so I would suggest to just leave it with the current proposal.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40393400
> + {
> + match = utils_str_equal(linebuf, "#!!PROXY_MAGIC!!\n");
> + fclose(f);
> + }
> + return match ? PROXY_MATCHED : PROXY_IGNORED;
> +}
> +@endcode
> +
> +GeanyProxyFuncs::load is a bit more complex. It reads the file, fills the inferior's PluginInfo
> +fields and calls GEANY_PLUGIN_REGISTER_FULL(). Additionally, it creates a per-plugin context that
> +holds GKeyFile instance (a poor man's interpeter context). You can also see that it does not call
> +GEANY_PLUGIN_REGISTER_FULL() if g_key_file_load_from_file() found an error (probably a syntax
> +problem) which means the sub-plugin cannot be enabled.
> +
> +It also installs wrapper functions for the inferior's GeanyPluginFuncs as Ini files aren't code.
> +It's very likely that your proxy needs something similar because you can only install function
s/inferior/sub-plugin/g rather than using mixed terms, sometimes "sub-plugin", sometimes "inferior".
In general in general English "sub-plugin" has fewer negative connotations compared to "inferior plugin" and isn't the main point of these proxy changes to make the "sub-plugins" appear to be normal plugins.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40382658
> +
> + plugin->proxy_funcs->probe = demoproxy_probe;
> + plugin->proxy_funcs->load = demoproxy_load;
> + plugin->proxy_funcs->unload = demoproxy_unload;
> +
> + return geany_plugin_register_proxy(plugin, extensions);
> +}
> +
> +@endcode
> +
> +The callback functions deserve a closer look.
> +
> +As already mentioned the file format includes a magic first line which must be present.
> +GeanyProxyFuncs::probe() verifies that it's present and avoids showing the sub-plugin in the
> +Plugin Manager if not.
> +
As a general comment, I got confused which functionalities are part of Geany, and which are part of the proxy because the language says a function "does something" making it sound like it exists and so is part of Geany, rather than "must do something" which states a requirement for the proxy. So the above sentence should read:
"GeanyProxyFuncs::probe() should verify that the correct identification is present and return `false` so Geany avoids showing the sub-plugin in the Plugin Manager."
That way its clear what the callback function does and what Geany does.
This applies in other places as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40382165
> +item2 = Bar
> +
> +[Help]
> +text = I'm a simple test. Nothing to see!
> +
> +[Info]
> +name = Demo Proxy Tester
> +description = I'm a simple test. Nothing to see!
> +version = 0.1
> +author = The Geany developer team
> +@endcode
> +
> +The first line acts as a verification that this file is truly a sub-plugin. Within the [Init] section
> +there is the menu items for Geany's tools menu. The [Help] section declares the sub-plugins help
> +text which is shown in its help dialog (via GeanyPluginFuncs::help). The [Info] section is
> +used as-is for filling the sub-plugins PluginInfo fields.
I would make this a list with each section a list item rather than one paragraph.
"The [Init] section contains the menu items ..." would be better English.
What is the number after item? Is it the position in the menu? Is it just a meaningless variation so the keys are different, in which case maybe it would maybe be better as a list rather than separate items, do mnemonic underscores work?
What do you mean "as-is" for the info section?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40381379
> +plugins should get a chance to load the candidate or not.
> +
> +@section proxy_dep_guideline Guideline for Runtime Errors
> +
> +A sub-plugin might not be able to run even if it's perfectly compatible with its proxy. This
> +includes the case when it lacks certain runtime dependencies such as programs or modules but also
> +syntactic problems or other errors.
> +
> +There are two basic classes:
> +
> +1) Runtime errors that can be determined at load time. For example, the shebang of a script
> +indicates a specific interpeter version but that version is not installed on the system. Your proxy
> +should respond the same way as for version-incompatible plugins: don't register the plugin at
> +all, but leave a message the user suggesting what has to be installed in order to work. Handle
> +sytanx errors in the scripts of sub-plugins the same way if possible.
> +
syntax
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629/files#r40380144
If one adds a new tag, <b>Geany</b> (very helpfully) adds the closing tag - line 22. But if one changes a tag that already has a matching closing tag it does not change the closing tag to match - line 23. Should it be changed to make this behaviour more consistent?
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/issues/664
> Wait a minute, for the new_hooks you convinced me that *this* method flawed (due to the points you mentioned) and now you're trying to sell it to me? I'm confused.
It's a little more subtle and a little less absurd than that :)
IIRC what you initially did in new_hooks was to fill in a struct to copy it back to the GeanyPlugin structure. As I agree the funcs are part of the plugin itself, and as it has the above-mentioned flaws, I found it to make a lot more sense to fill the existing structure right away.
Here the thing I find odd is that I feel like the plugin itself and the proxy are two separate things, so filling the first to register the second seem odd to me. They require calling 2 different register functions, but on the same structure. That's the only reason why I somewhat try to find an alternative API I find more straightforward (for me).
Do anybody else feel that way? @elextr @codebrainz ?
Anyway, having
```C
Proxy *proxy = proxy_new();
proxy->probe = my_probe;
...
plugin_registre_proxy(plugin, proxy);
```
would basically make as much sense than the plain struct initializer solution, but requires adding more API (the _new()). Not saying it's a great API either just now though :)
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629#issuecomment-143062346
Am 24.09.2015 um 23:42 schrieb Colomban Wendling:
>
> 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.
>
> I get that, my point was that I would find the API less odd if one
> filled a specific structure, rather than one already used for
> something else.
>
> e.g.
>
> Proxy proxy = {
> .extensions = {"so",NULL },
> .load = my_load,
> .probe = my_probe,
> .unload = my_unload
> };
> geany_plugin_register_proxy(plugin, &proxy);
Wait a minute, for the new_hooks you convinced me that *this* method
flawed (due to the points you mentioned) and now you're trying to sell
it to me? I'm confused.
The way I implemented aimed to be consistent with the just-merged
new_hooks and other stuff and doesn't have extensibility problems.
Best regards
---
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/629#issuecomment-143059222