Re: [Geany-Devel] Spawn module API

2015-06-29 Thread Dimitar Zhekov

On 27.6.2015 г. 23:50, Lex Trotman wrote:

On 28 June 2015 at 05:46, Dimitar Zhekov  wrote:

On 27.6.2015 г. 04:40, Lex Trotman wrote:


spawn_write_data



This one looks "platform independent", but it's actually very easy to create
a stdin-write function that blocks under Windows...


Better document it then :)


from spawn.c:853:
"The @a stdin_cb may write to @c channel only once per invocation, only 
if @c G_IO_OUT is set, and only a non-zero number of characters."


Though I haven't tested (or don't remember) whether writing more than 4K 
under Win~1 may block... If that's the case, your stdin_cb will better 
be identical to spawn_write_data.



spawn_get_exit_status_callback


I understood it as a convenient default callback, hardly worth not
exporting it if everybody is going to define the same one-liner
themselves.  And I'm sure you will document it so nobody is confused
:)


and from spawn.c:853:
"Convinience @c GChildWatchFunc callback that copies the child exit 
status into a gint pointed by @a exit_status."


It took me a considerable time to solve a seemingly simple task, so it 
would have been a shame not to document everything. :)


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-28 Thread Thomas Martitz

Am 28.06.2015 um 05:14 schrieb Matthew Brush:

On 2015-06-27 07:54 PM, Lex Trotman wrote:

On 28 June 2015 at 12:40, Matthew Brush  wrote:

On 2015-06-27 12:46 PM, Dimitar Zhekov wrote:


[...]

An updated list of the API-s asked to remain public:

meWIF*
lexspawn_get_program_name
lexspawn_check_command
me/lexspawn_kill_process
  spawn_async_with_pipes
lexspawn_async
me/lexspawn_with_callbacks
mespawn_write_data
lex?spawn_get_exit_status_cb
lexspawn_sync



We should only export what you have concrete plans to use during the 
next
cycle. If Lex wants update plugins during the next cycle to use 
other parts
of the API, we can expose them at the time that the PR is made. No 
point to

speculatively expose API that nobody has immediate plans to use.


If no API is available then nobody will have any plans so no API is
needed so nobody will have any plans for the egg so no chicken is
needed so 



It can easily be made available on-demand, as normal.

We should only be concerned about what Scope is requesting as it has 
concrete plans to use them during the next development cycle and 
doesn't want the plugin to depend on the next development version of 
Geany during that time[0]. There's no reason to speculatively rush 
other APIs in if no plugins have immediate plans to use them nor have 
the concern about depending on the development version of Geany.




Depending on a development version of Geany can be a concern.

I don't want to judge on the specific spawn_* APIs. But why not just 
release with the APIs being available (linkable by plugins) without 
already setting them officially in stone. In other words, find a method 
to mark these APIs experimental so that we can more easily change or 
remove them. This experimental flag should be set for at least one 
release cycle (but perhaps more depending on the situation). Then only 
plugin developers that are prepared to adapt their plugins quickly can 
use them (likely the same plugin developers who pushed the API into 
Geany for their plugins in the first place). Arguably, this should be 
the default process for any new API (IMO).


Best regards
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-27 Thread Matthew Brush

On 2015-06-27 07:54 PM, Lex Trotman wrote:

On 28 June 2015 at 12:40, Matthew Brush  wrote:

On 2015-06-27 12:46 PM, Dimitar Zhekov wrote:


[...]

An updated list of the API-s asked to remain public:

meWIF*
lexspawn_get_program_name
lexspawn_check_command
me/lexspawn_kill_process
  spawn_async_with_pipes
lexspawn_async
me/lexspawn_with_callbacks
mespawn_write_data
lex?spawn_get_exit_status_cb
lexspawn_sync



We should only export what you have concrete plans to use during the next
cycle. If Lex wants update plugins during the next cycle to use other parts
of the API, we can expose them at the time that the PR is made. No point to
speculatively expose API that nobody has immediate plans to use.


If no API is available then nobody will have any plans so no API is
needed so nobody will have any plans for the egg so no chicken is
needed so 



It can easily be made available on-demand, as normal.

We should only be concerned about what Scope is requesting as it has 
concrete plans to use them during the next development cycle and doesn't 
want the plugin to depend on the next development version of Geany 
during that time[0]. There's no reason to speculatively rush other APIs 
in if no plugins have immediate plans to use them nor have the concern 
about depending on the development version of Geany.


Cheers,
Matthew Brush

[0]: Which IMO is a kind of tenuous reason since most people will use 
the version of Scope that their distro provides or from other releases, 
which for the most part means the next released version of Geany/GP 
anyway. I guess maybe some bleeding edge distros might package it 
in-between releases or something, though.


___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-27 Thread Lex Trotman
On 28 June 2015 at 12:54, Lex Trotman  wrote:
> On 28 June 2015 at 12:40, Matthew Brush  wrote:
>> On 2015-06-27 12:46 PM, Dimitar Zhekov wrote:
>>>
>>> [...]
>>>
>>> An updated list of the API-s asked to remain public:
>>>
>>> meWIF*
>>> lexspawn_get_program_name
>>> lexspawn_check_command
>>> me/lexspawn_kill_process
>>>  spawn_async_with_pipes
>>> lexspawn_async
>>> me/lexspawn_with_callbacks
>>> mespawn_write_data
>>> lex?spawn_get_exit_status_cb
>>> lexspawn_sync
>>>
>>
>> We should only export what you have concrete plans to use during the next
>> cycle. If Lex wants update plugins during the next cycle to use other parts
>> of the API, we can expose them at the time that the PR is made. No point to
>> speculatively expose API that nobody has immediate plans to use.
>
> If no API is available then nobody will have any plans so no API is
> needed so nobody will have any plans for the egg so no chicken is
> needed so 

Or to look at it another way:

"Welcome to Geany 1.25, so that their command parsing is the same as
Geanys, all plugins are recommended to upgrade to the spawn_* API -
that we havn't released."

Cheers
Lex


>
>
>>
>> Cheers,
>> Matthew Brush
>> ___
>> 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] Spawn module API

2015-06-27 Thread Lex Trotman
On 28 June 2015 at 12:40, Matthew Brush  wrote:
> On 2015-06-27 12:46 PM, Dimitar Zhekov wrote:
>>
>> [...]
>>
>> An updated list of the API-s asked to remain public:
>>
>> meWIF*
>> lexspawn_get_program_name
>> lexspawn_check_command
>> me/lexspawn_kill_process
>>  spawn_async_with_pipes
>> lexspawn_async
>> me/lexspawn_with_callbacks
>> mespawn_write_data
>> lex?spawn_get_exit_status_cb
>> lexspawn_sync
>>
>
> We should only export what you have concrete plans to use during the next
> cycle. If Lex wants update plugins during the next cycle to use other parts
> of the API, we can expose them at the time that the PR is made. No point to
> speculatively expose API that nobody has immediate plans to use.

