Fallback to cross-platform minimal open when apprt is not available

include/ghostty.h

@@ -664,8 +664,8 @@ typedef struct {
 
 // apprt.action.OpenUrlKind
 typedef enum {
-  GHOSTTY_ACTION_OPEN_URL_KIND_TEXT,
   GHOSTTY_ACTION_OPEN_URL_KIND_UNKNOWN,
+  GHOSTTY_ACTION_OPEN_URL_KIND_TEXT,
 } ghostty_action_open_url_kind_e;
 
 // apprt.action.OpenUrl.C

src/Surface.zig

@@ -3724,11 +3724,7 @@ fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
                 .trim = false,
             });
             defer self.alloc.free(str);
-            _ = try self.rt_app.performAction(
+            try self.openUrl(.{ .kind = .unknown, .url = str });
-                .{ .surface = self },
-                .open_url,
-                .{ .kind = .unknown, .url = str },
-            );
         },
 
         ._open_osc8 => {
@@ -3736,17 +3732,35 @@ fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
                 log.warn("failed to get URI for OSC8 hyperlink", .{});
                 return false;
             };
-            _ = try self.rt_app.performAction(
+            try self.openUrl(.{ .kind = .unknown, .url = uri });
-                .{ .surface = self },
-                .open_url,
-                .{ .kind = .unknown, .url = uri },
-            );
         },
     }
 
     return true;
 }
 
+fn openUrl(
+    self: *Surface,
+    action: apprt.action.OpenUrl,
+) !void {
+    // If the apprt handles it then we're done.
+    if (try self.rt_app.performAction(
+        .{ .surface = self },
+        .open_url,
+        action,
+    )) return;
+
+    // apprt didn't handle it, fallback to our simple cross-platform
+    // URL opener. We log a warning because we want well-behaved
+    // apprts to handle this themselves.
+    log.warn("apprt did not handle open URL action, falling back to default opener", .{});
+    try internal_os.open(
+        self.alloc,
+        action.kind,
+        action.url,
+    );
+}
+
 /// Return the URI for an OSC8 hyperlink at the given position or null
 /// if there is no hyperlink.
 fn osc8URI(self: *Surface, pin: terminal.Pin) ?[]const u8 {
@@ -4965,13 +4979,7 @@ fn writeScreenFile(
             defer self.alloc.free(pathZ);
             try self.rt_surface.setClipboardString(pathZ, .standard, false);
         },
-        .open => {
+        .open => try self.openUrl(.{ .kind = .text, .url = path }),
-            _ = try self.rt_app.performAction(
-                .{ .surface = self },
-                .open_url,
-                .{ .kind = .text, .url = path },
-            );
-        },
         .paste => self.io.queueMessage(try termio.Message.writeReq(
             self.alloc,
             path,

src/apprt/action.zig

@@ -625,28 +625,35 @@ pub const ConfigChange = struct {
     }
 };
 
-/// The type of the data at the URL to open. This is used as a hint to
-/// potentially open the URL in a different way.
-/// Sync with: ghostty_action_open_url_kind_s
-pub const OpenUrlKind = enum(c_int) {
-    text,
-    unknown,
-};
-
 /// Open a URL
 pub const OpenUrl = struct {
     /// The type of data that the URL refers to.
-    kind: OpenUrlKind,
+    kind: Kind,
+
     /// The URL.
     url: []const u8,
 
+    /// The type of the data at the URL to open. This is used as a hint to
+    /// potentially open the URL in a different way.
+    ///
+    /// Sync with: ghostty_action_open_url_kind_e
+    pub const Kind = enum(c_int) {
+        /// The type is unknown. This is the default and apprts should
+        /// open the URL in the most generic way possible. For example,
+        /// on macOS this would be the equivalent of `open` or on Linux
+        /// this would be `xdg-open`.
+        unknown,
+
+        /// The URL is known to be a text file. In this case, the apprt
+        /// should try to open the URL in a text editor or viewer or
+        /// some equivalent, if possible.
+        text,
+    };
+
     // Sync with: ghostty_action_open_url_s
     pub const C = extern struct {
-        /// The type of data that the URL refers to.
+        kind: Kind,
-        kind: OpenUrlKind,
-        /// The URL (not zero terminated).
         url: [*]const u8,
-        /// The number of bytes in the URL.
         len: usize,
     };
 

src/apprt/gtk/App.zig

@@ -1759,12 +1759,18 @@ fn initActions(self: *App) void {
     }
 }
 
-// TODO: use https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.OpenURI.html
 pub fn openUrl(
     app: *App,
     value: apprt.action.OpenUrl,
 ) void {
-    internal_os.open(app.core_app.alloc, value.kind, value.url) catch |err| {
+    // TODO: use https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.OpenURI.html
-        log.warn("unable to open url: {}", .{err});
+
-    };
+    // Fallback to the minimal cross-platform way of opening a URL.
+    // This is always a safe fallback and enables for example Windows
+    // to open URLs (GTK on Windows via WSL is a thing).
+    internal_os.open(
+        app.core_app.alloc,
+        value.kind,
+        value.url,
+    ) catch |err| log.warn("unable to open url: {}", .{err});
 }

src/os/open.zig

@@ -1,9 +1,7 @@
 const std = @import("std");
 const builtin = @import("builtin");
 const Allocator = std.mem.Allocator;
-
 const apprt = @import("../apprt.zig");
-const CircBuf = @import("../datastruct/circ_buf.zig").CircBuf;
 
 const log = std.log.scoped(.@"os-open");
 
@@ -12,41 +10,16 @@ const log = std.log.scoped(.@"os-open");
 /// Any output on stderr is logged as a warning in the application logs.
 /// Output on stdout is ignored. The allocator is used to buffer the
 /// log output and may allocate from another thread.
+///
+/// This function is purposely simple for the sake of providing
+/// some portable way to open URLs. If you are implementing an
+/// apprt for Ghostty, you should consider doing something special-cased
+/// for your platform.
 pub fn open(
     alloc: Allocator,
-    kind: apprt.action.OpenUrlKind,
+    kind: apprt.action.OpenUrl.Kind,
     url: []const u8,
 ) !void {
-    // Make a copy of the URL so that we can use it in the thread without
-    // worrying about it getting freed by other threads.
-    const copy = try alloc.dupe(u8, url);
-    errdefer alloc.free(copy);
-
-    // Run in a thread so that it never blocks the main thread, no matter how
-    // long it takes to execute.
-    const thread = try std.Thread.spawn(.{}, _openThread, .{ alloc, kind, copy });
-
-    // Don't worry about the thread any more.
-    thread.detach();
-}
-
-fn _openThread(
-    alloc: Allocator,
-    kind: apprt.action.OpenUrlKind,
-    url: []const u8,
-) void {
-    _openThreadError(alloc, kind, url) catch |err| {
-        log.warn("error while opening url: {}", .{err});
-    };
-}
-
-fn _openThreadError(
-    alloc: Allocator,
-    kind: apprt.action.OpenUrlKind,
-    url: []const u8,
-) !void {
-    defer alloc.free(url);
-
     var exe: std.process.Child = switch (builtin.os.tag) {
         .linux, .freebsd => .init(
             &.{ "xdg-open", url },
@@ -70,95 +43,43 @@ fn _openThreadError(
         else => @compileError("unsupported OS"),
     };
 
-    // Ignore stdin & stdout, collect the output from stderr.
+    // Pipe stdout/stderr so we can collect output from the command.
     // This must be set before spawning the process.
-    exe.stdin_behavior = .Ignore;
+    exe.stdout_behavior = .Pipe;
-    exe.stdout_behavior = .Ignore;
     exe.stderr_behavior = .Pipe;
 
-    exe.spawn() catch |err| {
+    // Spawn the process on our same thread so we can detect failure
-        switch (err) {
+    // quickly.
-            error.FileNotFound => {
+    try exe.spawn();
-                log.warn("Unable to find {s}. Please install {s} and ensure that it is available on the PATH.", .{
-                    exe.argv[0],
-                    exe.argv[0],
-                });
-            },
-            else => |e| return e,
-        }
-        return;
-    };
 
-    const stderr = exe.stderr orelse {
+    // Create a thread that handles collecting output and reaping
-        log.warn("Unable to access the stderr of the spawned program!", .{});
+    // the process. This is done in a separate thread because SOME
-        return;
+    // open implementations block and some do not. It's easier to just
-    };
+    // spawn a thread to handle this so that we never block.
-
+    const thread = try std.Thread.spawn(.{}, openThread, .{ alloc, exe });
-    var cb = try CircBuf(u8, 0).init(alloc, 50 * 1024);
+    thread.detach();
-    defer cb.deinit(alloc);
-
-    // Read any error output and store it in a circular buffer so that we
-    // get that _last_ 50K of output.
-    while (true) {
-        var buf: [1024]u8 = undefined;
-        const len = try stderr.read(&buf);
-        if (len == 0) break;
-        try cb.appendSlice(buf[0..len]);
 }
 
-    // If we have any stderr output we log it. This makes it easier for users to
+fn openThread(alloc: Allocator, exe_: std.process.Child) !void {
-    // debug why some open commands may not work as expected.
+    // 50 KiB is the default value used by std.process.Child.run and should
-    if (cb.len() > 0) log: {
+    // be enough to get the output we care about.
-        {
+    const output_max_size = 50 * 1024;
-            var it = cb.iterator(.forward);
+
-            while (it.next()) |char| {
+    var stdout: std.ArrayListUnmanaged(u8) = .{};
-                if (std.mem.indexOfScalar(u8, &std.ascii.whitespace, char.*)) |_| continue;
+    var stderr: std.ArrayListUnmanaged(u8) = .{};
-                break;
+    defer {
-            }
+        stdout.deinit(alloc);
-            // it's all whitespace, don't log
+        stderr.deinit(alloc);
-            break :log;
-        }
-        var buf = std.ArrayList(u8).init(alloc);
-        defer buf.deinit();
-        var it = cb.iterator(.forward);
-        while (it.next()) |char| {
-            if (char.* == '\n') {
-                log.err("{s} stderr: {s}", .{ exe.argv[0], buf.items });
-                buf.clearRetainingCapacity();
-            }
-            try buf.append(char.*);
-        }
-        if (buf.items.len > 0)
-            log.err("{s} stderr: {s}", .{buf.items});
     }
 
-    const rc = exe.wait() catch |err| {
+    // Copy the exe so it is non-const. This is necessary because wait()
-        switch (err) {
+    // requires a mutable reference and we can't have one as a thread
-            error.FileNotFound => {
+    // param.
-                log.warn("Unable to find {s}. Please install {s} and ensure that it is available on the PATH.", .{
+    var exe = exe_;
-                    exe.argv[0],
+    try exe.collectOutput(alloc, &stdout, &stderr, output_max_size);
-                    exe.argv[0],
+    _ = try exe.wait();
-                });
-            },
-            else => |e| return e,
-        }
-        return;
-    };
 
-    switch (rc) {
+    // If we have any stderr output we log it. This makes it easier for
-        .Exited => |code| {
+    // users to debug why some open commands may not work as expected.
-            if (code != 0) {
+    if (stderr.items.len > 0) log.warn("wait stderr={s}", .{stderr.items});
-                log.warn("{s} exited with error code {d}", .{ exe.argv[0], code });
-            }
-        },
-        .Signal => |signal| {
-            log.warn("{s} was terminaled with signal {}", .{ exe.argv[0], signal });
-        },
-        .Stopped => |signal| {
-            log.warn("{s} was stopped with signal {}", .{ exe.argv[0], signal });
-        },
-        .Unknown => |code| {
-            log.warn("{s} had an unknown error {}", .{ exe.argv[0], code });
-        },
-    }
 }