Re: [Geany-Devel] New plugin loader mechanisms
Hi,
in the hope I answered your pluxy-related questions in the other post
I'll try to concentrate on the loader for this reply.
Am 29.03.2015 um 19:04 schrieb Colomban Wendling:
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
1) Provide the plugin handle to the plugin very early
2) 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?
See the other thread. plugin_b is also created by Geany, in terms of
locating the file, allocating GeanyPluginPrivate and integrations with
the PM dialog. Only the job of loading/unloading is offloaded to the
proxy (because it knows how to do it), and nothing else.
```
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;
};
```
Now this is a big methodology change, from Geany-allocated plugin
handles to plugin-allocated plugin handles. This change has so many
implications and touches so many things that I really don't want to do
it. And given we want to maintain compatiblity to old plugins we have to
support both at the same time.
My new loader is much less invasive, *by design*. It only changes the
way the already known hooks are registered and called (but still being
transparent to the plugin), in order to provide a solution to the
problems I outlined in the initial mail. And it provides binary
compatibility to older plugins at very little extra complexity.
I am mostly happy with how we load plugins, with my new loader I am
completely happy, so I don't feel the big change you propose is
justified. I don't do all of this for the sake of change, I want to
provide an effective solution, and the fundament for proxy plugins. I
don't want to change all the way we interact with plugins, which will
also require plugin developers to re-learn.
This is by far not the primary reason, but I also try to keep the
changes less invasive to actually improve the chance of it being
reviewed and merged in a reasonable time frame.
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.
GeanyPlugin is Geany's *handle* to the plugin and required to call lots
of our API functions. With no global pointer being set in the plugin we
have to provide the
Re: [Geany-Devel] New plugin loader mechanisms
Hi,
Le 30/03/2015 08:52, Thomas Martitz a écrit :
[…]
```
struct MyRealPlugin {
RealPlugin parent; /* offset 0 has the Geany struct, so it's binary
compatible */
/* plugin-specific fields here */
int the_game;
};
```
Now this is a big methodology change, from Geany-allocated plugin
handles to plugin-allocated plugin handles. This change has so many
implications and touches so many things that I really don't want to do
it. And given we want to maintain compatiblity to old plugins we have to
support both at the same time.
My new loader is much less invasive, *by design*. It only changes the
way the already known hooks are registered and called (but still being
transparent to the plugin), in order to provide a solution to the
problems I outlined in the initial mail. And it provides binary
compatibility to older plugins at very little extra complexity.
I am mostly happy with how we load plugins, with my new loader I am
completely happy, so I don't feel the big change you propose is
justified. I don't do all of this for the sake of change, I want to
provide an effective solution, and the fundament for proxy plugins. I
don't want to change all the way we interact with plugins, which will
also require plugin developers to re-learn.
This is by far not the primary reason, but I also try to keep the
changes less invasive to actually improve the chance of it being
reviewed and merged in a reasonable time frame.
OK, this explanation makes me feel better with your proposal :)
What concerned me mostly was that you were proposing a new API, but I
felt it being riddled with relics from the past -- e.g. instead of
designing the new thing to be great on its own, you made it to be the
same as the old one, just getting rid of the most obvious shortcomings.
And I didn't feel very comfortable with this, because I felt like we'd
then probably want to rewrite it again in the (near) future, which would
mean breakage or introducing a third API. Basically I feel that if a
new thing was introduced, it should be done right, and the complexity of
keeping compatibility should be a minor concern.
However, with some of the reasoning above (and below), I feel more
confident it is not only choices made for facility, so that's a bit of a
relief :)
And I must admit that indeed low change impact is unfortunately a
factor… Scotty, we need more (man) power! :)
[…]
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.
GeanyPlugin is Geany's *handle* to the plugin and required to call lots
of our API functions. With no global pointer being set in the plugin we
have to provide the pointer to the plugin code through function arguments.
Good point. Though in my new thing idea the plugin-provided self would
have been the new handle, but it's true that it'd require API changes.
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.
I don't understand this part. What's the implications of translated
plugin info? I think it only needs GeanyData to be available?
This last sentence is partly a leftover from when you still had a
set_info() plugin vfunc, so you can mostly forget it. The only real
point I tried to make was that we need the plugin info no matter what,
so we could (should) give it when registering the plugin, not after.
But you addressed that now, so this point is moot :)
[…] 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()?
But what's the problem with PluginCallback? It's a convenient and useful
feature to allow signal connections to be statically defined. The cost
to support is really tiny, and it's not like we deprecated this API
aspect so I don't see removing it is justified. Removing things from the
API still requires some considerations.
I was suggesting this consideration :)
I never saw much use for it, but if people prefer this way of setting
handlers, well, why not. Just remember this can *only* connect to
Geany's own signals, none else.
[…]
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?
It could be done this way too, indeed. Actually I don't use the
Re: [Geany-Devel] My non-C plugin roadmap
Am 30.03.2015 um 14:33 schrieb Lex Trotman:
What is is_plugin()? If its a function in Geany how does it get to
know about new types of plugins without being hard coded?
It knows because the extensions are registered by the function below
called by another plugin right? How do you make sure that the
register function has been called before you come across a file with
that extension?
For now, I kept it simple: Geany simply restarts the scan for plugins
loop when new extensions are added during the the scan (remember that
during the scan, each file is loaded and its init() is called, before
the next file is even attempted). The PM dialog is refreshed in the same
way when a pluxy is activated by the user.
It can be made smarter, but it's good enough at the moment.
https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L878 and
https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1272
It's new, small helper function I added. It loops through all known file
extensions, and returns the first pluxy (a PluginProxy *) for which a) the
supported file extension matches and b) the probe hook returned true (or is
NULL, for standard plugins).
$file is not a plugin if is_plugin returns NULL, i.e. no pluxy was found.
https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L844
File extensions and the proxy hooks (probe, load, unload) are registered by
a plugin during its init() through the new plugin_register_proxy() function.
Ok, so this registers a new type of plugin by its extension(s) and
that type is associated with a plugin that provides:
- the loader functionality?
- interface wrappers/bindings (like geanypy does)?
- starts/loads any other things, like the Python interpretor or a JVM
or Haskell runtime?
Yes, but the 2nd and 3rd points are entirely up to the pluxy. My
demopluxy.so doesn't do anything fancy. it creates a dummy plugin out of
a GKeyFile.
But the loader/unloader function is mandatory, it also acts as the entry
point for pluxies to start their bindings/vm machinery if necessary.
Here the pluxy added to the list of registered pluxies. This list is
initialized with the simulated pluxy that provides standard plugins (this is
not a plugin, it's contained in plugins.c, it's just to keep the code paths
equal).
https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1601
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)
I'm guessing probe() is a function that looks for something in the .so
that distinguishes if its new or old loader, but what about others?
It depends on the pluxy what the prope() function does! For my peasy pluxy
(that provides generic support for libpeas-based plugins), it looks if there
is a matching *.plugin for a given *.so, and if yes return a code so that
Geany does not attempt to process the .so itself.
https://github.com/kugel-/peasy/blob/master/src/peasy.c#L73
There is no probe() for standard plugins, it accepts all .so.
If it doesn't call probe how does it know if its a traditional plugin,
or a peas one or maybe some other version that makes a .so file?
(Haskell anybody :)
Geany doesn't and cannot know it's a peas plugin, though it could be
smarter at determining it's a standard plugin. But I currently
implemented a scheme where the first pluxy that matches wins, but if it
doesn't match all other pluxes are tried. Geany itself is tried last.
This should work for all real-world cases, even when there are multiple
(more than 2) providers for .so files. If the pluxies are accurate
enough at determining their own filetypes then no conflicts arise.
Geany is always tried last, so if it gets to process a .so file, then it
assumes it's a standard plugin like it's done in git master (no change
here)
In the load hook of the pluxy. Either the pluxy calls it directly or it
decides to provide a suitable binding so that the non-C script can call it
itself, but it has to be during the execution of the load hook.
Ok, so if there are all these hooks, probably later the traditional
plugins can be just another pre-registered set of hooks built into
Geany, but I agree with your approach of not trying to do that all in
the first step, leave the existing code as little changed as possible
until the new system settles down and then change the existing code to
use it.
It's done this way already, the hooks are compiled into Geany. As I said
the list of pluxies is initialized with a simulated one that provides
the standard plugins { .extension = { so, NULL }, .probe = NULL, .load
= plugin_load_so, .unload = plugin_unload_so };
Best regards
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] My non-C plugin roadmap
[...] Ok, this explains some of what I was asking on the other thread, so now I can ask the more specific questions below that are the key points in the confusion. with my new loader (no pluxies) it goes like this, and this is *very* similar to git master. user opens PM dialog 1 Geany calls load_all_plugins(), which calls load_plugins_from_path($path) 2 for each $file in $path, Geany checks if the extension is G_MODULE_SUFFIX, and calls plugin_new($file, ...) 3 plugin_new() calls the plugins's geany_load_module() (if new-style plugin, calls version_check, set_info() for old-style ones) 4 geany_load_module() is implemented by the plugin, and registers itself with geany_plugin_register() geany_plugin_register() adds the plugin to the plugin list, so that the PM can sort and show it Now, with pluxies, it is completely the same except for: 2* for each $file in $path, Geany calls is_plugin($file) What is is_plugin()? If its a function in Geany how does it get to know about new types of plugins without being hard coded? 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) I'm guessing probe() is a function that looks for something in the .so that distinguishes if its new or old loader, but what about others? 3* plugin_new() calls the load() hook registered by pluxies for the given extension. for standard plugins (without proxy) there is a predefined plugin_load_so() funtion that gets called instead. How does the load hook get defined for new types of plugins? 4* The load-hook calls geany_plugin_register(), here Geany core and proxies work the same way Where is the geany_plugin_register() defined for a plugin written in a language that isn't C/C++/Vala that can produce a .so file? I designed it such, that the difference between standard plugins and proxied plugins is all contained in the load hook. The rest of Geany does not know about the difference. This ensures proxied plugins are first class citizens. Thats the correct target I agree, I just don't understand the design details yet. Cheers Lex ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] My non-C plugin roadmap
Hello, some if your question are easier to answer by looking at the code, I'll link the appropriate sections. Am 30.03.2015 um 13:16 schrieb Lex Trotman: [...] Ok, this explains some of what I was asking on the other thread, so now I can ask the more specific questions below that are the key points in the confusion. with my new loader (no pluxies) it goes like this, and this is *very* similar to git master. user opens PM dialog 1 Geany calls load_all_plugins(), which calls load_plugins_from_path($path) 2 for each $file in $path, Geany checks if the extension is G_MODULE_SUFFIX, and calls plugin_new($file, ...) 3 plugin_new() calls the plugins's geany_load_module() (if new-style plugin, calls version_check, set_info() for old-style ones) 4 geany_load_module() is implemented by the plugin, and registers itself with geany_plugin_register() geany_plugin_register() adds the plugin to the plugin list, so that the PM can sort and show it Now, with pluxies, it is completely the same except for: 2* for each $file in $path, Geany calls is_plugin($file) What is is_plugin()? If its a function in Geany how does it get to know about new types of plugins without being hard coded? It's new, small helper function I added. It loops through all known file extensions, and returns the first pluxy (a PluginProxy *) for which a) the supported file extension matches and b) the probe hook returned true (or is NULL, for standard plugins). $file is not a plugin if is_plugin returns NULL, i.e. no pluxy was found. https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L844 File extensions and the proxy hooks (probe, load, unload) are registered by a plugin during its init() through the new plugin_register_proxy() function. Here the pluxy added to the list of registered pluxies. This list is initialized with the simulated pluxy that provides standard plugins (this is not a plugin, it's contained in plugins.c, it's just to keep the code paths equal). https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1601 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) I'm guessing probe() is a function that looks for something in the .so that distinguishes if its new or old loader, but what about others? It depends on the pluxy what the prope() function does! For my peasy pluxy (that provides generic support for libpeas-based plugins), it looks if there is a matching *.plugin for a given *.so, and if yes return a code so that Geany does not attempt to process the .so itself. https://github.com/kugel-/peasy/blob/master/src/peasy.c#L73 There is no probe() for standard plugins, it accepts all .so. Whether it's a new or old style plugin is determined later. It *could* be in a probe() hook for standard plugins as well, I just didn't happen to implement it that way (yet), because plugin_load_so needs to distinguish between the two anyway. https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L472 3* plugin_new() calls the load() hook registered by pluxies for the given extension. for standard plugins (without proxy) there is a predefined plugin_load_so() funtion that gets called instead. How does the load hook get defined for new types of plugins? Via the new API function plugin_register_proxy(). https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1601 4* The load-hook calls geany_plugin_register(), here Geany core and proxies work the same way Where is the geany_plugin_register() defined for a plugin written in a language that isn't C/C++/Vala that can produce a .so file? In the load hook of the pluxy. Either the pluxy calls it directly or it decides to provide a suitable binding so that the non-C script can call it itself, but it has to be during the execution of the load hook. The demopluxy does it in the load() hook, right after parsing the metadata of the example plugin (I completely made up a fake plugin format for demonstration purposes): https://github.com/kugel-/geany/blob/pluxy/plugins/demopluxy.c#L169 I have done it the same way for peasy too, because libpeas plugins can be python or js, and I didn't create bindings yet. But it shows working this way. https://github.com/kugel-/peasy/blob/master/src/peasy.c#L130 I designed it such, that the difference between standard plugins and proxied plugins is all contained in the load hook. The rest of Geany does not know about the difference. This ensures proxied plugins are first class citizens. Thats the correct target I agree, I just don't understand the design details yet. Thanks for the heads up! Best regards ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] My non-C plugin roadmap
On 30 March 2015 at 22:43, Thomas Martitz ku...@rockbox.org wrote: Hello, some if your question are easier to answer by looking at the code, I'll link the appropriate sections. No problem, the explanations below about how things are meant to be used are more help than looking at the code by itself, what is needed is the user guide, which these explanations make a start at, don't lose them :) Am 30.03.2015 um 13:16 schrieb Lex Trotman: [...] Ok, this explains some of what I was asking on the other thread, so now I can ask the more specific questions below that are the key points in the confusion. with my new loader (no pluxies) it goes like this, and this is *very* similar to git master. user opens PM dialog 1 Geany calls load_all_plugins(), which calls load_plugins_from_path($path) 2 for each $file in $path, Geany checks if the extension is G_MODULE_SUFFIX, and calls plugin_new($file, ...) 3 plugin_new() calls the plugins's geany_load_module() (if new-style plugin, calls version_check, set_info() for old-style ones) 4 geany_load_module() is implemented by the plugin, and registers itself with geany_plugin_register() geany_plugin_register() adds the plugin to the plugin list, so that the PM can sort and show it Now, with pluxies, it is completely the same except for: 2* for each $file in $path, Geany calls is_plugin($file) What is is_plugin()? If its a function in Geany how does it get to know about new types of plugins without being hard coded? It knows because the extensions are registered by the function below called by another plugin right? How do you make sure that the register function has been called before you come across a file with that extension? It's new, small helper function I added. It loops through all known file extensions, and returns the first pluxy (a PluginProxy *) for which a) the supported file extension matches and b) the probe hook returned true (or is NULL, for standard plugins). $file is not a plugin if is_plugin returns NULL, i.e. no pluxy was found. https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L844 File extensions and the proxy hooks (probe, load, unload) are registered by a plugin during its init() through the new plugin_register_proxy() function. Ok, so this registers a new type of plugin by its extension(s) and that type is associated with a plugin that provides: - the loader functionality? - interface wrappers/bindings (like geanypy does)? - starts/loads any other things, like the Python interpretor or a JVM or Haskell runtime? Here the pluxy added to the list of registered pluxies. This list is initialized with the simulated pluxy that provides standard plugins (this is not a plugin, it's contained in plugins.c, it's just to keep the code paths equal). https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1601 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) I'm guessing probe() is a function that looks for something in the .so that distinguishes if its new or old loader, but what about others? It depends on the pluxy what the prope() function does! For my peasy pluxy (that provides generic support for libpeas-based plugins), it looks if there is a matching *.plugin for a given *.so, and if yes return a code so that Geany does not attempt to process the .so itself. https://github.com/kugel-/peasy/blob/master/src/peasy.c#L73 There is no probe() for standard plugins, it accepts all .so. If it doesn't call probe how does it know if its a traditional plugin, or a peas one or maybe some other version that makes a .so file? (Haskell anybody :) Whether it's a new or old style plugin is determined later. It *could* be in a probe() hook for standard plugins as well, I just didn't happen to implement it that way (yet), because plugin_load_so needs to distinguish between the two anyway. https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L472 3* plugin_new() calls the load() hook registered by pluxies for the given extension. for standard plugins (without proxy) there is a predefined plugin_load_so() funtion that gets called instead. How does the load hook get defined for new types of plugins? Via the new API function plugin_register_proxy(). https://github.com/kugel-/geany/blob/pluxy/src/plugins.c#L1601 Ok, understand now 4* The load-hook calls geany_plugin_register(), here Geany core and proxies work the same way Where is the geany_plugin_register() defined for a plugin written in a language that isn't C/C++/Vala that can produce a .so file? In the load hook of the pluxy. Either the pluxy calls it directly or it decides to provide a suitable binding so that the non-C script can call it itself, but it has to be during the execution of the load hook. Ok, so if there are all these hooks, probably later the
[Geany-Devel] Placeholder replacement in (build) commands
Hi,
To offload the discussion from PR#441 [1] from this partly off-topic
discussion and give it more visibility, I'm moving it here to the ML.
There already was a thread on the subject that got mostly forgotten, see
[2].
I apologize for the long and complex email, but I don't know how to
present that in a more concise way without losing important information
or clarity.
Note: I will use *NIX-style command lines as examples. The ones for
Windows are slightly different, but the problematic is basically the
same (though we don't use the shell there so it's a bit simpler).
1. So, what are we talking about?
Some of our commands can contain placeholders. The most important ones
are the build commands, but also e.g. the terminal tool has '%c'. These
placeholders are replaced by Geany with various things, like file names,
paths, line numbers etc.
Some of the commands are executed by a shell (build commands) and some
aren't (terminal command).
2. What is the problem?
The replacement for these placeholders can be anything, so they might
contain characters that have a meaning in a command. The most obvious
example is quotes: imagine a file named `foo bar.c`. If we just
replace the placeholder with the raw value (as we currently do) in a
build command, e.g. `gcc -c -o %e.o %f`, it gives this:
gcc -c -o foo bar.o foo bar.c
which is obviously incorrect (see [1] or [2] if it's not obvious for you ;)
We need to escape (or quote) replacements in some way or another.
3. So, how can we fix this?
Several solutions have been suggested (well, actually 3.5 is new!), each
with pros and cons:
3.1. Insert quoted replacement as needed where they appear
This is what I first implemented: track the quotes in the user command,
close them before inserting a quoted/escaped replacement, and reopen it
right after. With the above command, it would give this:
gcc -c -o 'foo bar'.o 'foo bar.c'
which is valid and as we want it (again, see [1] and [2] if it's not
clear that it's valid).
This is what the patch at [3] (and [2], but the version there is
outdated) does.
3.1.1. Pros
* compatible with any current user commands (no breakage of the user
configuration, and fixes replacement in them);
* the user doesn't have to worry about the issue.
3.1.2. Cons
* requires understanding of the shell quoting rules (including sub-shell
like `` and $()), which might be non-trivial.
* if it gets something wrong, the users are mostly screwed (the only
thing they could do would be try to adapt the command in a way for Geany
to get it right)
3.2. Insert quoted replacements everywhere blindly
Replace blindly each placeholder with an escaped/quoted version of
itself. This is like 3.1 but doesn't try to manage quotes in the input
command. It would give:
gcc -c -o 'foo bar'.o 'foo bar.c'
(which is incorrect, see the cons below)
3.2.1. Pros
* easy to implement;
* simple, the user can easily know exactly what happens.
3.2.2. Cons:
* Incompatible with (some) current user commands: as the placeholder is
replaced without care, the placeholders need to appear outside other
quotes. E.g. the above example would have to be altered to remove
quoting around the %f placeholder not to be quoted twice: `gcc -c -o
%e.o %f`.
3.3. Provide format modifier performing specific quoting
Dimitar suggested introducing format modifiers like %f or %'f that
would quote in various ways (see
https://github.com/geany/geany/pull/441#issuecomment-87272057).
3.3.1. Pros
* The users can quote in various ways as they see fit;
* Explicit control on how things are quoted/escaped;
* Could support recursive quoting (quoting twice or even more), so Lex
could use it in a `python -c` or similar called by the shell ;) e.g.
`python -c print(%\f.replace(' ', '_'))` could end up in `python -c
print(foo\ \\\bar.c.replace(' ', '_'))` -- assuming we implement
the escaping Python requires. happy? :)
3.3.2. Cons
* it doesn't fix existing user commands (needs to make use of the new
format modifiers);
* requires to implement various kind of quoting (which requires knowing
several shell escaping rules);
* complex and uncommon format modifiers (the users have to pick the
right one, which might not be obvious);
* the users can screw themselves up if they don't use the appropriate
format (could work with some replacement but not others).
3.4. Parse command as an argument vector and replace in each argument
Instead of working on the command itself, use g_shell_parse_argv() and
perform replacements in argv[1:] (each argument separately).
3.4.1. Pros
* no need for quoting or escaping the replacement (as each argument is
done on its own).
3.4.2. Cons
* doesn't work with shell constructs, so it's not a solution for build
commands (as an argv is an argv, which can't support sub-shell, piping
and whatnot).
3.5. Use environment variables, not placeholders
Change the whole logic and use the environment to pass what currently
uses placeholders.
Re: [Geany-Devel] Placeholder replacement in (build) commands
On 30.3.2015 г. 20:18, Colomban Wendling wrote: To offload the discussion from PR#441 [1] from this partly off-topic discussion and give it more visibility, I'm moving it here to the ML. [...] One thing I forgot to mention is that it would be good to have some kind of OS-variable (single|double) quotes for the static text. Under Unix, one would normally use ' to avoid file name and variable expansion, but under Windows, the normal quotes are double (unless using bash.exe or something). That is, if the user specifies a build or printing command containing, say, «cx$dat.tmp», the Unix text must be single-quoted to avoid the expansion of $dat. But under Win~1, the single quotes are literal. And cx\$dat.tmp won't work either, \ under Win~1 will be literal[1]. Maybe %cx$dat.tmp%, since we already use % as metacharacter? In the case of solution 3.1, 3.1. Insert quoted replacement as needed where they appear the variable quote replacement must be done before any placeholders. [1] Win~1 escaping rules in short: is escaped with \, any literal \-es before a must be duplicated, all other \-es are literal. -- E-gards: Jimmy ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Placeholder replacement in (build) commands
Le 30/03/2015 21:59, Thomas Martitz a écrit : […] Is this a real problem (reported by someone) or just theoretical? Mostly theoretical, although we got a supposedly security-related mail about that issue (ref https://bugs.gentoo.org/show_bug.cgi?id=446986) […] After all, somone naming his files `foo bar.c` should expect to shoot himself in the foot. Agreed, but some people keep playing with guns and complain when they hurt themselves :) So, well, yes to some extent it's an imaginary problem I agree, but the code does have a problem and it stares back at me each time I pass through build.c :) But no, it's not an issue everyone keep complaining about. Cheers, Colomban ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] Placeholder replacement in (build) commands
Am 30.03.2015 um 19:18 schrieb Colomban Wendling: Hi, What do you think? Is this a real problem (reported by someone) or just theoretical? Unless there is actually a real problem caused by this for someone I'd vote for not doing anything. After all, somone naming his files `foo bar.c` should expect to shoot himself in the foot. Seriously, sounds like a possibility for over-engineering a non-existent problem. 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 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?
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
Re: [Geany-Devel] Placeholder replacement in (build) commands
Le 31/03/2015 02:10, Lex Trotman a écrit : […] Perhaps we should be more explicit in the manual that on *ix build commands are run in the shell and the user is responsible for either quoting the substitutions correctly, […] The user currently *cannot* do it correctly so it works with any possible replacement, that's actually the problem :] ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