If no API is available then nobody will have any plans so no API is
needed so nobody will have any plans for the egg so no chicken is
needed so 


>
> Cheers,
> Matthew Brush
> ___
> 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] Spawn module API

2015-06-27 Thread Matthew Brush

On 2015-06-27 12:46 PM, Dimitar Zhekov wrote:

[...]

An updated list of the API-s asked to remain public:

meWIF*
lexspawn_get_program_name
lexspawn_check_command
me/lexspawn_kill_process
 spawn_async_with_pipes
lexspawn_async
me/lexspawn_with_callbacks
mespawn_write_data
lex?spawn_get_exit_status_cb
lexspawn_sync



We should only export what you have concrete plans to use during the 
next cycle. If Lex wants update plugins during the next cycle to use 
other parts of the API, we can expose them at the time that the PR is 
made. No point to speculatively expose API that nobody has immediate 
plans to use.


Cheers,
Matthew Brush
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-27 Thread Lex Trotman
On 28 June 2015 at 05:46, Dimitar Zhekov  wrote:
> On 27.6.2015 г. 04:40, Lex Trotman wrote:
>
>> My concern is that, for a given platform, all commands a user enters
>> into Geany or plugins should use the same meta chars and quoting.
>> [...]
>
>
> Well, after we could not agree on universal syntax, the next best thing
> would be to at least have consistent syntax on each platform.

Yes.

>
>> Many plugins currently do their own command parsing to get an argv and
>> others use g_spawn_command (which is hard wired to unix rules, it uses
>> g_shell_parse_argv).  On windows these plugins will now use different
>> rules to the build commands because the build commands use spawn_* and
>> get platform dependent rules.
>
>
> They will, though not that different. Most important when specifying a
> windows command with the *nix rules is you need to use either \\ or / as
> directory separator. Both will work, Win~1 supports that.

I thought (based on memory of our previous discussions) that there
were differences in quoting as well.  Also though "Windows" might
accept / in paths, will all programs, or will they confuse it with
options?


>
>> These plugins should be encouraged to switch to the same rules as
>> Geany, ie to use spawn_* calls with a cmd not an argv.  That means the
>> appropriate spawn_* API should be available.
>
>
> As of me, they should be encouraged to use the native Windows rules for the
> benefit of the users. Unless we design Geany for *nix users which run Win~1
> from time to time. :)

It seems that way sometimes :)

>
> (I assume be "cmd" you mean "command line", not cmd.exe).

Yes, sorry, ambiguous choice of abbreviation.

>
>> The API we need to expose for this is the API that allows an unparsed
>> command to be used, not versions that only allow argv.
>
>
> All spawn calls support both a CL and an argv. If both are passed, argv is
> appended to CL.
>
>> So the spawn API that is a drop in replacement for g_spawn is only
>> needed where g_spawn doesn't work, and then it should only be used as
>> a temporary workaround __until the plugins move to using the cmd
>> version__.
>
>
> Sometimes it's native to use argv (scope: , "--quiet",
> "--interpreter=mi2"). Having only CL would mean that one needs to create it
> from argv, and we currently have no platform independent mechanism for that
> (it's not hard to write one, but still).

Does the user enter the individual argv elements?  Its fine to use
argv for code generated commands, in fact I would recommend it, let
spawn do whatever quoting it needs, don't have everybody try to code
it wrong themselves.  I am not in any way suggesting that the spawn_*
functions not have argv available, just that it not be used for *user
entered* commands.

>
>> So to me that means:
>>
>> /* platform dependent checking */
>>
>> spawn_get_program_name
>
>
> Do we actually have a use case for this one?..

Hmmm, no, scratch it then.

>
>> spawn_check_command
>>
>> /* platform dependent running */
>>
>> spawn_with_callbacks
>> spawn_async
>> spawn_sync
>>
>> /* associated support functions */
>>
>> spawn_kill_process
>
>
> This one is "platform dependent killing".

Yes, thats why I included it.

>
>> spawn_write_data
>
>
> This one looks "platform independent", but it's actually very easy to create
> a stdin-write function that blocks under Windows...

Better document it then :)

>
>> spawn_get_exit_status_callback
>
>
> If you mean spawn_get_exit_status_cb, that only copies int status into gint
> *exit_status. You can check the status using the WIF* macros. With spawn.h
> included, the basic ones are defined on both *nix and Win~1.

ok.

>
> --
>
> Well, if spawn_sync and spawn_async remain public, for easier plugin
> migration, and the functions that Scope needs remain public, that means only
> spawn_get_program_name, spawn_async_with_pipes and spawn_get_exit_status_cb
> will be hidden.
>
> Personally I'm for making spawn_async_with_pipes static until somebody
> requests it. It's powerful, but very hard to use properly.

Ok, most plugins seem to do fairly simple things.


>
> And spawn_get_exit_status_cb should be static as well. That's just a
> one-liner, and somebody may confuse it with a function that processes the
> child exit status in some way.

I understood it as a convenient default callback, hardly worth not
exporting it if everybody is going to define the same one-liner
themselves.  And I'm sure you will document it so nobody is confused
:)

>
> --
>
> An updated list of the API-s asked to remain public:
>
> me  WIF*
> lex spawn_get_program_name

scratch the above

> lex spawn_check_command
> me/lex  spawn_kill_process
> spawn_async_with_pipes
> lex spawn_async
> me/lex  spawn_with_callbacks
> me  spawn_write_data
> lex?spawn_get_exit_status_cb
> lex spawn_sync


Cheers
Lex

>
>
> --
> E-gards: Jimmy
> ___
> Devel mailing list
> Devel@lists.geany.org
> https://lists.geany.or

Re: [Geany-Devel] Spawn module API

2015-06-27 Thread Dimitar Zhekov

On 27.6.2015 г. 04:40, Lex Trotman wrote:


My concern is that, for a given platform, all commands a user enters
into Geany or plugins should use the same meta chars and quoting.
[...]


Well, after we could not agree on universal syntax, the next best thing 
would be to at least have consistent syntax on each platform.



Many plugins currently do their own command parsing to get an argv and
others use g_spawn_command (which is hard wired to unix rules, it uses
g_shell_parse_argv).  On windows these plugins will now use different
rules to the build commands because the build commands use spawn_* and
get platform dependent rules.


They will, though not that different. Most important when specifying a 
windows command with the *nix rules is you need to use either \\ or / as 
directory separator. Both will work, Win~1 supports that.



