[Geany-Devel] Spawn module API

[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