jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

Bindings to vte terminal? #77

Closed codic12 closed 3 years ago

codic12 commented 3 years ago

For creating terminal-emulators, https://developer.gnome.org/vte/unstable/VteTerminal.html is very useful, and support for it in crystal-gobject would be nice (perhaps it exists already, but a couple ripgrep's can't find it :/)

jhass commented 3 years ago

Did you try how far you get with just require_gobject "Vte"?

codic12 commented 3 years ago

Ah no, I didn't know about that; is require_gobject exported outside the library?

jhass commented 3 years ago

Sorry, I don't understand the question.

codic12 commented 3 years ago

Can require_gobject be used by the user, or is it strictly library-side?

jhass commented 3 years ago

Let me quote the from the readme:

However the main entry point is the require_gobject macro

codic12 commented 3 years ago

ah, thank you!

codic12 commented 3 years ago

I've made some progress;

require "gobject/gtk/autorun"
require_gobject "Vte"

window = Gtk::Window.new(title: "Terminal test!", border_width: 10)
window.connect "destroy", &->Gtk.main_quit
term = Vte::Terminal.new()
#term.feed_child("abc".to_slice)
window.add term
window.show_all

works as intended. but if I uncomment the term.feed_child, I get Error: undefined method 'to_unsafe' for Nil (compile-time type is (Slice(UInt8) | Nil)) (reproducible: https://play.crystal-lang.org/#/r/9q6n). The error occurs on this autogenerated line:

 > 1124 |   LibVte.terminal_feed_child(@pointer.as(LibVte::Terminal*), text ? text : nil, Int64.new(length))

(under the text ? text : nil). A fix for my example: (text ? text : nil).try &.to_unsafe, but I don't know how this would be applied when to_unsafe isn't explicitly called.

codic12 commented 3 years ago

also, i tried to port this:

    vte_terminal_spawn_async(VTE_TERMINAL(terminal),
        VTE_PTY_DEFAULT,
        NULL,       /* working directory  */
        "ls",    /* command */
        NULL,       /* environment */
        0,          /* spawn flags */
        NULL, NULL, /* child setup */
        NULL,       /* child pid */
        -1,         /* timeout */
        NULL, NULL, NULL);

as such:

term.spawn_async(
  Vte::Pty::Default,
  nil,
  "ls",
  nil,
  0,
  nil, nil,
  nil,
  -1,
  nil, nil, nil
)

But Vte::Pty::Default (nor Vte::Pty::Flags::Default, which I assumed because it was in the VtePtyFlags enum at https://developer.gnome.org/vte/unstable/vte-Vte-PTY.html) doesn't exist

codic12 commented 3 years ago

Ah, the parameter was Vte::PtyFlags::Default; now running into Error: no overload matches 'Vte::Terminal#spawn_async' with types Vte::PtyFlags, Nil, String, Nil, Int32, Nil, Nil, Nil, Int32, Nil, Nil, Nil. The generated signature is:

Vte::Terminal#spawn_async(pty_flags : Vte::PtyFlags, working_directory : ::String | ::Nil, argv : ::Enumerable, envv : ::Enumerable | ::Nil, spawn_flags_ : GLib::SpawnFlags, child_setup : GLib::SpawnChildSetupFunc | ::Nil, child_setup_data : ::Pointer(Void) | ::Nil, child_setup_data_destroy : GLib::DestroyNotify, timeout : ::Int, cancellable : Gio::Cancellable | ::Nil, callback : Vte::TerminalSpawnAsyncCallback | ::Nil, user_data : ::Pointer(Void) | ::Nil)

and the original:

void
vte_terminal_spawn_async (VteTerminal *terminal,
                          VtePtyFlags pty_flags,
                          const char *working_directory,
                          char **argv,
                          char **envv,
                          GSpawnFlags spawn_flags_,
                          GSpawnChildSetupFunc child_setup,
                          gpointer child_setup_data,
                          GDestroyNotify child_setup_data_destroy,
                          int timeout,
                          GCancellable *cancellable,
                          VteTerminalSpawnAsyncCallback callback,
                          gpointer user_data);
jhass commented 3 years ago

I get Error: undefined method 'to_unsafe' for Nil (compile-time type is (Slice(UInt8) | Nil))

IMO a compiler bug, it shouldn't try to call to_unsafe implicitly on nil. Anyways, I pushed a workaround for this.

jhass commented 3 years ago

The following call works for me (but throws an assertion, maybe for some missing setup or parameters?):

 term.spawn_async(
  :default,                    # pty_flags : Vte::PtyFlags,
  nil,                         #  working_directory : ::String | ::Nil,
  ["ls"],                      #  argv : ::Enumerable,
  nil,                         #  envv : ::Enumerable | ::Nil,
  :default,                    #  spawn_flags_ : GLib::SpawnFlags,
  nil,                         # child_setup : GLib::SpawnChildSetupFunc | ::Nil,
  nil,                         # child_setup_data : ::Pointer(Void) | ::Nil,
  GLib::DestroyNotify.new { }, # child_setup_data_destroy : GLib::DestroyNotify,
  -1,                          # timeout : ::Int,
  nil,                         # cancellable : Gio::Cancellable | ::Nil,
  nil,                         # callback : Vte::TerminalSpawnAsyncCallback | ::Nil,
  nil                          # user_data : ::Pointer(Void) | ::Nil
)

argv is an array of char*, so you need to pass an array (or any enumerable).

child_setup_data_destroy is documented to be nullable but unfortunately not annotated as such (upstream is generally open to patches fixing such IME), so we have to pass a dummy for now. This shard currently ships some convenience helpers for Gtk etc, but eventually that stuff should move to their own shards that depend on this one. So I'm fully supportive of a shard fixing this with a manual override of the generated bindings for cases like this. For example one for cairo already exists.

Looking at signatures like this with so many optional parameters makes we wonder if the generator should generate these with a default nil (foo : Bar? = nil). But then that means reordering arguments and probably forcing them to keyword arguments, also not too pretty :/

codic12 commented 3 years ago

Yeah, it compiles but throws an assertion at runtime ((test:7264): VTE-CRITICAL **: 10:00:09.631: void vte_terminal_spawn_async(VteTerminal*, VtePtyFlags, const char*, char**, char**, GSpawnFlags, GSpawnChildSetupFunc, gpointer, GDestroyNotif y, int, GCancellable*, VteTerminalSpawnAsyncCallback, gpointer): assertion '!child_setup_data_destroy || child_setup_data' failed. I don't think there was any missing setup, the original C code here was:

#include <vte/vte.h>
#include <gdk/gdk.h>

int
main(int argc, char *argv[])
{
    GtkWidget *window, *terminal;

    /* Initialise GTK, the window and the terminal */
    gtk_init(&argc, &argv);
    terminal = vte_terminal_new();
    vte_terminal_set_font_scale(terminal, 1.5);
    PangoFontDescription *font = pango_font_description_from_string("Ubuntu Mono"); // works without setting the font so this isn't the issue
    vte_terminal_set_font(terminal, font);
    window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
    gtk_window_set_title(GTK_WINDOW(window), "Terminal");

    vte_terminal_spawn_async(VTE_TERMINAL(terminal),
        VTE_PTY_DEFAULT,
        NULL,       /* working directory  */
        "ls",    /* command */
        NULL,       /* environment */
        0,          /* spawn flags */
        NULL, NULL, /* child setup */
        NULL,       /* child pid */
        -1,         /* timeout */
        NULL, NULL, NULL);

    /* Connect some signals */
    g_signal_connect(window, "delete-event", gtk_main_quit, NULL);
    g_signal_connect(terminal, "child-exited", gtk_main_quit, NULL);

    /* Put widgets together and run the main loop */
    gtk_container_add(GTK_CONTAINER(window), terminal);
    gtk_widget_show_all(window);
    gtk_main();
}

also, the term.feed_child still does not compile; same error as before (and I did update, even removed lib/ and set branch:master to be safe)

jhass commented 3 years ago

Please set branch: main, you should be getting a warning to that extend :)

Given it needs passing a dummy for child_setup_data_destroy and the assertion mentions child_setup_data_destroy and child_setup_data, maybe it needs a dummy for child_setup_data too. Or you override the generated wrapper with one that allows nil for child_setup_data_destroy

codic12 commented 3 years ago

Ah, my bad; although I'm still getting the feed_child error... as for spawn_async, the assertion specifically seems to be that it only runs if there is either NO child_setup_data_destroy (it isn't nullable so this isn't possible with crystal) OR child_setup_data is passed. child_setup_data seems to be a ::Pointer(Void), so I have no idea how I pass in a dummy callback here...

jhass commented 3 years ago

Try passing pointerof(something).as(Void*), it doesn't need to be callback.

I literally used your term_feed example code above with the line commented in to fix the bug :/

codic12 commented 3 years ago
term.spawn_async(
  :default,                    # pty_flags : Vte::PtyFlags,
  nil,                         #  working_directory : ::String | ::Nil,
  ["ls"],                      #  argv : ::Enumerable,
  nil,                         #  envv : ::Enumerable | ::Nil,
  :default,                    #  spawn_flags_ : GLib::SpawnFlags,
  nil,                         # child_setup : GLib::SpawnChildSetupFunc | ::Nil,
  pointerof(term).as(Void*),                       # child_setup_data : ::Pointer(Void) | ::Nil,
  GLib::DestroyNotify.new { }, # child_setup_data_destroy : GLib::DestroyNotify,
  -1,                          # timeout : ::Int,
  nil,                         # cancellable : Gio::Cancellable | ::Nil,
  nil,                         # callback : Vte::TerminalSpawnAsyncCallback | ::Nil,
  nil                          # user_data : ::Pointer(Void) | ::Nil
)

still throws the assertion. And it's this exact code:

require "gobject/gtk/autorun"
require_gobject "Vte"

window = Gtk::Window.new(title: "Terminal test!", border_width: 10)
window.connect "destroy", &->Gtk.main_quit
term = Vte::Terminal.new()
term.feed_child("abc".to_slice)
window.add term
window.show_all

Even removed lib/ and set branch: main to be safe...

jhass commented 3 years ago

Maybe your shard.lock still points at an old version? Try shards update, it should be saying gobject (0.9.1 at 99b0960).

I guess if the dummy doesn't work, then it's the manual override to make the callback argument optional. If you clone this repo and run src/generator/build_namespace.cr Vte > Vte.cr inside, you should get the generated binding code to use as a starting point.

jhass commented 3 years ago

Something like

class Vte::Terminal
  def spawn_async(
    pty_flags : Vte::PtyFlags,
    working_directory : ::String?,
    argv : ::Enumerable,
    envv : ::Enumerable?,
    spawn_flags_ : GLib::SpawnFlags,
    child_setup : GLib::SpawnChildSetupFunc?,
    child_setup_data : Void*?,
    child_setup_data_destroy : GLib::DestroyNotify?,
    timeout : ::Int, cancellable : Gio::Cancellable?,
    callback : Vte::TerminalSpawnAsyncCallback?,
    user_data : Void*?
  )
    LibVte.terminal_spawn_async(
      @pointer.as(LibVte::Terminal*),
      pty_flags,
      working_directory ? working_directory.to_unsafe : nil,
      (__argv_ary = argv.map { |__item| __item.to_unsafe }.to_a).to_unsafe,
      envv ? (__envv_ary = envv.map { |__item| __item.to_unsafe }.to_a).to_unsafe : nil,
      spawn_flags_,
      child_setup ? child_setup : nil,
      child_setup_data ? child_setup_data : nil,
      child_setup_data_destroy ? child_setup_data_destroy : nil,
      Int32.new(timeout),
      cancellable ? cancellable.to_unsafe_cancellable : nil,
      callback ? callback : nil,
      user_data ? user_data : nil)
  end
end
codic12 commented 3 years ago

ahh yes, shard.lock was the issue. works now :+1: That function signature does allow child_setup_data to be a nullable, no? anyways, giving it a Void pointer (pointerof(term).as(Void*)) still throws the assertion so that is weird...

jhass commented 3 years ago

Yeah, no idea why it does still assert with two dummies. Using the above override and passing nil for both parameters gets rid of the assertion for me, not that much is happening after.

hugopl commented 3 years ago

Not sure what version you guys are using but here on vte 0.62.1 argv needs to be a NULL terminated array of NULL terminated strings, in C if I use just { "/bin/sh" } it segfaults.

#include <gtk/gtk.h>
#include <vte/vte.h>

int main(int argc, char *argv[]) {
  gtk_init(&argc, &argv);
  GtkWidget* wnd = gtk_window_new(0);
  g_signal_connect(wnd, "destroy", G_CALLBACK(gtk_main_quit), wnd);

  VteTerminal* vte = VTE_TERMINAL(vte_terminal_new());
  gtk_container_add(GTK_CONTAINER(wnd), GTK_WIDGET(vte));

  char* cmd[] = {"/bin/sh", NULL};
  vte_terminal_spawn_async(vte,
                           VTE_PTY_DEFAULT,
                           NULL, // working dir
                           cmd,  // argv
                           NULL, // envv
                           G_SPAWN_DEFAULT,
                           NULL, // child_setup
                           NULL, // child_setup_data
                           NULL, // child_setup_data_destroy
                           -1,   // timeout
                           NULL, // cancelable
                           NULL, // callback
                           NULL  // userdata);
  gtk_widget_show_all(GTK_WIDGET(wnd));
  gtk_main();
}
hugopl commented 3 years ago

I sent the patch upstream.. if this can be called a patch :-P https://gitlab.gnome.org/GNOME/vte/-/issues/293

hugopl commented 3 years ago

I think this issue can be closed, it also works for me modifying the spawn_async call to accept nil on child_data_destroy parameter and this will be fixed in next VTE release. I'm only in afraid about the argv parameter that have the annotation array zero-terminated=1 and I think it is not crashing on Crystal version just by coincidence, since I got crashes on C code when not using a null terminated array.

jhass commented 3 years ago

It's probably worth to do a little shard that hides this ugly API for something nice :)

codic12 commented 3 years ago

Sorry for the very delayed reply, but indeed the patch works, thank you!