From cbcb0b795c43453d22d2138ae5edc8deb7fa3727 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 6 Jul 2025 15:10:14 -0700 Subject: [PATCH] Fallback to cross-platform minimal open when apprt is not available --- include/ghostty.h | 2 +- src/Surface.zig | 42 ++++++----- src/apprt/action.zig | 33 +++++---- src/apprt/gtk/App.zig | 14 ++-- src/os/open.zig | 161 +++++++++++------------------------------- 5 files changed, 97 insertions(+), 155 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 16ca21d89..2a4a7fb6e 100644 --- a/include/ghostty.h +++ b/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 diff --git a/src/Surface.zig b/src/Surface.zig index db272dddc..a4a8d46df 100644 --- a/src/Surface.zig +++ b/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( - .{ .surface = self }, - .open_url, - .{ .kind = .unknown, .url = str }, - ); + try self.openUrl(.{ .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( - .{ .surface = self }, - .open_url, - .{ .kind = .unknown, .url = uri }, - ); + try self.openUrl(.{ .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 => { - _ = try self.rt_app.performAction( - .{ .surface = self }, - .open_url, - .{ .kind = .text, .url = path }, - ); - }, + .open => try self.openUrl(.{ .kind = .text, .url = path }), .paste => self.io.queueMessage(try termio.Message.writeReq( self.alloc, path, diff --git a/src/apprt/action.zig b/src/apprt/action.zig index 79f8740d2..1c3c7c72c 100644 --- a/src/apprt/action.zig +++ b/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: OpenUrlKind, - /// The URL (not zero terminated). + kind: Kind, url: [*]const u8, - /// The number of bytes in the URL. len: usize, }; diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index a046291ef..369090ee2 100644 --- a/src/apprt/gtk/App.zig +++ b/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| { - log.warn("unable to open url: {}", .{err}); - }; + // TODO: use https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.OpenURI.html + + // 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}); } diff --git a/src/os/open.zig b/src/os/open.zig index 6841c76ab..9b069c80f 100644 --- a/src/os/open.zig +++ b/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 = .Ignore; + exe.stdout_behavior = .Pipe; exe.stderr_behavior = .Pipe; - exe.spawn() catch |err| { - switch (err) { - error.FileNotFound => { - 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; - }; + // Spawn the process on our same thread so we can detect failure + // quickly. + try exe.spawn(); - const stderr = exe.stderr orelse { - log.warn("Unable to access the stderr of the spawned program!", .{}); - return; - }; - - var cb = try CircBuf(u8, 0).init(alloc, 50 * 1024); - 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 - // debug why some open commands may not work as expected. - if (cb.len() > 0) log: { - { - var it = cb.iterator(.forward); - while (it.next()) |char| { - if (std.mem.indexOfScalar(u8, &std.ascii.whitespace, char.*)) |_| continue; - break; - } - // it's all whitespace, don't log - 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| { - switch (err) { - error.FileNotFound => { - 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; - }; - - switch (rc) { - .Exited => |code| { - if (code != 0) { - 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 }); - }, - } + // Create a thread that handles collecting output and reaping + // the process. This is done in a separate thread because SOME + // 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 }); + thread.detach(); +} + +fn openThread(alloc: Allocator, exe_: std.process.Child) !void { + // 50 KiB is the default value used by std.process.Child.run and should + // be enough to get the output we care about. + const output_max_size = 50 * 1024; + + var stdout: std.ArrayListUnmanaged(u8) = .{}; + var stderr: std.ArrayListUnmanaged(u8) = .{}; + defer { + stdout.deinit(alloc); + stderr.deinit(alloc); + } + + // Copy the exe so it is non-const. This is necessary because wait() + // requires a mutable reference and we can't have one as a thread + // param. + var exe = exe_; + try exe.collectOutput(alloc, &stdout, &stderr, output_max_size); + _ = try exe.wait(); + + // If we have any stderr output we log it. This makes it easier for + // users to debug why some open commands may not work as expected. + if (stderr.items.len > 0) log.warn("wait stderr={s}", .{stderr.items}); }