These plugins should be encouraged to switch to the same rules as
Geany, ie to use spawn_* calls with a cmd not an argv.  That means the
appropriate spawn_* API should be available.


As of me, they should be encouraged to use the native Windows rules for 
the benefit of the users. Unless we design Geany for *nix users which 
run Win~1 from time to time. :)


(I assume be "cmd" you mean "command line", not cmd.exe).


The API we need to expose for this is the API that allows an unparsed
command to be used, not versions that only allow argv.


All spawn calls support both a CL and an argv. If both are passed, argv 
is appended to CL.



So the spawn API that is a drop in replacement for g_spawn is only
needed where g_spawn doesn't work, and then it should only be used as
a temporary workaround __until the plugins move to using the cmd
version__.


Sometimes it's native to use argv (scope: , "--quiet", 
"--interpreter=mi2"). Having only CL would mean that one needs to create 
it from argv, and we currently have no platform independent mechanism 
for that (it's not hard to write one, but still).



So to me that means:

/* platform dependent checking */

spawn_get_program_name


Do we actually have a use case for this one?..


spawn_check_command

/* platform dependent running */

spawn_with_callbacks
spawn_async
spawn_sync

/* associated support functions */

spawn_kill_process


This one is "platform dependent killing".


spawn_write_data


This one looks "platform independent", but it's actually very easy to 
create a stdin-write function that blocks under Windows...



spawn_get_exit_status_callback


If you mean spawn_get_exit_status_cb, that only copies int status into 
gint *exit_status. You can check the status using the WIF* macros. With 
spawn.h included, the basic ones are defined on both *nix and Win~1.


--

Well, if spawn_sync and spawn_async remain public, for easier plugin 
migration, and the functions that Scope needs remain public, that means 
only spawn_get_program_name, spawn_async_with_pipes and 
spawn_get_exit_status_cb will be hidden.


Personally I'm for making spawn_async_with_pipes static until somebody 
requests it. It's powerful, but very hard to use properly.


And spawn_get_exit_status_cb should be static as well. That's just a 
one-liner, and somebody may confuse it with a function that processes 
the child exit status in some way.


--

An updated list of the API-s asked to remain public:

me  WIF*
lex spawn_get_program_name
lex spawn_check_command
me/lex  spawn_kill_process
spawn_async_with_pipes
lex spawn_async
me/lex  spawn_with_callbacks
me  spawn_write_data
lex?spawn_get_exit_status_cb
lex spawn_sync

--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-26 Thread Lex Trotman
On 27 June 2015 at 03:13, Dimitar Zhekov  wrote:
> On 26.6.2015 г. 01:47, Lex Trotman wrote:
>
>> Hi Dimitar,
>
>
> Hi, Lex.
>
>> On 26 June 2015 at 03:13, Dimitar Zhekov wrote:
>>>
>>>
>>> It uses quoting internally [...bla-bla-bla unneeded tech details...]
>>
>>
>> I'm now totally confused, so let me ask the important question directly:
>
>
> I should have been more terse, sorry. :)

Thats ok, maybe I havn't been totally clear about where I see the
"problem" and its impact on the exposed API.

>
>> A user is entering a command into Geany, if the command is going to be
>> run by spawn_* will they enter it the same way as they would if the
>> command is going to be run by g_spawn*?  In all versions,
>> sync/async/pipes or not.
>
>
> Short answer: yes.
>
> (Long answer: spawning under Unix and Windows was never exactly the same.
> When splitting a command line into an argument vector with
> g_shell_parse_argv(), and then running it with g_spawn(), or using
> g_spawn('/bin/sh', command), which are the standard practices under Unix, \
> will work as escape character, and ' as argument quoter, as expected under
> *nix. That doesn't work under Windows, didn't work in 1.23/1.24 either, and
> I haven't checked any earlier versions.)

Your long answer contradicts your short answer by illustrating some
places where they differ. :)

My concern is that, for a given platform, all commands a user enters
into Geany or plugins should use the same meta chars and quoting.  My
concern is that a user should not be faced with the problem that some
commands have to be entered with windows rules and some with unix
rules.

Many plugins currently do their own command parsing to get an argv and
others use g_spawn_command (which is hard wired to unix rules, it uses
g_shell_parse_argv).  On windows these plugins will now use different
rules to the build commands because the build commands use spawn_* and
get platform dependent rules.

These plugins should be encouraged to switch to the same rules as
Geany, ie to use spawn_* calls with a cmd not an argv.  That means the
appropriate spawn_* API should be available.

The API we need to expose for this is the API that allows an unparsed
command to be used, not versions that only allow argv.

So the spawn API that is a drop in replacement for g_spawn is only
needed where g_spawn doesn't work, and then it should only be used as
a temporary workaround until the plugins move to using the cmd
version.

So to me that means:

/* platform dependent checking */

spawn_get_program_name
spawn_check_command

/* platform dependent running */

spawn_with_callbacks
spawn_async
spawn_sync

/* associated support functions */

spawn_kill_process
spawn_write_data
spawn_get_exit_status_callback


Cheers
Lex

>
>
> --
> E-gards: Jimmy
> ___
> 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] Spawn module API

2015-06-26 Thread Dimitar Zhekov

On 24.6.2015 г. 02:05, Colomban Wendling wrote:


BTW, I just noticed that on Windows spawn_async_with_pipes() requires
the GLib main loop to be running if child_pid=NULL (well, it registers a
watch func, not sure what happens if it doesn't execute -- zombie?).
We probably don't care much though.


Fixed child_pid=NULL with PR 536.

Also defined GEANY_API_SYMBOL as empty when compiling the spawn testing 
program.


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-26 Thread Dimitar Zhekov

On 26.6.2015 г. 01:47, Lex Trotman wrote:


Hi Dimitar,


Hi, Lex.


On 26 June 2015 at 03:13, Dimitar Zhekov wrote:


It uses quoting internally [...bla-bla-bla unneeded tech details...]


I'm now totally confused, so let me ask the important question directly:


I should have been more terse, sorry. :)


A user is entering a command into Geany, if the command is going to be
run by spawn_* will they enter it the same way as they would if the
command is going to be run by g_spawn*?  In all versions,
sync/async/pipes or not.


Short answer: yes.

(Long answer: spawning under Unix and Windows was never exactly the 
same. When splitting a command line into an argument vector with 
g_shell_parse_argv(), and then running it with g_spawn(), or using 
g_spawn('/bin/sh', command), which are the standard practices under 
Unix, \ will work as escape character, and ' as argument quoter, as 
expected under *nix. That doesn't work under Windows, didn't work in 
1.23/1.24 either, and I haven't checked any earlier versions.)


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-26 Thread Lex Trotman
On 26 June 2015 at 17:45, Jiří Techet  wrote:
> On Fri, Jun 26, 2015 at 3:31 AM, Lex Trotman  wrote:
>>
>>
>> There is also a custom spawn in geanyctags, I didn't look at what it
>> did differently.
>>
>
> There isn't - I'm using utils_spawn_sync() inside the spawn_cmd() function.

Explains why it had a custom spawn and two utils_spawn_sync() calls :)

>
> Cheers,
>
> Jiri
>
>
> ___
> 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] Spawn module API

2015-06-26 Thread Jiří Techet
On Fri, Jun 26, 2015 at 3:31 AM, Lex Trotman  wrote:

>
> There is also a custom spawn in geanyctags, I didn't look at what it
> did differently.
>
>
There isn't - I'm using utils_spawn_sync() inside the spawn_cmd() function.

Cheers,

Jiri
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-25 Thread Lex Trotman
On 26 June 2015 at 11:11, Matthew Brush  wrote:
> On 2015-06-20 08:12 PM, Matthew Brush wrote:
>>
>> Hi All,
>>
>> I just noticed that the new spawn code exposes almost every single bit
>> of API possible. Do we really want to do that, or should we limit it
>> only to what is currently needed by any plugins? A quick survey of
>> Geany-Plugins shows no usage of any of this yet.
>>
>> IMO, we shouldn't expose anything which is not needed by plugins,
>> especially if it's not related to the plugin API.

On IRC Colomban had me look at what spawning G-P plugins did (I looked
at C code only).

The result is that there are twelve g_spawn* calls which should be
migrated to spawn_* equivalents.  At least for those that use commands
input by the user so the quoting is the same as Geany.

There are five that use utils_spawn (which uses spawn_* internally)
and they also should be migrated if they do their own command
decoding.

The above migrations will allow plugins that do their own decoding of
commands to be simplified and become consistent.

There is also a custom spawn in geanyctags, I didn't look at what it
did differently.

>>
>
> One further thought before we get locked into the exposed API.
>
> Shouldn't all the spawn stuff be in the utils_* namespace? From the plugin
> developer perspective it's just some more utility functions like the ones it
> improves upon already in the utils_ namespace and that seems to be the place
> where we dump all the general purpose convenience functions and stuff that
> makes up for GLib short-comings, and that's effectively what this is.

It was proposed to make spawn a library, that would mean a separate
namespace would be better.

>
> If we wanted to keep the file-wise isolation of the spawn code, we could
> just tweak the Doxygen comments a bit to put the functions under that
> section of the API docs, rather than giving a handful of helper functions
> their own whole "module" (API-wise, ex. "namespace" and docs).

Is there a "misc" section? :)

But otherwise yeah, the utils section of the manual seems ok.

>
> Any opinions?
>
> Cheers,
> Matthew Brush
>
> ___
> 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] Spawn module API

2015-06-25 Thread Matthew Brush

On 2015-06-20 08:12 PM, Matthew Brush wrote:

Hi All,

I just noticed that the new spawn code exposes almost every single bit
of API possible. Do we really want to do that, or should we limit it
only to what is currently needed by any plugins? A quick survey of
Geany-Plugins shows no usage of any of this yet.

IMO, we shouldn't expose anything which is not needed by plugins,
especially if it's not related to the plugin API.



One further thought before we get locked into the exposed API.

Shouldn't all the spawn stuff be in the utils_* namespace? From the 
plugin developer perspective it's just some more utility functions like 
the ones it improves upon already in the utils_ namespace and that seems 
to be the place where we dump all the general purpose convenience 
functions and stuff that makes up for GLib short-comings, and that's 
effectively what this is.


If we wanted to keep the file-wise isolation of the spawn code, we could 
just tweak the Doxygen comments a bit to put the functions under that 
section of the API docs, rather than giving a handful of helper 
functions their own whole "module" (API-wise, ex. "namespace" and docs).


Any opinions?

Cheers,
Matthew Brush
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-25 Thread Lex Trotman
Hi Dimitar,

I'm now totally confused, so let me ask the important question directly:

A user is entering a command into Geany, if the command is going to be
run by spawn_* will they enter it the same way as they would if the
command is going to be run by g_spawn*?  In all versions,
sync/async/pipes or not.

Colomban suggested that plugins can use g_spawn* for some commands
because they "work" for both Windows and Linux, and spawn_* for
others, but this is a bad idea if commands need to be entered
differently, since an end user then needs to know which commands to
enter in which way.

So If the answer is they are different then we need to make Geany (and
any spawning plugins with user entered commands) use all the same
commands on the platform, all spawn_* on Windows and all g_spawn* on
Linux.

Cheers
Lex

On 26 June 2015 at 03:13, Dimitar Zhekov  wrote:
> On 24.6.2015 г. 03:12, Lex Trotman wrote:
>>
>> On 24 June 2015 at 10:06, Colomban Wendling
>> wrote:
>>>
>>> Le 24/06/2015 01:57, Lex Trotman a écrit :

 Colomban,

 Correct me if I'm wrong, but despite my loudly voiced misgivings :)
 doesn't the spawn_* series do command quoting and g_spawn* not?
>
>
> No. It uses quoting only internally, (a) when creating a Windows command
> line from argv, for obvious reasons, and (b) if your Windows CL program name
> contains spaces, to make sure that you won't run "C:\Program.exe" or
> "C:\Program Files\Foo.exe" if your "C:\Program Files\Foo\Bar.exe" does not
> exist. Yes, CreateProcess() will really try them, if you miss the quotes by
> mistake.
>
> [g_]spawn*() via argv produce identical results.
>
>>> Well, those support an additional "command" parameter that indeed is
>>> read in a platform-dependent manner.  (this was a discussion point and
>>> ended like this).
>>>
>>> They however also support argvs just fine, so you can use those too and
>>> they would work the same.  With this, you can also imagine using
>>> g_shell_parse_argv() on all platforms if you like, just like you would
>>> do with g_spawn().
>>
>>
>> Then Geany should be changed to do that too, for all commands (IIRC
>> build commands already do it).
>
>
> Unless one is using \ as escape character or ' for quoting under Windows,
> the commands will work. The default filetypes always contain "%x" and never
> '%x', so they will work.
>
>> Note by "user" I mean the end user, not the plugin programmer.  Having
>> the end user need to know if they should quote commands or not is bad
>> (tm).  If that is already the case then it should be fixed.
>
>
> They only need to quote a %x which may contain spaces, as usual, using
> double quotes, as in the default filetypes. The documentation examples also
> show double quotes, and unless I'm not missing something, we don't guarantee
> that \ and ' will work under Windows like in Unix.
>
> The new spawn is more Windows friendly, as it'll never treat the Windows
> directory separator \ as an escape character, and I think that will benefit
> the end users. The forward slashes will work as well (they work since MS-DOS
> 5.0).
>
> --
> E-gards: Jimmy
>
> ___
> 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] Spawn module API

2015-06-25 Thread Dimitar Zhekov

On 24.6.2015 г. 03:12, Lex Trotman wrote:

On 24 June 2015 at 10:06, Colomban Wendling  wrote:

Le 24/06/2015 01:57, Lex Trotman a écrit :

Colomban,

Correct me if I'm wrong, but despite my loudly voiced misgivings :)
doesn't the spawn_* series do command quoting and g_spawn* not?


No. It uses quoting only internally, (a) when creating a Windows command 
line from argv, for obvious reasons, and (b) if your Windows CL program 
name contains spaces, to make sure that you won't run "C:\Program.exe" 
or "C:\Program Files\Foo.exe" if your "C:\Program Files\Foo\Bar.exe" 
does not exist. Yes, CreateProcess() will really try them, if you miss 
the quotes by mistake.


[g_]spawn*() via argv produce identical results.


Well, those support an additional "command" parameter that indeed is
read in a platform-dependent manner.  (this was a discussion point and
ended like this).

They however also support argvs just fine, so you can use those too and
they would work the same.  With this, you can also imagine using
g_shell_parse_argv() on all platforms if you like, just like you would
do with g_spawn().


Then Geany should be changed to do that too, for all commands (IIRC
build commands already do it).


Unless one is using \ as escape character or ' for quoting under 
Windows, the commands will work. The default filetypes always contain 
"%x" and never '%x', so they will work.



Note by "user" I mean the end user, not the plugin programmer.  Having
the end user need to know if they should quote commands or not is bad
(tm).  If that is already the case then it should be fixed.


They only need to quote a %x which may contain spaces, as usual, using 
double quotes, as in the default filetypes. The documentation examples 
also show double quotes, and unless I'm not missing something, we don't 
guarantee that \ and ' will work under Windows like in Unix.


The new spawn is more Windows friendly, as it'll never treat the Windows 
directory separator \ as an escape character, and I think that will 
benefit the end users. The forward slashes will work as well (they work 
since MS-DOS 5.0).


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-25 Thread Dimitar Zhekov

On 24.6.2015 г. 02:05, Colomban Wendling wrote:


IMO, spawn_with_callbacks() is *the* key API from spawn, what makes it
great.


It is, indeed.


spawn_async_with_pipes() can be nice, but most people probably shouldn't
use them as they have same IO trickery most IO layers have, where you
have to take care of not filling up any pipes (write) or forget to empty
them (read).


It's doccomment specially says "use spawn_with_callbacks() unless you 
need to setup specific event sources". Equivalent to glib 
g_spawn_async_with_pipes - shoot yourself in the foot. I'm not opposed 
to hiding it completely.



BTW, I just noticed that on Windows spawn_async_with_pipes() requires
the GLib main loop to be running if child_pid=NULL (well, it registers a
watch func, not sure what happens if it doesn't execute -- zombie?).
We probably don't care much though.


I should check that, and either document it, or remove support for 
child_pid = NULL - especially if the function is hidden.



spawn_async() doesn't seem so much useful to me, [...]

spawn_sync() is nice because it allows to easily pipe through a process
(send some data to stdin and and read the output).  How often is this
useful for everyone I don't know [...]


I wrote those for completeness, as equivalents to g_spawn_[a]_sync. 
Though the prototypes are different, and spawn_sync supports binary I/O. 
The reasons I mentioned before apply - no helper process, native command 
line, better locale support, and likely improved security.



spawn_get_exit_status_cb() seem useless to the API IMO.  It's a trivial
function currently only used by spawn itself.  It might even be sensible
to make it completely internal.


I'm not against it.


I would have said that spawn_write_data() wasn't really required because
it's relatively simple and not used by Geany outside of spawn, but it's
indeed probably handy in combination with spawn_with_callbacks() to
anyone wanting to feed a simple buffer to stdin.


spawn_write_data() can be used either directly as a callback, or invoked 
by a more complex callback, which would probably be the case with Scope. 
It's indeed simple (if you know that the optimal read size under Windows 
is 2K), but do we need to repeat it, or write something similar, each 
time we want to write something to child's stdin?



spawn_get_program_name() is only used in spawn itself and doesn't seem
to be a strict requirement.


May be handy to get the program name from a command line... but most 
people would probably need spawn_check_command().



spawn_check_command() seem nice to validate a user command before
actually running it, so it might be useful to anyone wanting to run
user-supplied commands through the spawn API, to e.g. check for issues
right when the user defines the command (like how we do it in the custom
commands dialog).


Any plugin that lets you define a command line may benefit from it.
For now, I plan a per-program gdb executable setting in Scope, but 
without any gdb options, and thus don't need it...


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-25 Thread Dimitar Zhekov

On 24.6.2015 г. 01:18, Matthew Brush wrote:

On 2015-06-23 09:04 AM, Dimitar Zhekov wrote:


[...]

 From you're previous email you mentioned the stuff checked with [x] below:

[x] spawn_kill_process()
[x] spawn_with_callbacks()
[x] spawn_write_data()
[x] SpawnFlags
[x] SpawnReadFunc
[x] SpawnWriteData

Is that sufficient, or is there other stuff? I will remove the /** from
anything that is not expected to be needed, if nobody objects.


Well, I can't be 100% sure before the implementation, but since 
everything in spawn is stateless, I can simply copy anything else, 
should need be, as a temporary measure.



About the WIF* macros, those (at present) are not "officially" part of
the plugin API as they have no Doxygen comments


They are documented in POSIX wait [1]. Though, as being defined by Geany 
under Windows, a short Doxygen comment may be needed.


> Well you know my opinion :) [...] I think we all
> also agree it's stupid for plugins to have to copy&paste code that
> exists already or else use an inferior version.

On that note, scope/src/plugme contains functions that I need exported 
from Geany since forever. Especially 
plugme_ui_setup_open_button_callback(), which is defined in several 
plugins under different names with varying quality. IIRC, the original 
Geany functions support Windows native file selection dialogs, but no 
plugin does. "Inferior version"-s.


> regardless if something possibly better[1] like GProcess comes
> available to us, and we don't even use it internally anymore.
> [1]: I have no idea if it's better, just an example.

If it's based on the glib spawning functions, it'll be no better than 
them. And as long as the glib developers want to support 
G_SPAWN_LEAVE_DESCRIPTORS_OPEN under Windows and convert anything to 
UNICODE, the glib functions will not improve.


[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html

--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Lex Trotman
On 24 June 2015 at 10:06, Colomban Wendling  wrote:
> Le 24/06/2015 01:57, Lex Trotman a écrit :
>> Colomban,
>>
>> Correct me if I'm wrong, but despite my loudly voiced misgivings :)
>> doesn't the spawn_* series do command quoting and g_spawn* not?
>>
>> If that the case they should not be mixed on the same platform
>> otherwise the user has to know which commands are run with which
>> function to know if they need to do the quoting themselves.
>
> Well, those support an additional "command" parameter that indeed is
> read in a platform-dependent manner.  (this was a discussion point and
> ended like this).
>
> They however also support argvs just fine, so you can use those too and
> they would work the same.  With this, you can also imagine using
> g_shell_parse_argv() on all platforms if you like, just like you would
> do with g_spawn().

Then Geany should be changed to do that too, for all commands (IIRC
build commands already do it).

Note by "user" I mean the end user, not the plugin programmer.  Having
the end user need to know if they should quote commands or not is bad
(tm).  If that is already the case then it should be fixed.

Cheers
Lex


>
> So well, yes, the user has to know which syntax is needed, but that's
> basically true already.
>
> Cheers,
> Colomban
> ___
> 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] Spawn module API

2015-06-23 Thread Colomban Wendling
Le 24/06/2015 01:57, Lex Trotman a écrit :
> Colomban,
> 
> Correct me if I'm wrong, but despite my loudly voiced misgivings :)
> doesn't the spawn_* series do command quoting and g_spawn* not?
> 
> If that the case they should not be mixed on the same platform
> otherwise the user has to know which commands are run with which
> function to know if they need to do the quoting themselves.

Well, those support an additional "command" parameter that indeed is
read in a platform-dependent manner.  (this was a discussion point and
ended like this).

They however also support argvs just fine, so you can use those too and
they would work the same.  With this, you can also imagine using
g_shell_parse_argv() on all platforms if you like, just like you would
do with g_spawn().

So well, yes, the user has to know which syntax is needed, but that's
basically true already.

Cheers,
Colomban
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Lex Trotman
Colomban,

Correct me if I'm wrong, but despite my loudly voiced misgivings :)
doesn't the spawn_* series do command quoting and g_spawn* not?

If that the case they should not be mixed on the same platform
otherwise the user has to know which commands are run with which
function to know if they need to do the quoting themselves.

Cheers
Lex
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Colomban Wendling
Le 24/06/2015 00:18, Matthew Brush a écrit :
> […]
> 
> I think the general policy is to export stuff on demand as plugins need
> it. Seeing as you wrote the API in question, I'm assuming you know best
> the stuff you will need, so I don't personally see much problem
> preemptively exposing that stuff[0].
> 
> From you're previous email you mentioned the stuff checked with [x] below:
> 
> [ ] spawn_get_program_name()
> [ ] spawn_check_command()
> [x] spawn_kill_process()
> [ ] spawn_async_with_pipes()
> [ ] spawn_async()
> [x] spawn_with_callbacks()
> [x] spawn_write_data()
> [ ] spawn_get_exit_status()
> [ ] spawn_sync()
> 
> [x] SpawnFlags
> [x] SpawnReadFunc
> [x] SpawnWriteData

Seems to make sense, only spawn_write_data() doesn't strike me as
totally required, but I understand.
TLDR version below.

> Is that sufficient, or is there other stuff? I will remove the /** from
> anything that is not expected to be needed, if nobody objects.

IMO, spawn_with_callbacks() is *the* key API from spawn, what makes it
great.

spawn_async_with_pipes() can be nice, but most people probably shouldn't
use them as they have same IO trickery most IO layers have, where you
have to take care of not filling up any pipes (write) or forget to empty
them (read).  The only real difference wit g_spawn_async*() is that it
uses native API on Windows (which indeed solves some issues so is
useful, but that's not my point).
Inside Geany, we can rely on the GLib main loop to be running so anyone
can use the handier spawn_with_callbacks().

BTW, I just noticed that on Windows spawn_async_with_pipes() requires
the GLib main loop to be running if child_pid=NULL (well, it registers a
watch func, not sure what happens if it doesn't execute -- zombie?).
We probably don't care much though.

spawn_async() doesn't seem so much useful to me, as it doesn't (seem) to
have so much advantages over g_spawn_async(), which works okay (as there
is no pipes involved).  Also, it's a thin wrapper around
spawn_async_with_pipes().  I don't know about speed or anything though.

spawn_sync() is nice because it allows to easily pipe through a process
(send some data to stdin and and read the output).  How often is this
useful for everyone I don't know -- but any plugin wanting to call a
filter command might benefit from it.  Also, it's not that hard to
reproduce using spawn_with_callbacks() as that one has SPAWN_SYNC, so
only the communication callbacks have to be implemented.

spawn_get_exit_status_cb() seem useless to the API IMO.  It's a trivial
function currently only used by spawn itself.  It might even be sensible
to make it completely internal.

I would have said that spawn_write_data() wasn't really required because
it's relatively simple and not used by Geany outside of spawn, but it's
indeed probably handy in combination with spawn_with_callbacks() to
anyone wanting to feed a simple buffer to stdin.

spawn_get_program_name() is only used in spawn itself and doesn't seem
to be a strict requirement.

spawn_check_command() seem nice to validate a user command before
actually running it, so it might be useful to anyone wanting to run
user-supplied commands through the spawn API, to e.g. check for issues
right when the user defines the command (like how we do it in the custom
commands dialog).

>>  > We should make Colomban decide :)
>>
>> The leading developers should decide - including you.
>>
> 
> Well you know my opinion :) I think everyone pretty much agrees we
> shouldn't expose stuff unrelated to the plugin API, and I think we all
> also agree it's stupid for plugins to have to copy&paste code that
> exists already or else use an inferior version. I also think everyone
> agrees that a separate utility library would be a good idea.

I'm everyone and all :)

Regards,
Colomban
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Matthew Brush

On 2015-06-23 01:40 PM, Lex Trotman wrote:

On 24 June 2015 at 02:04, Dimitar Zhekov  wrote:

On 23.6.2015 г. 02:25, Matthew Brush wrote:


[...]



One thing I forgot: the plugin API currently exports utils_spawn_[a]sync,
and a few plugins use utils_spawn_sync. These functions were (partially
correct) wrappers around the old glib/win32 spawn, and are now wrappers
around spawn_[a]sync. Someday, in the distant future, they should be
obsoleted...


I get what you're saying but I also
feel uneasy about blanket exporting of APIs with no current users of it,
so we don't know exactly what really needs to be exported.


In a previous post Dimitar has listed the API he requests for his
plugin, so that (at least) should be made available in 1.25.  It is
spurious to make him and therefore any potential windows users wait
for another release cycle.



He listed some of the API that is exposed. There's nothing spurious 
about being cautious about mass-exporting API that nobody even asks for, 
IMO.






Well, with nothing from spawn exported, we can be pretty sure that all
plugins _will_ be using utils_spawn and g_spawn instead. :)


Yes, the argument that nothing uses it is also spurious given the API
only just came into existence.



Nobody made any argument that nobody uses, I just mentioned that nobody 
is using it YET (since they could've, as it's all exported for plugins 
ATM in Git master), so we could therefore unexpose it without causing 
anybody grief. There's nothing spurious going on here.


Cheers,
Matthew Brush

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Matthew Brush

On 2015-06-23 09:04 AM, Dimitar Zhekov wrote:

On 23.6.2015 г. 02:25, Matthew Brush wrote:


[...]


One thing I forgot: the plugin API currently exports
utils_spawn_[a]sync, and a few plugins use utils_spawn_sync. These
functions were (partially correct) wrappers around the old glib/win32
spawn, and are now wrappers around spawn_[a]sync. Someday, in the
distant future, they should be obsoleted...



+1


 > I get what you're saying but I also
 > feel uneasy about blanket exporting of APIs with no current users of it,
 > so we don't know exactly what really needs to be exported.

Well, with nothing from spawn exported, we can be pretty sure that all
plugins _will_ be using utils_spawn and g_spawn instead. :)



I think the general policy is to export stuff on demand as plugins need 
it. Seeing as you wrote the API in question, I'm assuming you know best 
the stuff you will need, so I don't personally see much problem 
preemptively exposing that stuff[0].


From you're previous email you mentioned the stuff checked with [x] below:

[ ] spawn_get_program_name()
[ ] spawn_check_command()
[x] spawn_kill_process()
[ ] spawn_async_with_pipes()
[ ] spawn_async()
[x] spawn_with_callbacks()
[x] spawn_write_data()
[ ] spawn_get_exit_status()
[ ] spawn_sync()

[x] SpawnFlags
[x] SpawnReadFunc
[x] SpawnWriteData

Is that sufficient, or is there other stuff? I will remove the /** from 
anything that is not expected to be needed, if nobody objects.


About the WIF* macros, those (at present) are not "officially" part of 
the plugin API as they have no Doxygen comments, though they will still 
be visible to plugins since they're macros and bypass the linker trick 
we use to hide functions. I think it would be best to add documentation 
to those (and possible define them into the "GEANY_" 'namespace'?), at 
the very least to ensure nobody accidentally moves or hides them. Just 
the other day I tried to move them into spawn.c thinking they were there 
on accident, but then I left them because another .c file uses them so 
they need to be in a (not necessarily public) header.



 > We should make Colomban decide :)

The leading developers should decide - including you.



Well you know my opinion :) I think everyone pretty much agrees we 
shouldn't expose stuff unrelated to the plugin API, and I think we all 
also agree it's stupid for plugins to have to copy&paste code that 
exists already or else use an inferior version. I also think everyone 
agrees that a separate utility library would be a good idea. The problem 
I have is that once this API makes it into the next release, it's 
"forever" stuck inside Geany and we'll have to keep it indefinitely, 
regardless if something possibly better[1] like GProcess comes available 
to us, and we don't even use it internally anymore.


Cheers,
Matthew Brush

[0]: At least until we establish a policy around what belongs in the 
plugin API or not.

[1]: I have no idea if it's better, just an example.
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-23 Thread Lex Trotman
On 24 June 2015 at 02:04, Dimitar Zhekov  wrote:
> On 23.6.2015 г. 02:25, Matthew Brush wrote:
>
>> [...]
>
>
> One thing I forgot: the plugin API currently exports utils_spawn_[a]sync,
> and a few plugins use utils_spawn_sync. These functions were (partially
> correct) wrappers around the old glib/win32 spawn, and are now wrappers
> around spawn_[a]sync. Someday, in the distant future, they should be
> obsoleted...
>
>> I get what you're saying but I also
>> feel uneasy about blanket exporting of APIs with no current users of it,
>> so we don't know exactly what really needs to be exported.

In a previous post Dimitar has listed the API he requests for his
plugin, so that (at least) should be made available in 1.25.  It is
spurious to make him and therefore any potential windows users wait
for another release cycle.


>
> Well, with nothing from spawn exported, we can be pretty sure that all
> plugins _will_ be using utils_spawn and g_spawn instead. :)

Yes, the argument that nothing uses it is also spurious given the API
only just came into existence.

Cheers
Lex

>
>> We should make Colomban decide :)
>
> The leading developers should decide - including you.
>
> --
> E-gards: Jimmy
> ___
> 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] Spawn module API

2015-06-23 Thread Dimitar Zhekov

On 23.6.2015 г. 02:25, Matthew Brush wrote:


[...]


One thing I forgot: the plugin API currently exports 
utils_spawn_[a]sync, and a few plugins use utils_spawn_sync. These 
functions were (partially correct) wrappers around the old glib/win32 
spawn, and are now wrappers around spawn_[a]sync. Someday, in the 
distant future, they should be obsoleted...


> I get what you're saying but I also
> feel uneasy about blanket exporting of APIs with no current users of it,
> so we don't know exactly what really needs to be exported.

Well, with nothing from spawn exported, we can be pretty sure that all 
plugins _will_ be using utils_spawn and g_spawn instead. :)


> We should make Colomban decide :)

The leading developers should decide - including you.

--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-22 Thread Matthew Brush

On 2015-06-22 10:18 AM, Dimitar Zhekov wrote:

On 21.6.2015 г. 20:39, Matthew Brush wrote:

On 2015-06-21 04:25 AM, Dimitar Zhekov wrote:

On 21.6.2015 г. 06:12, Matthew Brush wrote:


[...]


Spawn neither requires not uses anything from Geany (except i18n), and
does not change anything in Geany state, so it's functions are not



Yeah, it almost should be a "separate" library (at least like TM or
MIO). Personally I don't like the tendency of Geany's plugin API to be
getting bloated with stuff that has nothing to do with Geany or plugins,
but rather general purpose stuff making up for shortcomings in GLib, or
general-purpose convenience functions. But I think maybe this is a
conversation for another day.


+1 on that. Any generic purpose functions, not really dependent on
Geany, should be separated in a library, and used by Geany and the
plugins on equal basic. Perhaps in a next version...

Some reasons for the plugins to use the spawn API, in no particular order:

- supports native CL under Win~1 (not \-is-escape *nix parsing)
- does not spawn helper processes (creating a process is costly under
Win~1, though that's only notable for > several)
- handles locale arguments better (the former FiF locale error)
- may provide better security, depending on how glib starts processes

But I will not try to force the API on anybody, and do not want anyone
else to do so. If the reasons are not compelling, so be it.



I might try it with the code-format plugin eventually as it's currently 
incredibly slow to do real-time formatting on Windows, because it spawns 
clang-format _a lot_.



In the next plugins (after this release), Scope will require at least
WIF*, spawn_kill_process, SpawnFlags, SpawnReadFunc,
spawn_with_callbacks, SpawnWriteData and spawn_write_data.
It'll be good if 'debugger' uses them as well...



I figured there would be some use in at least Scope. Do you have any
problem if we make all the API private for now and then as you need it
in Scope (or any other plugins requesting it) we can export precisely
what is needed, at that time?


I want the next Scope to depend on Geany-1.25 stable-release. If the
functions are hidden now, it'll have to depend on Geany-git-date, which
I'd rather avoid...



We should make Colomban decide :)  I get what you're saying but I also 
feel uneasy about blanket exporting of APIs with no current users of it, 
so we don't know exactly what really needs to be exported.


Cheers,
Matthew Brush
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-22 Thread Dimitar Zhekov

On 21.6.2015 г. 20:39, Matthew Brush wrote:

On 2015-06-21 04:25 AM, Dimitar Zhekov wrote:

On 21.6.2015 г. 06:12, Matthew Brush wrote:


[...]


Spawn neither requires not uses anything from Geany (except i18n), and
does not change anything in Geany state, so it's functions are not



Yeah, it almost should be a "separate" library (at least like TM or
MIO). Personally I don't like the tendency of Geany's plugin API to be
getting bloated with stuff that has nothing to do with Geany or plugins,
but rather general purpose stuff making up for shortcomings in GLib, or
general-purpose convenience functions. But I think maybe this is a
conversation for another day.


+1 on that. Any generic purpose functions, not really dependent on 
Geany, should be separated in a library, and used by Geany and the 
plugins on equal basic. Perhaps in a next version...


Some reasons for the plugins to use the spawn API, in no particular order:

- supports native CL under Win~1 (not \-is-escape *nix parsing)
- does not spawn helper processes (creating a process is costly under 
Win~1, though that's only notable for > several)

- handles locale arguments better (the former FiF locale error)
- may provide better security, depending on how glib starts processes

But I will not try to force the API on anybody, and do not want anyone 
else to do so. If the reasons are not compelling, so be it.



In the next plugins (after this release), Scope will require at least
WIF*, spawn_kill_process, SpawnFlags, SpawnReadFunc,
spawn_with_callbacks, SpawnWriteData and spawn_write_data.
It'll be good if 'debugger' uses them as well...



I figured there would be some use in at least Scope. Do you have any
problem if we make all the API private for now and then as you need it
in Scope (or any other plugins requesting it) we can export precisely
what is needed, at that time?


I want the next Scope to depend on Geany-1.25 stable-release. If the 
functions are hidden now, it'll have to depend on Geany-git-date, which 
I'd rather avoid...


[A note to myself: the README for this release should state that Scope 
depends on glib-2.18 now, not only Geany-1.22.]


--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-21 Thread Matthew Brush

On 2015-06-21 04:25 AM, Dimitar Zhekov wrote:

On 21.6.2015 г. 06:12, Matthew Brush wrote:


I just noticed that the new spawn code exposes almost every single bit
of API possible. Do we really want to do that, or should we limit it
only to what is currently needed by any plugins? A quick survey of
Geany-Plugins shows no usage of any of this yet.

IMO, we shouldn't expose anything which is not needed by plugins,
especially if it's not related to the plugin API.


The API is designed not only to ease/fix the spawn pipe I/O, but as a
possible replacement for all glib spawn functions - and these may be
invoked by any plugin. (There is no replacement for g_spawn_close_pid,
which works fine, and for g_spawn_check_exit_status, because it's 2.34+
only - instead, the WIF* macros are defined system-independently.)

Spawn neither requires not uses anything from Geany (except i18n), and
does not change anything in Geany state, so it's functions are not
strictly "Geany" - it could be a separate library just as well.



Yeah, it almost should be a "separate" library (at least like TM or 
MIO). Personally I don't like the tendency of Geany's plugin API to be 
getting bloated with stuff that has nothing to do with Geany or plugins, 
but rather general purpose stuff making up for shortcomings in GLib, or 
general-purpose convenience functions. But I think maybe this is a 
conversation for another day.



In the next plugins (after this release), Scope will require at least
WIF*, spawn_kill_process, SpawnFlags, SpawnReadFunc,
spawn_with_callbacks, SpawnWriteData and spawn_write_data.
It'll be good if 'debugger' uses them as well...



I figured there would be some use in at least Scope. Do you have any 
problem if we make all the API private for now and then as you need it 
in Scope (or any other plugins requesting it) we can export precisely 
what is needed, at that time?


Cheers,
Matthew Brush

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-21 Thread Dimitar Zhekov

On 21.6.2015 г. 06:12, Matthew Brush wrote:


I just noticed that the new spawn code exposes almost every single bit
of API possible. Do we really want to do that, or should we limit it
only to what is currently needed by any plugins? A quick survey of
Geany-Plugins shows no usage of any of this yet.

IMO, we shouldn't expose anything which is not needed by plugins,
especially if it's not related to the plugin API.


The API is designed not only to ease/fix the spawn pipe I/O, but as a 
possible replacement for all glib spawn functions - and these may be 
invoked by any plugin. (There is no replacement for g_spawn_close_pid, 
which works fine, and for g_spawn_check_exit_status, because it's 2.34+ 
only - instead, the WIF* macros are defined system-independently.)


Spawn neither requires not uses anything from Geany (except i18n), and 
does not change anything in Geany state, so it's functions are not 
strictly "Geany" - it could be a separate library just as well.


In the next plugins (after this release), Scope will require at least 
WIF*, spawn_kill_process, SpawnFlags, SpawnReadFunc, 
spawn_with_callbacks, SpawnWriteData and spawn_write_data.

It'll be good if 'debugger' uses them as well...

--
E-gards: Jimmy
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Spawn module API

2015-06-21 Thread Matthew Brush

On 2015-06-20 08:12 PM, Matthew Brush wrote:

Hi All,

I just noticed that the new spawn code exposes almost every single bit
of API possible. Do we really want to do that, or should we limit it
only to what is currently needed by any plugins? A quick survey of
Geany-Plugins shows no usage of any of this yet.

IMO, we shouldn't expose anything which is not needed by plugins,
especially if it's not related to the plugin API.



See: https://github.com/geany/geany/pull/523

Cheers,
Matthew Brush

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel