Skip to content

Vte support added to giddy#3

Merged
elementgreen merged 9 commits intoKymorphia:mainfrom
dejlek:dejan/vte
Feb 21, 2025
Merged

Vte support added to giddy#3
elementgreen merged 9 commits intoKymorphia:mainfrom
dejlek:dejan/vte

Conversation

@dejlek
Copy link
Copy Markdown
Contributor

@dejlek dejlek commented Feb 19, 2025

As the title says - this PR adds VTE (GTK4) support to giddy. Once we have Gtk3 added we can also add VTE-GTK3 as well.

This was a good learning exercise for me, and I still consider it a work in process. Have a look and let me know what needs to be modified. I will also write a tiny example application later today and put it somewhere online. Will attach screenshot here so we know it all works as expected.

I will NOT consider this PR ready to be merged until Terminal.spawnAsync() works. At the moment giddy does not generate this method.

Update: I've implemented a workaround that calls wrapped c_vte_terminal_spawn_async and it works:
image

Now the real questions is how do I instruct giddy to inject my own implementation of asyncSpawn() ??

The screenshot above is result of the following (just important part of the Application):

void myAsyncSpawn(Terminal terminal) {
    auto cVteTerminalPtr = terminal.cPtr;

    string workingDirectory = "/home/dejan";
    const(char)* _workingDirectory = workingDirectory.toCString(No.Alloc);

    string[] dArgs = ["/usr/bin/env", "bash"];
    char*[] _tmpargv;
    foreach (s; dArgs)
    _tmpargv ~= s.toCString(No.Alloc);
    _tmpargv ~= null;
    char** _argv = _tmpargv.ptr;

    c_vte_terminal_spawn_async(cast(VteTerminal*)cVteTerminalPtr,
        PtyFlags.Default,
        _workingDirectory,
        _argv,
        null, SpawnFlags.Default, null, null, null, 3600, null, null, null
    );
}

class VteWindow : ApplicationWindow {
    Label label;

    this(Application app) {
        super(app);
        setTitle("Main Window");
        setDefaultSize(600, 800);
        setShowMenubar = false;

        label = new Label("GID:VTE DEMO");
        auto box = new Box(Orientation.Vertical, 0);
        box.append(label);

        // Create the VTE instance
        Terminal terminal = new Terminal();
        myAsyncSpawn(terminal);

        // and add it to the box
        box.append(terminal);

        // Add the box as the child widget to the main window
        this.setChild(box);
    }
}

Full code here: https://codeberg.org/dejan/gid-examples

@Kymorphia
Copy link
Copy Markdown
Collaborator

Kymorphia commented Feb 19, 2025

It looks like vte_terminal_spawn_async has multiple callback functions and closure (void*) data arguments. Seems there is an unintended limitation with gidgen currently where it can only handle one closure data argument, which results in this method being disabled. I'm looking into this now.

One thing to note, you should be able to use the vte_terminal_spawn_async alias without using the underlying c_vte_terminal_spawn_async pointer.

@Kymorphia
Copy link
Copy Markdown
Collaborator

I fixed the issue with gidgen which was causing Pty.spawnAsync, Pty.spawnWithFdsAsync, Terminal.spawnAsync, and Terminal.spawnWithFdsAsync to be disabled. This fixed 7 methods in Gio as well which is an added bonus. For reference there was code to reject callbacks which had multiple anonymous void* arguments, but this was also erroneously being applied to regular methods/functions, which meant any method/function with multiple callbacks each with their own data closure arguments would get disabled.

The remaining methods Vte.Terminal.get_text, Vte.Terminal.get_text_include_trailing_spaces, and Vte.Terminal.get_text_range have a deprecated GArray parameter that uses an opaque structure type which is now supposed to always be passed as null. While the XML could probably be patched just to declare this as a pointer and then pass null from D, probably a better way would be to add a feature to gidgen which provides for the ability to pass a static value to a parameter and leave it out of the binding. This could also be accomplished through a custom implementation of these methods.

The Vte.Terminal.setup-context-menu signal has an EventContext parameter which is an opaque structure (just a pointer). This could probably be easily supported by just passing the pointer. These types of situations probably have troublesome memory management in some scenarios, but I think this case wouldn't have any issues if it was implemented that way.

@dejlek
Copy link
Copy Markdown
Contributor Author

dejlek commented Feb 19, 2025

PERFECT! Will give it a try tomorrow. Once the spawnAsync works the way it should this PR can be considered ready to be merged. Thanks EG!

@Kymorphia
Copy link
Copy Markdown
Collaborator

I managed to get the Vte.Terminal.setup-context-menu signal working (gidgen commit), just by enabling opaque pointer types to be passed to signal callbacks, which will probably work (didn't actually test it). That leaves just the 3 methods with GArray parameters. Looking more closely I see that the methods themselves are marked as deprecated. Maybe they should just be marked as ignore?

@Kymorphia
Copy link
Copy Markdown
Collaborator

Looks like those methods are indeed replaced by other methods. If you agree that disabling those methods makes sense, you could add the following to the defs/Vte.d file:

//# Disable some problematic deprecated methods
//!set class[Terminal].method[get_text][disable] 1
//!set class[Terminal].method[get_text_include_trailing_spaces][disable] 1
//!set class[Terminal].method[get_text_range][disable] 1

That would result in 100% coverage in Vte.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants