Vte.Pty.spawn_async() is low-level; you should use Vte.Terminal.spawn_async(). Which somehow does exist in the gir but not the vapi, weird.

I don't see anything wrong with the annotations on the function, so I have no idea why the gir->vapi process makes the wrong API. Please ask the vala experts?

On a side note, to catch these kinds of problems at a somewhat higher rate, could we disallow deprecated vte methods on our two test apps?

It does seem like Vte.Terminal.spawn_async() is the API I want, so then my problem becomes the lack of generated Vala bindings for that method.

If I manually add bindings for the spawn_async() method to a local copy of vte-2.91.vapi, it works perfectly. However, it seems there's no way around maintaining the entire set of bindings if I use that solution, which seems a little fragile.

I have no experience using the GIR->VAPI generator and have no idea why it omits bindings for just this one method. Could it be something to do with the comment above it (for VteTerminalSpawnAsyncCallback) in src/vtegtk.cc not being followed with some kind of code?

Created attachment 362640 [details] [review]
Add metadata for VAPI generation of Terminal.spawn_async

From the Vte-2.91.gir at https://github.com/nemequ/vala-girs/blob/master/gir-1.0/Vte-2.91.gir vte_terminal_spawn_async () has been marked as introspectable="0".
I'm not sure why that is, I've seen that with C functions that use varargs.

This patch tells vapigen to ignore the introspectable="0" attribute.

With this applied, is the vapi it creates actually usable and correct?

(In reply to Christian Persch from comment #5)
> With this applied, is the vapi it creates actually usable and correct?

Sorry, I should have made that clear. I came across this in passing and I only made a visual inspection of the VAPI. Here is the diff between master and the patch applied:

--- vte-2.91.vapi	2017-10-31 18:08:55.389985554 +0000
+++ vte-2.91-with-async.vapi	2017-10-31 15:45:42.561462147 +0000
@@ -135,6 +135,7 @@
 		public void set_scrollback_lines (long lines);
 		public void set_size (long columns, long rows);
 		public void set_word_char_exceptions (string exceptions);
+		public void spawn_async (Vte.PtyFlags pty_flags, string? working_directory, [CCode (array_length = false, array_null_terminated = true)] string[] argv, [CCode (array_length = false, array_null_terminated = true)] string[]? envv, GLib.SpawnFlags spawn_flags_, int timeout, GLib.Cancellable? cancellable, Vte.TerminalSpawnAsyncCallback callback);
 		[Deprecated (since = "0.48")]
 		public bool spawn_sync (Vte.PtyFlags pty_flags, string? working_directory, [CCode (array_length = false, array_null_terminated = true)] string[] argv, [CCode (array_length = false, array_null_terminated = true)] string[]? envv, GLib.SpawnFlags spawn_flags, [CCode (delegate_target_pos = 6.5)] GLib.SpawnChildSetupFunc? child_setup, out GLib.Pid child_pid, GLib.Cancellable? cancellable = null) throws GLib.Error;
 		public void unselect_all ();

So it doesn't break the existing VAPI and the spawn_async method now appears in the VAPI, but I have not written any code to use the newly available method. The reporter seems to be writing code with the binding so it would be useful if they tested this. While it could be committed now, there may be further enhancements needed as the VAPI is used. Also this only attempts to fix this for Vala. With the introspectable="0" attribute it won't be used in other bindings generated from the GIR/typelib, for example Python.

Thanks. I have committed this to master, so it can get the necessary testing.

I think at least part of the problem here is the callback from vte_terminal_spawn_async (). It should ideally be GAsyncReadyCallback, the same as vte_pty_spawn_async (). This is the GIO standard and may well have support in GIR generation as well. Using GTask as the GAsyncResult allows for error conditions, this is probably g_task_return_error (), but untested.

Vala has special async methods that fit around GAsyncReadyCallback. PyGobject may also be expecting GAsyncReadyCallbacks: https://pygobject.readthedocs.io/en/latest/guide/threading.html

(In reply to Al Thomas from comment #8)
> I think at least part of the problem here is the callback from
> vte_terminal_spawn_async (). It should ideally be GAsyncReadyCallback, the
> same as vte_pty_spawn_async (). This is the GIO standard and may well have
> support in GIR generation as well. Using GTask as the GAsyncResult allows
> for error conditions, this is probably g_task_return_error (), but untested.

Unfortunately, I found that GAsyncReadyCallback doesn't mix well with GtkWidget, or anything where the initiator of the async call might have gone away by the time the reply arrives. VteTerminalSpawnAsyncCallback was my attempt to make this API easily used from the UI and hide the details from the caller; and it's not possible to change it now anyway (until the next API/ABI break).

In the meanwhile, has anyone tested that the committed patch does indeed produce the correct code when used?