[Geany-Devel] Spawn module API

[Lex Trotman]

On 28 June 2015 at 05:46, Dimitar Zhekov <dimitar.zhekov at gmail.com> 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: <gdb-executable>, "--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 at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel