From 487b1d72ab7512415d3c13a3d356a3916969c984 Mon Sep 17 00:00:00 2001 From: Anthony <42196548+Sheeplet1@users.noreply.github.com> Date: Sun, 27 Jul 2025 17:45:12 +1000 Subject: [PATCH 01/70] cli: add filtering hotkey to list_themes --- src/cli/list_themes.zig | 71 +++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/src/cli/list_themes.zig b/src/cli/list_themes.zig index b85f98445..fe119f0a4 100644 --- a/src/cli/list_themes.zig +++ b/src/cli/list_themes.zig @@ -17,6 +17,8 @@ const zf = @import("zf"); // scroll position for larger lists. const SMALL_LIST_THRESHOLD = 10; +const ColorScheme = enum { all, dark, light }; + pub const Options = struct { /// If true, print the full path to the theme. path: bool = false, @@ -25,7 +27,7 @@ pub const Options = struct { plain: bool = false, /// Specifies the color scheme of the themes to include in the list. - color: enum { all, dark, light } = .all, + color: ColorScheme = .all, pub fn deinit(self: Options) void { _ = self; @@ -146,28 +148,11 @@ pub fn run(gpa_alloc: std.mem.Allocator) !u8 { count += 1; const path = try std.fs.path.join(alloc, &.{ loc.dir, entry.name }); - // if there is no need to filter just append the theme to the list - if (opts.color == .all) { - try themes.append(.{ - .path = path, - .location = loc.location, - .theme = try alloc.dupe(u8, entry.name), - }); - continue; - } - - // otherwise check if the theme should be included based on the provided options - var config = try Config.default(alloc); - defer config.deinit(); - try config.loadFile(config._arena.?.allocator(), path); - - if (shouldIncludeTheme(opts, config)) { - try themes.append(.{ - .path = path, - .location = loc.location, - .theme = try alloc.dupe(u8, entry.name), - }); - } + try themes.append(.{ + .path = path, + .location = loc.location, + .theme = try alloc.dupe(u8, entry.name), + }); }, else => {}, } @@ -182,7 +167,7 @@ pub fn run(gpa_alloc: std.mem.Allocator) !u8 { std.mem.sortUnstable(ThemeListElement, themes.items, {}, ThemeListElement.lessThan); if (tui.can_pretty_print and !opts.plain and std.posix.isatty(std.io.getStdOut().handle)) { - try preview(gpa_alloc, themes.items); + try preview(gpa_alloc, themes.items, opts.color); return 0; } @@ -222,8 +207,9 @@ const Preview = struct { }, color_scheme: vaxis.Color.Scheme, text_input: vaxis.widgets.TextInput, + theme_filter: ColorScheme, - pub fn init(allocator: std.mem.Allocator, themes: []ThemeListElement) !*Preview { + pub fn init(allocator: std.mem.Allocator, themes: []ThemeListElement, colorScheme: ColorScheme) !*Preview { const self = try allocator.create(Preview); self.* = .{ @@ -240,11 +226,10 @@ const Preview = struct { .mode = .normal, .color_scheme = .light, .text_input = vaxis.widgets.TextInput.init(allocator, &self.vx.unicode), + .theme_filter = colorScheme, }; - for (0..themes.len) |i| { - try self.filtered.append(i); - } + try self.updateFiltered(); return self; } @@ -308,6 +293,8 @@ const Preview = struct { self.filtered.clearRetainingCapacity(); + var theme_config = try Config.default(self.allocator); + defer theme_config.deinit(); if (self.text_input.buf.realLength() > 0) { const first_half = self.text_input.buf.firstHalf(); const second_half = self.text_input.buf.secondHalf(); @@ -328,6 +315,9 @@ const Preview = struct { while (it.next()) |token| try tokens.append(token); for (self.themes, 0..) |*theme, i| { + try theme_config.loadFile(theme_config._arena.?.allocator(), theme.path); + if (!shouldIncludeTheme(self.theme_filter, theme_config)) continue; + theme.rank = zf.rank(theme.theme, tokens.items, .{ .to_lower = true, .plain = true, @@ -336,8 +326,11 @@ const Preview = struct { } } else { for (self.themes, 0..) |*theme, i| { - try self.filtered.append(i); - theme.rank = null; + try theme_config.loadFile(theme_config._arena.?.allocator(), theme.path); + if (shouldIncludeTheme(self.theme_filter, theme_config)) { + try self.filtered.append(i); + theme.rank = null; + } } } @@ -438,6 +431,14 @@ const Preview = struct { self.themes[self.filtered.items[self.current]].path, alloc, ); + if (key.matches('f', .{})) { + switch (self.theme_filter) { + .all => self.theme_filter = .dark, + .dark => self.theme_filter = .light, + .light => self.theme_filter = .all, + } + try self.updateFiltered(); + } }, .help => { if (key.matches('q', .{})) @@ -695,6 +696,7 @@ const Preview = struct { const key_help = [_]struct { keys: []const u8, help: []const u8 }{ .{ .keys = "^C, q, ESC", .help = "Quit." }, .{ .keys = "F1, ?, ^H", .help = "Toggle help window." }, + .{ .keys = "f", .help = "Cycle through theme filters." }, .{ .keys = "k, ↑", .help = "Move up 1 theme." }, .{ .keys = "ScrollUp", .help = "Move up 1 theme." }, .{ .keys = "PgUp", .help = "Move up 20 themes." }, @@ -1615,18 +1617,17 @@ fn color(config: Config, palette: usize) vaxis.Color { const lorem_ipsum = @embedFile("lorem_ipsum.txt"); -fn preview(allocator: std.mem.Allocator, themes: []ThemeListElement) !void { - var app = try Preview.init(allocator, themes); +fn preview(allocator: std.mem.Allocator, themes: []ThemeListElement, colorScheme: ColorScheme) !void { + var app = try Preview.init(allocator, themes, colorScheme); defer app.deinit(); try app.run(); } -fn shouldIncludeTheme(opts: Options, theme_config: Config) bool { +fn shouldIncludeTheme(theme_filter: ColorScheme, theme_config: Config) bool { const rf = @as(f32, @floatFromInt(theme_config.background.r)) / 255.0; const gf = @as(f32, @floatFromInt(theme_config.background.g)) / 255.0; const bf = @as(f32, @floatFromInt(theme_config.background.b)) / 255.0; const luminance = 0.2126 * rf + 0.7152 * gf + 0.0722 * bf; const is_dark = luminance < 0.5; - - return (opts.color == .dark and is_dark) or (opts.color == .light and !is_dark); + return (theme_filter == .all) or (theme_filter == .dark and is_dark) or (theme_filter == .light and !is_dark); } From db2984de6e575f6415e61231713ad098704642a5 Mon Sep 17 00:00:00 2001 From: Anthony <42196548+Sheeplet1@users.noreply.github.com> Date: Wed, 30 Jul 2025 14:29:38 +1000 Subject: [PATCH 02/70] cli: update var name --- src/cli/list_themes.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli/list_themes.zig b/src/cli/list_themes.zig index fe119f0a4..f05a689c6 100644 --- a/src/cli/list_themes.zig +++ b/src/cli/list_themes.zig @@ -209,7 +209,7 @@ const Preview = struct { text_input: vaxis.widgets.TextInput, theme_filter: ColorScheme, - pub fn init(allocator: std.mem.Allocator, themes: []ThemeListElement, colorScheme: ColorScheme) !*Preview { + pub fn init(allocator: std.mem.Allocator, themes: []ThemeListElement, theme_filter: ColorScheme) !*Preview { const self = try allocator.create(Preview); self.* = .{ @@ -226,7 +226,7 @@ const Preview = struct { .mode = .normal, .color_scheme = .light, .text_input = vaxis.widgets.TextInput.init(allocator, &self.vx.unicode), - .theme_filter = colorScheme, + .theme_filter = theme_filter, }; try self.updateFiltered(); @@ -1617,8 +1617,8 @@ fn color(config: Config, palette: usize) vaxis.Color { const lorem_ipsum = @embedFile("lorem_ipsum.txt"); -fn preview(allocator: std.mem.Allocator, themes: []ThemeListElement, colorScheme: ColorScheme) !void { - var app = try Preview.init(allocator, themes, colorScheme); +fn preview(allocator: std.mem.Allocator, themes: []ThemeListElement, theme_filter: ColorScheme) !void { + var app = try Preview.init(allocator, themes, theme_filter); defer app.deinit(); try app.run(); } From 933543a0d234528cd0ac8fbea9214e2bd73bf401 Mon Sep 17 00:00:00 2001 From: Luzian Bieri Date: Fri, 15 Aug 2025 23:20:43 +0200 Subject: [PATCH 03/70] refactor: extract clipboard setting logic into `copySelectionToClipboards` function --- src/Surface.zig | 60 +++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 866505717..f45185b94 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1833,6 +1833,32 @@ fn clipboardWrite(self: *const Surface, data: []const u8, loc: apprt.Clipboard) }; } +fn copySelectionToClipboards( + self: *Surface, + sel: terminal.Selection, + clipboards: []const apprt.Clipboard, +) void { + const buf = self.io.terminal.screen.selectionString(self.alloc, .{ + .sel = sel, + .trim = self.config.clipboard_trim_trailing_spaces, + }) catch |err| { + log.err("error reading selection string err={}", .{err}); + return; + }; + defer self.alloc.free(buf); + + for (clipboards) |clipboard| self.rt_surface.setClipboardString( + buf, + clipboard, + false, + ) catch |err| { + log.err( + "error setting clipboard string clipboard={} err={}", + .{ clipboard, err }, + ); + }; +} + /// Set the selection contents. /// /// This must be called with the renderer mutex held. @@ -1850,33 +1876,13 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { const sel = sel_ orelse return; if (prev_) |prev| if (sel.eql(prev)) return; - const buf = self.io.terminal.screen.selectionString(self.alloc, .{ - .sel = sel, - .trim = self.config.clipboard_trim_trailing_spaces, - }) catch |err| { - log.err("error reading selection string err={}", .{err}); - return; - }; - defer self.alloc.free(buf); - - // Set the clipboard. This is not super DRY but it is clear what - // we're doing for each setting without being clever. switch (self.config.copy_on_select) { .false => unreachable, // handled above with an early exit // Both standard and selection clipboards are set. .clipboard => { const clipboards: []const apprt.Clipboard = &.{ .standard, .selection }; - for (clipboards) |clipboard| self.rt_surface.setClipboardString( - buf, - clipboard, - false, - ) catch |err| { - log.err( - "error setting clipboard string clipboard={} err={}", - .{ clipboard, err }, - ); - }; + copySelectionToClipboards(self, sel, clipboards); }, // The selection clipboard is set if supported, otherwise the standard. @@ -1885,17 +1891,7 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { .selection else .standard; - - self.rt_surface.setClipboardString( - buf, - clipboard, - false, - ) catch |err| { - log.err( - "error setting clipboard string clipboard={} err={}", - .{ clipboard, err }, - ); - }; + copySelectionToClipboards(self, sel, &.{clipboard}); }, } } From 324d92ea3147659b10e6289d23eaa4f8c602982e Mon Sep 17 00:00:00 2001 From: mitchellh <1299+mitchellh@users.noreply.github.com> Date: Sun, 17 Aug 2025 00:15:25 +0000 Subject: [PATCH 04/70] deps: Update iTerm2 color schemes --- build.zig.zon | 4 ++-- build.zig.zon.json | 6 +++--- build.zig.zon.nix | 6 +++--- build.zig.zon.txt | 2 +- flatpak/zig-packages.json | 6 +++--- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index 55a693496..49c8d3e66 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -112,8 +112,8 @@ // Other .apple_sdk = .{ .path = "./pkg/apple-sdk" }, .iterm2_themes = .{ - .url = "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/3cbeca99efa10beba24b0efe86331736f09f9ed1.tar.gz", - .hash = "N-V-__8AABemXQQj_VhMpwuOSOiSzywW_yGD6aEL9YGI9uBu", + .url = "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/8b639f0c2605557bd23ba1b940842c67bbfd4ed0.tar.gz", + .hash = "N-V-__8AAAlgXwSghpDmXBXZM4Rpd80WKOXVWTrcL0ucVmls", .lazy = true, }, }, diff --git a/build.zig.zon.json b/build.zig.zon.json index 24f1053ba..ad295c883 100644 --- a/build.zig.zon.json +++ b/build.zig.zon.json @@ -49,10 +49,10 @@ "url": "https://deps.files.ghostty.org/imgui-1220bc6b9daceaf7c8c60f3c3998058045ba0c5c5f48ae255ff97776d9cd8bfc6402.tar.gz", "hash": "sha256-oF/QHgTPEat4Hig4fGIdLkIPHmBEyOJ6JeYD6pnveGA=" }, - "N-V-__8AABemXQQj_VhMpwuOSOiSzywW_yGD6aEL9YGI9uBu": { + "N-V-__8AAAlgXwSghpDmXBXZM4Rpd80WKOXVWTrcL0ucVmls": { "name": "iterm2_themes", - "url": "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/3cbeca99efa10beba24b0efe86331736f09f9ed1.tar.gz", - "hash": "sha256-gl42NOZ59ok+umHCHbdBQhWCgFVpj5PAZDVGhJRpbiA=" + "url": "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/8b639f0c2605557bd23ba1b940842c67bbfd4ed0.tar.gz", + "hash": "sha256-PySWF/9IAK4DZCkd5FRpiaIl6et2Qm6t8IKCTzh/Xa0=" }, "N-V-__8AAIC5lwAVPJJzxnCAahSvZTIlG-HhtOvnM1uh-66x": { "name": "jetbrains_mono", diff --git a/build.zig.zon.nix b/build.zig.zon.nix index 380bafaeb..a0184924f 100644 --- a/build.zig.zon.nix +++ b/build.zig.zon.nix @@ -162,11 +162,11 @@ in }; } { - name = "N-V-__8AABemXQQj_VhMpwuOSOiSzywW_yGD6aEL9YGI9uBu"; + name = "N-V-__8AAAlgXwSghpDmXBXZM4Rpd80WKOXVWTrcL0ucVmls"; path = fetchZigArtifact { name = "iterm2_themes"; - url = "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/3cbeca99efa10beba24b0efe86331736f09f9ed1.tar.gz"; - hash = "sha256-gl42NOZ59ok+umHCHbdBQhWCgFVpj5PAZDVGhJRpbiA="; + url = "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/8b639f0c2605557bd23ba1b940842c67bbfd4ed0.tar.gz"; + hash = "sha256-PySWF/9IAK4DZCkd5FRpiaIl6et2Qm6t8IKCTzh/Xa0="; }; } { diff --git a/build.zig.zon.txt b/build.zig.zon.txt index 14bb0e8df..f1114952f 100644 --- a/build.zig.zon.txt +++ b/build.zig.zon.txt @@ -28,7 +28,7 @@ https://deps.files.ghostty.org/zig_js-12205a66d423259567764fa0fc60c82be35365c21a https://deps.files.ghostty.org/ziglyph-b89d43d1e3fb01b6074bc1f7fc980324b04d26a5.tar.gz https://deps.files.ghostty.org/zlib-1220fed0c74e1019b3ee29edae2051788b080cd96e90d56836eea857b0b966742efb.tar.gz https://github.com/jcollie/ghostty-gobject/releases/download/0.14.1-2025-08-09-37-1/ghostty-gobject-0.14.1-2025-08-09-37-1.tar.zst -https://github.com/mbadolato/iTerm2-Color-Schemes/archive/3cbeca99efa10beba24b0efe86331736f09f9ed1.tar.gz +https://github.com/mbadolato/iTerm2-Color-Schemes/archive/8b639f0c2605557bd23ba1b940842c67bbfd4ed0.tar.gz https://github.com/mitchellh/libxev/archive/7f803181b158a10fec8619f793e3b4df515566cb.tar.gz https://github.com/mitchellh/zig-objc/archive/c9e917a4e15a983b672ca779c7985d738a2d517c.tar.gz https://github.com/natecraddock/zf/archive/7aacbe6d155d64d15937ca95ca6c014905eb531f.tar.gz diff --git a/flatpak/zig-packages.json b/flatpak/zig-packages.json index d50371f5f..ce90daffd 100644 --- a/flatpak/zig-packages.json +++ b/flatpak/zig-packages.json @@ -61,9 +61,9 @@ }, { "type": "archive", - "url": "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/3cbeca99efa10beba24b0efe86331736f09f9ed1.tar.gz", - "dest": "vendor/p/N-V-__8AABemXQQj_VhMpwuOSOiSzywW_yGD6aEL9YGI9uBu", - "sha256": "825e3634e679f6893eba61c21db7414215828055698f93c06435468494696e20" + "url": "https://github.com/mbadolato/iTerm2-Color-Schemes/archive/8b639f0c2605557bd23ba1b940842c67bbfd4ed0.tar.gz", + "dest": "vendor/p/N-V-__8AAAlgXwSghpDmXBXZM4Rpd80WKOXVWTrcL0ucVmls", + "sha256": "3f249617ff4800ae0364291de4546989a225e9eb76426eadf082824f387f5dad" }, { "type": "archive", From 7f8d215955844d91b29ac4016d7379f5d228f3c6 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sun, 17 Aug 2025 21:32:39 -0500 Subject: [PATCH 05/70] gtk-ng: fix race condition when checking border bell feature Fixes #8266 When a surface is first created, there's a race condition between when the config is set on the surface and when the code to check if a border should be drawn around the surface is run. Fix that by exiting early if the bell isn't ringing, before we check to see if there's a config set on the surface and issuing the warning message. --- src/apprt/gtk-ng/class/surface.zig | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/apprt/gtk-ng/class/surface.zig b/src/apprt/gtk-ng/class/surface.zig index 580436bd3..b412d6b6b 100644 --- a/src/apprt/gtk-ng/class/surface.zig +++ b/src/apprt/gtk-ng/class/surface.zig @@ -554,13 +554,21 @@ pub const Surface = extern struct { config_: ?*Config, bell_ringing_: c_int, ) callconv(.c) c_int { + const bell_ringing = bell_ringing_ != 0; + + // If the bell isn't ringing exit early because when the surface is + // first created there's a race between this code being run and the + // config being set on the surface. That way we don't overwhelm people + // with the warning that we issue if the config isn't set and overwhelm + // ourselves with large numbers of bug reports. + if (!bell_ringing) return @intFromBool(false); + const config = if (config_) |v| v.get() else { - log.warn("config unavailable for computing whether border should be shown , likely bug", .{}); + log.warn("config unavailable for computing whether border should be shown, likely bug", .{}); return @intFromBool(false); }; - const bell_ringing = bell_ringing_ != 0; - return @intFromBool(config.@"bell-features".border and bell_ringing); + return @intFromBool(config.@"bell-features".border); } pub fn toggleFullscreen(self: *Self) void { From 38e69b2e969a98f4f1a4e94f1cc474f66312ed6e Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 14 Aug 2025 17:43:13 -0500 Subject: [PATCH 06/70] gtk-ng: use virtual methods to draw the inspector Insead of signals between the ImGui widget and the Inspector widget, make the Inspector widget a subclass of the ImGui widget and use virtual methods to handle setup and rendering of the Inspector. --- src/apprt/gtk-ng/class.zig | 12 +++ src/apprt/gtk-ng/class/imgui_widget.zig | 88 ++++++++++++++++---- src/apprt/gtk-ng/class/inspector_widget.zig | 53 +++++------- src/apprt/gtk-ng/ui/1.5/inspector-widget.blp | 10 +-- 4 files changed, 109 insertions(+), 54 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index 68879d19c..e8f7289a8 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -53,6 +53,18 @@ pub fn Common( } }).private else {}; + /// Get the class structure for the object. + /// + /// This seems ugly and unsafe to me but this is what GObject is doing + /// under the hood. + /// + /// https://gitlab.gnome.org/GNOME/glib/-/blob/main/gobject/gtype.h?ref_type=heads#L555-571 + /// https://gitlab.gnome.org/GNOME/glib/-/blob/main/gobject/gtype.h?ref_type=heads#L2673 + pub fn getClass(self: *Self) ?*Self.Class { + const type_instance: *gobject.TypeInstance = @ptrCast(self); + return @ptrCast(type_instance.f_g_class orelse return null); + } + /// A helper that creates a property that reads and writes a /// private field with only shallow copies. This is good for primitives /// such as bools, numbers, etc. diff --git a/src/apprt/gtk-ng/class/imgui_widget.zig b/src/apprt/gtk-ng/class/imgui_widget.zig index 1522f2bc1..499018add 100644 --- a/src/apprt/gtk-ng/class/imgui_widget.zig +++ b/src/apprt/gtk-ng/class/imgui_widget.zig @@ -67,6 +67,34 @@ pub const ImguiWidget = extern struct { }; }; + pub const virtual_methods = struct { + /// This virtual method will be called to allow the Dear ImGui + /// application to do one-time setup of the context. The correct context + /// will be current when the virtual method is called. + pub const setup = struct { + pub fn call(class: anytype, object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) void { + return gobject.ext.as(Self.Class, class).setup.?(gobject.ext.as(Self, object)); + } + + pub fn implement(class: anytype, implementation: *const fn (p_object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) callconv(.c) void) void { + gobject.ext.as(Self.Class, class).setup = @ptrCast(implementation); + } + }; + + /// This virtual method will be called at each frame to allow the Dear + /// ImGui application to draw the application. The correct context will + /// be current when the virtual method is called. + pub const render = struct { + pub fn call(class: anytype, object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) void { + return gobject.ext.as(Self.Class, class).render.?(gobject.ext.as(Self, object)); + } + + pub fn implement(class: anytype, implementation: *const fn (p_object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) callconv(.c) void) void { + gobject.ext.as(Self.Class, class).render = @ptrCast(implementation); + } + }; + }; + const Private = struct { /// GL area where we display the Dear ImGui application. gl_area: *gtk.GLArea, @@ -113,6 +141,25 @@ pub const ImguiWidget = extern struct { priv.gl_area.queueRender(); } + //--------------------------------------------------------------- + // Public wrappers for virtual methods + + /// This virtual method will be called to allow the Dear ImGui application + /// to do one-time setup of the context. The correct context will be current + /// when the virtual method is called. + pub fn setup(self: *Self) callconv(.c) void { + const class = self.getClass() orelse return; + virtual_methods.setup.call(class, self); + } + + /// This virtual method will be called at each frame to allow the Dear ImGui + /// application to draw the application. The correct context will be current + /// when the virtual method is called. + pub fn render(self: *Self) callconv(.c) void { + const class = self.getClass() orelse return; + virtual_methods.render.call(class, self); + } + //--------------------------------------------------------------- // Private Methods @@ -232,13 +279,8 @@ pub const ImguiWidget = extern struct { // initialize the ImgUI OpenGL backend for our context. _ = cimgui.ImGui_ImplOpenGL3_Init(null); - // Setup our app - signals.setup.impl.emit( - self, - null, - .{}, - null, - ); + // Call the virtual method to setup the UI. + self.setup(); } /// Handle a request to unrealize the GLArea @@ -279,13 +321,8 @@ pub const ImguiWidget = extern struct { self.newFrame(); cimgui.c.igNewFrame(); - // Use the callback to draw the UI. - signals.render.impl.emit( - self, - null, - .{}, - null, - ); + // Call the virtual method to draw the UI. + self.render(); // Render cimgui.c.igRender(); @@ -422,15 +459,34 @@ pub const ImguiWidget = extern struct { cimgui.c.ImGuiIO_AddInputCharactersUTF8(io, bytes); } + //--------------------------------------------------------------- + // Default virtual method handlers + + /// Default setup function. Does nothing but log a warning. + fn defaultSetup(_: *Self) callconv(.c) void { + log.warn("default Dear ImGui setup called, this is a bug.", .{}); + } + + /// Default render function. Does nothing but log a warning. + fn defaultRender(_: *Self) callconv(.c) void { + log.warn("default Dear ImGui render called, this is a bug.", .{}); + } + const C = Common(Self, Private); pub const as = C.as; pub const ref = C.ref; pub const refSink = C.refSink; pub const unref = C.unref; + pub const getClass = C.getClass; const private = C.private; pub const Class = extern struct { parent_class: Parent.Class, + + /// Function pointers for virtual methods. + setup: ?*const fn (*Self) callconv(.c) void = null, + render: ?*const fn (*Self) callconv(.c) void = null, + var parent: *Parent.Class = undefined; pub const Instance = Self; @@ -444,6 +500,10 @@ pub const ImguiWidget = extern struct { }), ); + // Initialize our virtual methods with default functions. + class.setup = defaultSetup; + class.render = defaultRender; + // Bindings class.bindTemplateChildPrivate("gl_area", .{}); class.bindTemplateChildPrivate("im_context", .{}); diff --git a/src/apprt/gtk-ng/class/inspector_widget.zig b/src/apprt/gtk-ng/class/inspector_widget.zig index f71970a88..4321dcd57 100644 --- a/src/apprt/gtk-ng/class/inspector_widget.zig +++ b/src/apprt/gtk-ng/class/inspector_widget.zig @@ -17,7 +17,7 @@ const log = std.log.scoped(.gtk_ghostty_inspector_widget); pub const InspectorWidget = extern struct { const Self = @This(); parent_instance: Parent, - pub const Parent = adw.Bin; + pub const Parent = ImguiWidget; pub const getGObjectType = gobject.ext.defineClass(Self, .{ .name = "GhosttyInspectorWidget", .instanceInit = &init, @@ -50,9 +50,6 @@ pub const InspectorWidget = extern struct { /// We attach a weak notify to the object. surface: ?*Surface = null, - /// The embedded Dear ImGui widget. - imgui_widget: *ImguiWidget, - pub var offset: c_int = 0; }; @@ -78,13 +75,30 @@ pub const InspectorWidget = extern struct { ); } + /// Called to do initial setup of the UI. + fn imguiSetup( + _: *Self, + ) callconv(.c) void { + Inspector.setup(); + } + + /// Called for every frame to draw the UI. + fn imguiRender( + self: *Self, + ) callconv(.c) void { + const priv = self.private(); + const surface = priv.surface orelse return; + const core_surface = surface.core() orelse return; + const inspector = core_surface.inspector orelse return; + inspector.render(); + } + //--------------------------------------------------------------- // Public methods /// Queue a render of the Dear ImGui widget. pub fn queueRender(self: *Self) void { - const priv = self.private(); - priv.imgui_widget.queueRender(); + self.as(ImguiWidget).queueRender(); } //--------------------------------------------------------------- @@ -189,24 +203,6 @@ pub const InspectorWidget = extern struct { // for completeness sake we should clean this up. } - fn imguiRender( - _: *ImguiWidget, - self: *Self, - ) callconv(.c) void { - const priv = self.private(); - const surface = priv.surface orelse return; - const core_surface = surface.core() orelse return; - const inspector = core_surface.inspector orelse return; - inspector.render(); - } - - fn imguiSetup( - _: *ImguiWidget, - _: *Self, - ) callconv(.c) void { - Inspector.setup(); - } - const C = Common(Self, Private); pub const as = C.as; pub const ref = C.ref; @@ -230,13 +226,6 @@ pub const InspectorWidget = extern struct { }), ); - // Bindings - class.bindTemplateChildPrivate("imgui_widget", .{}); - - // Template callbacks - class.bindTemplateCallback("imgui_render", &imguiRender); - class.bindTemplateCallback("imgui_setup", &imguiSetup); - // Properties gobject.ext.registerProperties(class, &.{ properties.surface.impl, @@ -245,6 +234,8 @@ pub const InspectorWidget = extern struct { // Signals // Virtual methods + ImguiWidget.virtual_methods.setup.implement(class, imguiSetup); + ImguiWidget.virtual_methods.render.implement(class, imguiRender); gobject.Object.virtual_methods.dispose.implement(class, &dispose); } diff --git a/src/apprt/gtk-ng/ui/1.5/inspector-widget.blp b/src/apprt/gtk-ng/ui/1.5/inspector-widget.blp index 985a7ed23..ac15ab4e5 100644 --- a/src/apprt/gtk-ng/ui/1.5/inspector-widget.blp +++ b/src/apprt/gtk-ng/ui/1.5/inspector-widget.blp @@ -1,18 +1,10 @@ using Gtk 4.0; -using Adw 1; -template $GhosttyInspectorWidget: Adw.Bin { +template $GhosttyInspectorWidget: $GhosttyImguiWidget { styles [ "inspector", ] hexpand: true; vexpand: true; - - Adw.Bin { - $GhosttyImguiWidget imgui_widget { - render => $imgui_render(); - setup => $imgui_setup(); - } - } } From f147a89b6839bc6c31d79befbf391375e51b50f9 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 15 Aug 2025 11:01:30 -0500 Subject: [PATCH 07/70] gtk-ng: use gitlab permalinks --- src/apprt/gtk-ng/class.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index e8f7289a8..ee29954cb 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -58,8 +58,8 @@ pub fn Common( /// This seems ugly and unsafe to me but this is what GObject is doing /// under the hood. /// - /// https://gitlab.gnome.org/GNOME/glib/-/blob/main/gobject/gtype.h?ref_type=heads#L555-571 - /// https://gitlab.gnome.org/GNOME/glib/-/blob/main/gobject/gtype.h?ref_type=heads#L2673 + /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L555-571 + /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L2673 pub fn getClass(self: *Self) ?*Self.Class { const type_instance: *gobject.TypeInstance = @ptrCast(self); return @ptrCast(type_instance.f_g_class orelse return null); From dd072d2e01635003da49541489434ddf07b20523 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 16 Aug 2025 00:29:26 -0500 Subject: [PATCH 08/70] gtk-ng: add some initial notes on memory layout of GObjects --- src/apprt/gtk-ng/class.zig | 42 +++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index ee29954cb..b3ea70a37 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -53,13 +53,49 @@ pub fn Common( } }).private else {}; - /// Get the class structure for the object. + /// Get the class for the object. /// - /// This seems ugly and unsafe to me but this is what GObject is doing - /// under the hood. + /// This _seems_ ugly and unsafe but this is what GObject is doing + /// under the hood when it wants to access the class instance: /// /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L555-571 /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L2673 + /// + /// This works because when you create a new object in GObject, the pointer + /// that you get back wasn't allocated using your original type struct. + /// Instead you get the following block of data: + /// + /// + /// ┌─────────────────┐ + /// │ │ + /// │ │ + /// │ Private │ + /// │ │ + /// │ │ + /// │ │ + /// │ │ + /// ┌───►├─────────────────┤ + /// *object────┘ │ │ + /// │ TypeInstance │ + /// │ │ ┌───►┌────────────────┐ + /// │ │ │ │ │ + /// │ │ │ │ Class │ + /// │ *g_class ────┼───┘ │ │ + /// │ │ │ │ + /// └─────────────────┘ │ │ + /// │ │ + /// │ │ + /// └────────────────┘ + /// + /// First is a block of data that holds the instance's private data. That private + /// data is accessed by taking the instance pointer and subtracting the size of the + /// private data to get a pointer to the data. + /// + /// Next is an instance of a TypeInstance, whose purpose is to hold a pointer + /// to the class instance. + /// + /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.c#L1792-1912 + /// pub fn getClass(self: *Self) ?*Self.Class { const type_instance: *gobject.TypeInstance = @ptrCast(self); return @ptrCast(type_instance.f_g_class orelse return null); From 23b3adedc3dfc7c62fd42f3ab467a5ee929bc6a9 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 16 Aug 2025 15:12:07 -0500 Subject: [PATCH 09/70] gtk-ng: remove signals from imgui_widget --- src/apprt/gtk-ng/class/imgui_widget.zig | 38 ++----------------------- 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/src/apprt/gtk-ng/class/imgui_widget.zig b/src/apprt/gtk-ng/class/imgui_widget.zig index 499018add..d9851b4a1 100644 --- a/src/apprt/gtk-ng/class/imgui_widget.zig +++ b/src/apprt/gtk-ng/class/imgui_widget.zig @@ -35,37 +35,7 @@ pub const ImguiWidget = extern struct { pub const properties = struct {}; - pub const signals = struct { - /// Emitted when the child widget should render. During the callback, - /// the Imgui context is valid. - pub const render = struct { - pub const name = "render"; - pub const connect = impl.connect; - const impl = gobject.ext.defineSignal( - name, - Self, - &.{}, - void, - ); - }; - - /// Emitted when first realized to allow the embedded ImGui application - /// to initialize itself. When this is called, the ImGui context - /// is properly set. - /// - /// This might be called multiple times, but each time it is - /// called a new Imgui context will be created. - pub const setup = struct { - pub const name = "setup"; - pub const connect = impl.connect; - const impl = gobject.ext.defineSignal( - name, - Self, - &.{}, - void, - ); - }; - }; + pub const signals = struct {}; pub const virtual_methods = struct { /// This virtual method will be called to allow the Dear ImGui @@ -484,8 +454,8 @@ pub const ImguiWidget = extern struct { parent_class: Parent.Class, /// Function pointers for virtual methods. - setup: ?*const fn (*Self) callconv(.c) void = null, - render: ?*const fn (*Self) callconv(.c) void = null, + setup: ?*const fn (*Self) callconv(.c) void, + render: ?*const fn (*Self) callconv(.c) void, var parent: *Parent.Class = undefined; pub const Instance = Self; @@ -524,8 +494,6 @@ pub const ImguiWidget = extern struct { class.bindTemplateCallback("im_commit", &imCommit); // Signals - signals.render.impl.register(.{}); - signals.setup.impl.register(.{}); // Virtual methods gobject.Object.virtual_methods.dispose.implement(class, &dispose); From 34d10db7ea98e87d9c9ad977c86693b5a4d55aa0 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 16 Aug 2025 16:51:25 -0500 Subject: [PATCH 10/70] gtk-ng: remove some woefully naive musings on GObject memory layout --- src/apprt/gtk-ng/class.zig | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index b3ea70a37..708bc3e60 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -61,41 +61,6 @@ pub fn Common( /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L555-571 /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L2673 /// - /// This works because when you create a new object in GObject, the pointer - /// that you get back wasn't allocated using your original type struct. - /// Instead you get the following block of data: - /// - /// - /// ┌─────────────────┐ - /// │ │ - /// │ │ - /// │ Private │ - /// │ │ - /// │ │ - /// │ │ - /// │ │ - /// ┌───►├─────────────────┤ - /// *object────┘ │ │ - /// │ TypeInstance │ - /// │ │ ┌───►┌────────────────┐ - /// │ │ │ │ │ - /// │ │ │ │ Class │ - /// │ *g_class ────┼───┘ │ │ - /// │ │ │ │ - /// └─────────────────┘ │ │ - /// │ │ - /// │ │ - /// └────────────────┘ - /// - /// First is a block of data that holds the instance's private data. That private - /// data is accessed by taking the instance pointer and subtracting the size of the - /// private data to get a pointer to the data. - /// - /// Next is an instance of a TypeInstance, whose purpose is to hold a pointer - /// to the class instance. - /// - /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.c#L1792-1912 - /// pub fn getClass(self: *Self) ?*Self.Class { const type_instance: *gobject.TypeInstance = @ptrCast(self); return @ptrCast(type_instance.f_g_class orelse return null); From 1693c9a2aca107df208a3bfd82e3218fab88efbc Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 16 Aug 2025 22:59:18 -0500 Subject: [PATCH 11/70] gtk-ng: add some better comments on why getClass works --- src/apprt/gtk-ng/class.zig | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index 708bc3e60..0bfc049f6 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -55,8 +55,29 @@ pub fn Common( /// Get the class for the object. /// - /// This _seems_ ugly and unsafe but this is what GObject is doing - /// under the hood when it wants to access the class instance: + /// This _seems_ ugly and unsafe but this is how GObject + /// works under the hood. From the [GObject Type System + /// Concepts](https://docs.gtk.org/gobject/concepts.html) documentation: + /// + /// Every object must define two structures: its class structure + /// and its instance structure. All class structures must contain + /// as first member a GTypeClass structure. All instance structures + /// must contain as first member a GTypeInstance structure. + /// … + /// These constraints allow the type system to make sure that + /// every object instance (identified by a pointer to the object’s + /// instance structure) contains in its first bytes a pointer to the + /// object’s class structure. + /// … + /// The C standard mandates that the first field of a C structure is + /// stored starting in the first byte of the buffer used to hold the + /// structure’s fields in memory. This means that the first field of + /// an instance of an object B is A’s first field which in turn is + /// GTypeInstance‘s first field which in turn is g_class, a pointer + /// to B’s class structure. + /// + /// This means that to access the class structure for an object you cast it + /// to `*gobject.TypeInstance` and then access the `f_g_class` field. /// /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L555-571 /// https://gitlab.gnome.org/GNOME/glib/-/blob/2c08654b62d52a31c4e4d13d7d85e12b989e72be/gobject/gtype.h#L2673 From 675ba0e9b8b4d7cbbe6855cfb0eace58d8bccbd7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 18 Aug 2025 09:58:47 -0700 Subject: [PATCH 12/70] apprt/gtk-ng: defineVirtualMethod helper --- src/apprt/gtk-ng/class.zig | 72 +++++++++++++++++++++++++ src/apprt/gtk-ng/class/imgui_widget.zig | 24 ++------- 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/apprt/gtk-ng/class.zig b/src/apprt/gtk-ng/class.zig index 0bfc049f6..4b46f8365 100644 --- a/src/apprt/gtk-ng/class.zig +++ b/src/apprt/gtk-ng/class.zig @@ -1,6 +1,7 @@ //! This files contains all the GObject classes for the GTK apprt //! along with helpers to work with them. +const std = @import("std"); const glib = @import("glib"); const gobject = @import("gobject"); const gtk = @import("gtk"); @@ -87,6 +88,77 @@ pub fn Common( return @ptrCast(type_instance.f_g_class orelse return null); } + /// Define a virtual method. The `Self.Class` type must have a field + /// named `name` which is a function pointer in the following form: + /// + /// ?*const fn (*Self) callconv(.c) void + /// + /// The virtual method may take additional parameters and specify + /// a non-void return type. The parameters and return type must be + /// valid for the C calling convention. + pub fn defineVirtualMethod( + comptime name: [:0]const u8, + ) type { + return struct { + pub fn call( + class: anytype, + object: *ClassInstance(@TypeOf(class)), + params: anytype, + ) (fn_info.return_type orelse void) { + const func = @field( + gobject.ext.as(Self.Class, class), + name, + ).?; + @call(.auto, func, .{ + gobject.ext.as(Self, object), + } ++ params); + } + + pub fn implement( + class: anytype, + implementation: *const ImplementFunc(@TypeOf(class)), + ) void { + @field(gobject.ext.as( + Self.Class, + class, + ), name) = @ptrCast(implementation); + } + + /// The type info of the virtual method. + const fn_info = fn_info: { + // This is broken down like this so its slightly more + // readable. We expect a field named "name" on the Class + // with the rough type of `?*const fn` and we need the + // function info. + const Field = @FieldType(Self.Class, name); + const opt = @typeInfo(Field).optional; + const ptr = @typeInfo(opt.child).pointer; + break :fn_info @typeInfo(ptr.child).@"fn"; + }; + + /// The instance type for a class. + fn ClassInstance(comptime T: type) type { + return @typeInfo(T).pointer.child.Instance; + } + + /// The function type for implementations. This is the same type + /// as the virtual method but the self parameter points to the + /// target instead of the original class. + fn ImplementFunc(comptime T: type) type { + var params: [fn_info.params.len]std.builtin.Type.Fn.Param = undefined; + @memcpy(¶ms, fn_info.params); + params[0].type = *ClassInstance(T); + return @Type(.{ .@"fn" = .{ + .calling_convention = fn_info.calling_convention, + .is_generic = fn_info.is_generic, + .is_var_args = fn_info.is_var_args, + .return_type = fn_info.return_type, + .params = ¶ms, + } }); + } + }; + } + /// A helper that creates a property that reads and writes a /// private field with only shallow copies. This is good for primitives /// such as bools, numbers, etc. diff --git a/src/apprt/gtk-ng/class/imgui_widget.zig b/src/apprt/gtk-ng/class/imgui_widget.zig index d9851b4a1..854dec20b 100644 --- a/src/apprt/gtk-ng/class/imgui_widget.zig +++ b/src/apprt/gtk-ng/class/imgui_widget.zig @@ -41,28 +41,12 @@ pub const ImguiWidget = extern struct { /// This virtual method will be called to allow the Dear ImGui /// application to do one-time setup of the context. The correct context /// will be current when the virtual method is called. - pub const setup = struct { - pub fn call(class: anytype, object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) void { - return gobject.ext.as(Self.Class, class).setup.?(gobject.ext.as(Self, object)); - } - - pub fn implement(class: anytype, implementation: *const fn (p_object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) callconv(.c) void) void { - gobject.ext.as(Self.Class, class).setup = @ptrCast(implementation); - } - }; + pub const setup = C.defineVirtualMethod("setup"); /// This virtual method will be called at each frame to allow the Dear /// ImGui application to draw the application. The correct context will /// be current when the virtual method is called. - pub const render = struct { - pub fn call(class: anytype, object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) void { - return gobject.ext.as(Self.Class, class).render.?(gobject.ext.as(Self, object)); - } - - pub fn implement(class: anytype, implementation: *const fn (p_object: *@typeInfo(@TypeOf(class)).pointer.child.Instance) callconv(.c) void) void { - gobject.ext.as(Self.Class, class).render = @ptrCast(implementation); - } - }; + pub const render = C.defineVirtualMethod("render"); }; const Private = struct { @@ -119,7 +103,7 @@ pub const ImguiWidget = extern struct { /// when the virtual method is called. pub fn setup(self: *Self) callconv(.c) void { const class = self.getClass() orelse return; - virtual_methods.setup.call(class, self); + virtual_methods.setup.call(class, self, .{}); } /// This virtual method will be called at each frame to allow the Dear ImGui @@ -127,7 +111,7 @@ pub const ImguiWidget = extern struct { /// when the virtual method is called. pub fn render(self: *Self) callconv(.c) void { const class = self.getClass() orelse return; - virtual_methods.render.call(class, self); + virtual_methods.render.call(class, self, .{}); } //--------------------------------------------------------------- From 058a91d217318b41a6ecc569e07e918975427aa8 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 17:39:46 -0600 Subject: [PATCH 13/70] bitmap_allocator: improve findFreeChunks for spans >64 This allows them to be packed more efficiently, rather than always starting at a bitmap start. --- src/terminal/bitmap_allocator.zig | 84 +++++++++++++++++-------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/src/terminal/bitmap_allocator.zig b/src/terminal/bitmap_allocator.zig index 68d968768..e9d609ff5 100644 --- a/src/terminal/bitmap_allocator.zig +++ b/src/terminal/bitmap_allocator.zig @@ -188,50 +188,56 @@ fn findFreeChunks(bitmaps: []u64, n: usize) ?usize { // I'm not a bit twiddling expert. Perhaps even SIMD could be used here // but unsure. Contributor friendly: let's benchmark and improve this! - // Large chunks require special handling. In this case we look for - // divFloor sequential chunks that are maxInt, then look for the mod - // normally in the next bitmap. + // Large chunks require special handling. if (n > @bitSizeOf(u64)) { - const div = @divFloor(n, @bitSizeOf(u64)); - const mod = n % @bitSizeOf(u64); - var seq: usize = 0; - for (bitmaps, 0..) |*bitmap, idx| { - // If we aren't fully empty then reset the sequence - if (bitmap.* != std.math.maxInt(u64)) { - seq = 0; + var i: usize = 0; + search: while (i < bitmaps.len) { + // Number of chunks available at the end of this bitmap. + const prefix = @clz(~bitmaps[i]); + + // If there are no chunks available at the end of this bitmap + // then we can't start in it, so we'll try the next one. + if (prefix == 0) { + i += 1; continue; } - // If we haven't reached the sequence count we're looking for - // then add one and continue, we're still accumulating blanks. - if (seq != div) { - seq += 1; - if (seq != div or mod > 0) continue; + // Starting position if we manage to find the span we need here. + const start_bitmap = i; + const start_bit = 64 - prefix; + + // The remaining number of sequential free chunks we need to find. + var rem: usize = n - prefix; + + i += 1; + while (rem > 64) : (i += 1) { + // We ran out of bitmaps, there's no sufficiently large gap. + if (i >= bitmaps.len) return null; + + // There's more than 64 remaining chunks and this bitmap has + // content in it, so we try starting again with this bitmap. + if (bitmaps[i] != std.math.maxInt(u64)) continue :search; + + // This bitmap is completely free, we can subtract 64 from + // our remaining number. + rem -= 64; } - // We've reached the seq count see if this has mod starting empty - // blanks. - if (mod > 0) { - const final = @as(u64, std.math.maxInt(u64)) >> @intCast(64 - mod); - if (bitmap.* & final == 0) { - // No blanks, reset. - seq = 0; - continue; - } + // If the number of available chunks at the start of this bitmap + // is less than the remaining required, we have to try again. + if (@ctz(~bitmaps[i]) < rem) continue; - bitmap.* ^= final; + const suffix = (n - prefix) % 64; + + // Found! Mark everything between our start and end as full. + bitmaps[start_bitmap] ^= ~@as(u64, 0) >> @intCast(start_bit) << @intCast(start_bit); + const full_bitmaps = @divFloor(n - prefix - suffix, 64); + for (bitmaps[start_bitmap + 1 ..][0..full_bitmaps]) |*bitmap| { + bitmap.* = 0; } + if (suffix > 0) bitmaps[i] ^= ~@as(u64, 0) >> @intCast(64 - suffix); - // Found! Set all in our sequence to full and mask our final. - // The "zero_mod" modifier below handles the case where we have - // a perfectly divisible number of chunks so we don't have to - // mark the trailing bitmap. - const zero_mod = @intFromBool(mod == 0); - const start_idx = idx - (seq - zero_mod); - const end_idx = idx + zero_mod; - for (start_idx..end_idx) |i| bitmaps[i] = 0; - - return (start_idx * 64); + return start_bitmap * 64 + start_bit; } return null; @@ -349,18 +355,18 @@ test "findFreeChunks larger than 64 chunks not at beginning" { }; const idx = findFreeChunks(&bitmaps, 65).?; try testing.expectEqual( - 0b11111111_00000000_00000000_00000000_00000000_00000000_00000000_00000000, + 0b00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000, bitmaps[0], ); try testing.expectEqual( - 0b00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000, + 0b11111110_00000000_00000000_00000000_00000000_00000000_00000000_00000000, bitmaps[1], ); try testing.expectEqual( - 0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111110, + 0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111, bitmaps[2], ); - try testing.expectEqual(@as(usize, 64), idx); + try testing.expectEqual(@as(usize, 56), idx); } test "findFreeChunks larger than 64 chunks exact" { From 6d7982c8caa8b57fd99ccf8f1c68375bb7e11cc6 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 17:42:34 -0600 Subject: [PATCH 14/70] bitmap_allocator: improve/fix `free` This previously had logic in it that was very wrong and could lead to memory corruption or a failure to properly mark data as freed. Also introduces a bunch of tests for various edge case behavior. --- src/terminal/bitmap_allocator.zig | 500 +++++++++++++++++++++++++++++- 1 file changed, 483 insertions(+), 17 deletions(-) diff --git a/src/terminal/bitmap_allocator.zig b/src/terminal/bitmap_allocator.zig index e9d609ff5..724c71be5 100644 --- a/src/terminal/bitmap_allocator.zig +++ b/src/terminal/bitmap_allocator.zig @@ -108,28 +108,59 @@ pub fn BitmapAllocator(comptime chunk_size: comptime_int) type { const chunks = self.chunks.ptr(base); const chunk_idx = @divExact(@intFromPtr(slice.ptr) - @intFromPtr(chunks), chunk_size); - // From the chunk index, we can find the starting bitmap index - // and the bit within the last bitmap. - var bitmap_idx = @divFloor(chunk_idx, 64); - const bitmap_bit = chunk_idx % 64; const bitmaps = self.bitmap.ptr(base); - // If our chunk count is over 64 then we need to handle the - // case where we have to mark multiple bitmaps. - if (chunk_count > 64) { - const bitmaps_full = @divFloor(chunk_count, 64); - for (0..bitmaps_full) |i| bitmaps[bitmap_idx + i] = std.math.maxInt(u64); - bitmap_idx += bitmaps_full; + // Current bitmap index. + var i: usize = @divFloor(chunk_idx, 64); + // Number of chunks we still have to mark as free. + var rem: usize = chunk_count; + + // Mark any bits in the starting bitmap that need to be marked. + { + // Bit index. + const bit = chunk_idx % 64; + // Number of bits we need to mark in this bitmap. + const bits = @min(rem, 64 - bit); + + bitmaps[i] |= ~@as(u64, 0) >> @intCast(64 - bits) << @intCast(bit); + rem -= bits; } - // Set the bitmap to mark the chunks as free. Note we have to - // do chunk_count % 64 to handle the case where our chunk count - // is using multiple bitmaps. - const bitmap = &bitmaps[bitmap_idx]; - for (0..chunk_count % 64) |i| { - const mask = @as(u64, 1) << @intCast(bitmap_bit + i); - bitmap.* |= mask; + // Mark any full bitmaps worth of bits that need to be marked. + i += 1; + while (rem > 64) : (i += 1) { + bitmaps[i] = std.math.maxInt(u64); + rem -= 64; } + + // Mark any bits at the start of this last bitmap if it needs it. + if (rem > 0) { + bitmaps[i] |= ~@as(u64, 0) >> @intCast(64 - rem); + } + } + + /// For testing only. + fn isAllocated(self: *Self, base: anytype, slice: anytype) bool { + comptime assert(@import("builtin").is_test); + + const bytes = std.mem.sliceAsBytes(slice); + const aligned_len = std.mem.alignForward(usize, bytes.len, chunk_size); + const chunk_count = @divExact(aligned_len, chunk_size); + + const chunks = self.chunks.ptr(base); + const chunk_idx = @divExact(@intFromPtr(slice.ptr) - @intFromPtr(chunks), chunk_size); + + const bitmaps = self.bitmap.ptr(base); + + for (chunk_idx..chunk_idx + chunk_count) |i| { + const bitmap = @divFloor(i, bitmap_bit_size); + const bit = i % bitmap_bit_size; + if (bitmaps[bitmap] & (@as(u64, 1) << @intCast(bit)) != 0) { + return false; + } + } + + return true; } /// For debugging @@ -489,3 +520,438 @@ test "BitmapAllocator alloc large" { ptr[0] = 'A'; bm.free(buf, ptr); } + +test "BitmapAllocator alloc and free one bitmap" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate exactly one bitmap worth of bytes. + const slice = try bm.alloc(u8, buf, Alloc.bitmap_bit_size); + try testing.expectEqual(Alloc.bitmap_bit_size, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([Alloc.bitmap_bit_size]u8, @splat(0x11)), + slice, + ); + + // Free it + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free half bitmap" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate exactly half a bitmap worth of bytes. + const slice = try bm.alloc(u8, buf, Alloc.bitmap_bit_size / 2); + try testing.expectEqual(Alloc.bitmap_bit_size / 2, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + // Free it + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free two half bitmaps" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate exactly one bitmap worth of bytes across two allocations. + const slice = try bm.alloc(u8, buf, Alloc.bitmap_bit_size / 2); + try testing.expectEqual(Alloc.bitmap_bit_size / 2, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + const slice2 = try bm.alloc(u8, buf, Alloc.bitmap_bit_size / 2); + try testing.expectEqual(Alloc.bitmap_bit_size / 2, slice2.len); + + @memset(slice2, 0x22); + try testing.expectEqualSlices( + u8, + &@as([Alloc.bitmap_bit_size / 2]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice2)); + bm.free(buf, slice2); + try testing.expect(!bm.isAllocated(buf, slice2)); + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free 1.5 bitmaps" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate exactly 1.5 bitmaps worth of bytes. + const slice = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free two 1.5 bitmaps" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate exactly 3 bitmaps worth of bytes across two allocations. + const slice = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + const slice2 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice2.len); + + @memset(slice2, 0x22); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice2)); + bm.free(buf, slice2); + try testing.expect(!bm.isAllocated(buf, slice2)); + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free 1.5 bitmaps offset by 0.75" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate three quarters of a bitmap first. + const slice = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 4); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 4, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + // Then a 1.5 bitmap sized allocation, so that it spans + // from 0.75 to 2.25, occupying bits in 3 different bitmaps. + const slice2 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice2.len); + + @memset(slice2, 0x22); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice2)); + bm.free(buf, slice2); + try testing.expect(!bm.isAllocated(buf, slice2)); + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free three 0.75 bitmaps" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 3 bitmaps. + const cap = Alloc.bitmap_bit_size * 3; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // Allocate three quarters of a bitmap three times. + const slice = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 4); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 4, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + const slice2 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 4); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 4, slice2.len); + + @memset(slice2, 0x22); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + const slice3 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 4); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 4, slice3.len); + + @memset(slice3, 0x33); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x33)), + slice3, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice2)); + bm.free(buf, slice2); + try testing.expect(!bm.isAllocated(buf, slice2)); + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + try testing.expect(bm.isAllocated(buf, slice3)); + bm.free(buf, slice3); + try testing.expect(!bm.isAllocated(buf, slice3)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([3]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..3], + ); +} + +test "BitmapAllocator alloc and free two 1.5 bitmaps offset 0.75" { + const Alloc = BitmapAllocator(1); + // Capacity such that we'll have 4 bitmaps. + const cap = Alloc.bitmap_bit_size * 4; + + const testing = std.testing; + const alloc = testing.allocator; + const layout = Alloc.layout(cap); + const buf = try alloc.alignedAlloc(u8, Alloc.base_align, layout.total_size); + defer alloc.free(buf); + + var bm = Alloc.init(.init(buf), layout); + + // First allocate a 0.75 bitmap + const slice = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 4); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 4, slice.len); + + @memset(slice, 0x11); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + // Then two 1.5 bitmaps + const slice2 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice2.len); + + @memset(slice2, 0x22); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + const slice3 = try bm.alloc(u8, buf, 3 * Alloc.bitmap_bit_size / 2); + try testing.expectEqual(3 * Alloc.bitmap_bit_size / 2, slice3.len); + + @memset(slice3, 0x33); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x33)), + slice3, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 2]u8, @splat(0x22)), + slice2, + ); + try testing.expectEqualSlices( + u8, + &@as([3 * Alloc.bitmap_bit_size / 4]u8, @splat(0x11)), + slice, + ); + + // Free them + try testing.expect(bm.isAllocated(buf, slice2)); + bm.free(buf, slice2); + try testing.expect(!bm.isAllocated(buf, slice2)); + try testing.expect(bm.isAllocated(buf, slice)); + bm.free(buf, slice); + try testing.expect(!bm.isAllocated(buf, slice)); + try testing.expect(bm.isAllocated(buf, slice3)); + bm.free(buf, slice3); + try testing.expect(!bm.isAllocated(buf, slice3)); + + // All of our bitmaps should be free. + try testing.expectEqualSlices( + u64, + &@as([4]u64, @splat(~@as(u64, 0))), + bm.bitmap.ptr(buf)[0..4], + ); +} From 61fc290ad195f411136e0da55b92299b1b014ed1 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 18:24:57 -0600 Subject: [PATCH 15/70] test(PageList): add failing test for reflow hyperlink OOM --- src/terminal/PageList.zig | 95 +++++++++++++++++++++++++++++++++++++++ src/terminal/page.zig | 2 +- 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index a7563ac8c..cfc791ce5 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -7029,6 +7029,7 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon try testing.expect(!cells[3].hasText()); } } + test "PageList resize reflow more cols cursor in wrapped row" { const testing = std.testing; const alloc = testing.allocator; @@ -7222,6 +7223,100 @@ test "PageList resize reflow more cols no reflow preserves semantic prompt" { } } +test "PageList resize reflow exceeds hyperlink memory forcing capacity increase" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 10, 0); + defer s.deinit(); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + + // Grow to the capacity of the first page and add + // one more row so that we have two pages total. + { + const page = &s.pages.first.?.data; + page.pauseIntegrityChecks(true); + for (page.size.rows..page.capacity.rows) |_| { + _ = try s.grow(); + } + page.pauseIntegrityChecks(false); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + try s.growRows(1); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + + // We now have two pages. + try std.testing.expect(s.pages.first.? != s.pages.last.?); + try std.testing.expectEqual(s.pages.last.?, s.pages.first.?.next); + } + + // We use almost all string alloc capacity with a hyperlink in the final + // row of the first page, and do the same on the first row of the second + // page. We also mark the row as wrapped so that when we resize with more + // cols the row unwraps and we have a single row that requires almost two + // times the base string alloc capacity. + // + // This forces the reflow to increase capacity. + // + // +--+ = PAGE 0 + // : : + // | X… <- where X is hyperlinked with almost all string cap. + // +--+ + // +--+ = PAGE 1 + // …X | <- X here also almost hits string cap with a hyperlink. + // +--+ + + // Almost hit string alloc cap in bottom right of first page. + // Mark the final row as wrapped. + { + const page = &s.pages.first.?.data; + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = 0 }, + .uri = "a" ** (pagepkg.string_bytes_default - 1), + }); + const rac = page.getRowAndCell(page.size.cols - 1, page.size.rows - 1); + rac.row.wrap = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + }; + try page.setHyperlink(rac.row, rac.cell, id); + try std.testing.expectError( + error.StringsOutOfMemory, + page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = "AAAAAAAAAAAAAAAAAAAAAAAAAA", + }), + ); + } + + // Almost hit string alloc cap in top left of second page. + // Mark the first row as a wrap continuation. + { + const page = &s.pages.last.?.data; + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = "a" ** (pagepkg.string_bytes_default - 1), + }); + const rac = page.getRowAndCell(0, 0); + rac.row.wrap_continuation = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + }; + try page.setHyperlink(rac.row, rac.cell, id); + try std.testing.expectError( + error.StringsOutOfMemory, + page.insertHyperlink(.{ + .id = .{ .implicit = 2 }, + .uri = "AAAAAAAAAAAAAAAAAAAAAAAAAA", + }), + ); + } + + // Resize to 1 column wider, unwrapping the row. + try s.resize(.{ .cols = s.cols + 1, .reflow = true }); +} + test "PageList resize reflow more cols unwrap wide spacer head" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/page.zig b/src/terminal/page.zig index fea16c28b..fd5439c32 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -53,7 +53,7 @@ const string_chunk_len = 32; const string_chunk = string_chunk_len * @sizeOf(u8); const StringAlloc = BitmapAllocator(string_chunk); const string_count_default = StringAlloc.bitmap_bit_size; -const string_bytes_default = string_count_default * string_chunk; +pub const string_bytes_default = string_count_default * string_chunk; /// Default number of hyperlinks we support. /// From c105d70c73f26ef4b5282d95b529ecff2a6f99bf Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 18:30:50 -0600 Subject: [PATCH 16/70] PageList: increase capacity for hyperlink OOM during reflow It's possible for the hyperlink or string capacity to be exceeded in a single row, in which case it doesn't matter if we move the row to a new page, it will still be a problem. This was causing actual crashes under some circumstances. --- src/terminal/PageList.zig | 75 ++++++++++++++++++++++++++++++++++---- src/terminal/hyperlink.zig | 33 ++++++++++------- 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index cfc791ce5..149b4fe34 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1032,25 +1032,67 @@ const ReflowCursor = struct { const src_id = src_page.lookupHyperlink(cell).?; const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); - // If our page can't support an additional cell with - // a hyperlink ID then we create a new page for this row. + // If our page can't support an additional cell + // with a hyperlink then we increase capacity. if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { - try self.moveLastRowToNewPage(list, cap); + try self.adjustCapacity(list, .{ + .hyperlink_bytes = cap.hyperlink_bytes * 2, + }); + } + + // Ensure that the string alloc has sufficient capacity + // to dupe the link (and the ID if it's not implicit). + const additional_required_string_capacity = + src_link.uri.len + + switch (src_link.id) { + .explicit => |v| v.len, + .implicit => 0, + }; + if (self.page.string_alloc.alloc( + u8, + self.page.memory, + additional_required_string_capacity, + )) |slice| { + // We have enough capacity, free the test alloc. + self.page.string_alloc.free(self.page.memory, slice); + } else |_| { + // Grow our capacity until we can + // definitely fit the extra bytes. + var new_string_capacity: usize = cap.string_bytes; + while (new_string_capacity - cap.string_bytes < additional_required_string_capacity) { + new_string_capacity *= 2; + } + try self.adjustCapacity(list, .{ + .string_bytes = new_string_capacity, + }); } const dst_id = self.page.hyperlink_set.addWithIdContext( self.page.memory, + // We made sure there was enough capacity for this above. try src_link.dupe(src_page, self.page), src_id, .{ .page = self.page }, - ) catch id: { - // We have no space for this link, - // so make a new page for this row. - try self.moveLastRowToNewPage(list, cap); + ) catch |err| id: { + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by adjusting capacity, either with + // no actual change or with an increased hyperlink cap. + try self.adjustCapacity(list, switch (err) { + error.OutOfMemory => .{ + .hyperlink_bytes = cap.hyperlink_bytes * 2, + }, + error.NeedsRehash => .{}, + }); - break :id try self.page.hyperlink_set.addContext( + // We assume this one will succeed. We dupe the link + // again, and don't have to worry about the other one + // because adjusting the capacity naturally clears up + // any managed memory not associated with a cell yet. + break :id try self.page.hyperlink_set.addWithIdContext( self.page.memory, try src_link.dupe(src_page, self.page), + src_id, .{ .page = self.page }, ); } orelse src_id; @@ -1150,6 +1192,22 @@ const ReflowCursor = struct { } } + /// Adjust the capacity of the current page. + fn adjustCapacity( + self: *ReflowCursor, + list: *PageList, + adjustment: AdjustCapacity, + ) !void { + const old_x = self.x; + const old_y = self.y; + + self.* = .init(try list.adjustCapacity( + self.node, + adjustment, + )); + self.cursorAbsolute(old_x, old_y); + } + /// True if this cursor is at the bottom of the page by capacity, /// i.e. we can't scroll anymore. fn bottom(self: *const ReflowCursor) bool { @@ -7862,6 +7920,7 @@ test "PageList resize reflow less cols wrapped rows with graphemes" { try testing.expectEqual(@as(u21, 'A'), cps[0]); } } + test "PageList resize reflow less cols cursor in wrapped row" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/hyperlink.zig b/src/terminal/hyperlink.zig index bb9e78ca6..c608321b1 100644 --- a/src/terminal/hyperlink.zig +++ b/src/terminal/hyperlink.zig @@ -185,6 +185,25 @@ pub const PageEntry = struct { other.uri.offset.ptr(other_base)[0..other.uri.len], ); } + + /// Free the memory for this entry from its page. + pub fn free( + self: *const PageEntry, + page: *Page, + ) void { + const alloc = &page.string_alloc; + switch (self.id) { + .implicit => {}, + .explicit => |v| alloc.free( + page.memory, + v.offset.ptr(page.memory)[0..v.len], + ), + } + alloc.free( + page.memory, + self.uri.offset.ptr(page.memory)[0..self.uri.len], + ); + } }; /// The set of hyperlinks. This is ref-counted so that a set of cells @@ -215,19 +234,7 @@ pub const Set = RefCountedSet( } pub fn deleted(self: *const @This(), link: PageEntry) void { - const page = self.page.?; - const alloc = &page.string_alloc; - switch (link.id) { - .implicit => {}, - .explicit => |v| alloc.free( - page.memory, - v.offset.ptr(page.memory)[0..v.len], - ), - } - alloc.free( - page.memory, - link.uri.offset.ptr(page.memory)[0..link.uri.len], - ); + link.free(self.page.?); } }, ); From 3fcfc34ef7d55c598d4b868782ccc6de88339fe7 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 18:39:10 -0600 Subject: [PATCH 17/70] test(PageList): add failing test for reflow grapheme OOM --- src/terminal/PageList.zig | 108 ++++++++++++++++++++++++++++++++++++++ src/terminal/page.zig | 2 +- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 149b4fe34..3c9f60b7e 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -7375,6 +7375,114 @@ test "PageList resize reflow exceeds hyperlink memory forcing capacity increase" try s.resize(.{ .cols = s.cols + 1, .reflow = true }); } +test "PageList resize reflow exceeds grapheme memory forcing capacity increase" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 10, 0); + defer s.deinit(); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + + // Grow to the capacity of the first page and add + // one more row so that we have two pages total. + { + const page = &s.pages.first.?.data; + page.pauseIntegrityChecks(true); + for (page.size.rows..page.capacity.rows) |_| { + _ = try s.grow(); + } + page.pauseIntegrityChecks(false); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + try s.growRows(1); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + + // We now have two pages. + try std.testing.expect(s.pages.first.? != s.pages.last.?); + try std.testing.expectEqual(s.pages.last.?, s.pages.first.?.next); + } + + // We use almost all grapheme alloc capacity with a grapheme in the final + // row of the first page, and do the same on the first row of the second + // page. We also mark the row as wrapped so that when we resize with more + // cols the row unwraps and we have a single row that requires almost two + // times the base grapheme alloc capacity. + // + // This forces the reflow to increase capacity. + // + // +--+ = PAGE 0 + // : : + // | X… <- where X is a grapheme which uses almost all the capacity. + // +--+ + // +--+ = PAGE 1 + // …X | <- X here also almost hits grapheme cap. + // +--+ + + // Almost hit grapheme alloc cap in bottom right of first page. + // Mark the final row as wrapped. + { + const page = &s.pages.first.?.data; + const rac = page.getRowAndCell(page.size.cols - 1, page.size.rows - 1); + rac.row.wrap = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + }; + try page.setGraphemes( + rac.row, + rac.cell, + &@as( + [@divFloor( + pagepkg.grapheme_bytes_default - 1, + @sizeOf(u21), + )]u21, + @splat('a'), + ), + ); + try std.testing.expectError( + error.OutOfMemory, + page.grapheme_alloc.alloc( + u21, + page.memory, + 16, + ), + ); + } + + // Almost hit grapheme alloc cap in top left of second page. + // Mark the first row as a wrap continuation. + { + const page = &s.pages.last.?.data; + const rac = page.getRowAndCell(0, 0); + rac.row.wrap = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + }; + try page.setGraphemes( + rac.row, + rac.cell, + &@as( + [@divFloor( + pagepkg.grapheme_bytes_default - 1, + @sizeOf(u21), + )]u21, + @splat('a'), + ), + ); + try std.testing.expectError( + error.OutOfMemory, + page.grapheme_alloc.alloc( + u21, + page.memory, + 16, + ), + ); + } + + // Resize to 1 column wider, unwrapping the row. + try s.resize(.{ .cols = s.cols + 1, .reflow = true }); +} + test "PageList resize reflow more cols unwrap wide spacer head" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/page.zig b/src/terminal/page.zig index fd5439c32..2a631ac78 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -34,7 +34,7 @@ const grapheme_chunk_len = 4; const grapheme_chunk = grapheme_chunk_len * @sizeOf(u21); const GraphemeAlloc = BitmapAllocator(grapheme_chunk); const grapheme_count_default = GraphemeAlloc.bitmap_bit_size; -const grapheme_bytes_default = grapheme_count_default * grapheme_chunk; +pub const grapheme_bytes_default = grapheme_count_default * grapheme_chunk; const GraphemeMap = AutoOffsetHashMap(Offset(Cell), Offset(u21).Slice); /// The allocator used for shared utf8-encoded strings within a page. From 15aa9df051a95d59ff497f226530b58e5e6bdb37 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 18:45:07 -0600 Subject: [PATCH 18/70] PageList: increase capacity for grapheme OOM during reflow --- src/terminal/PageList.zig | 41 +++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 3c9f60b7e..8571912d5 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1004,23 +1004,34 @@ const ReflowCursor = struct { // Copy the graphemes const cps = src_page.lookupGrapheme(cell).?; - // If our page can't support an additional cell with - // graphemes then we create a new page for this row. + // If our page can't support an additional cell + // with graphemes then we increase capacity. if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { - try self.moveLastRowToNewPage(list, cap); - } else { - // Attempt to allocate the space that would be required for - // these graphemes, and if it's not available, create a new - // page for this row. - if (self.page.grapheme_alloc.alloc( - u21, - self.page.memory, - cps.len, - )) |slice| { - self.page.grapheme_alloc.free(self.page.memory, slice); - } else |_| { - try self.moveLastRowToNewPage(list, cap); + try self.adjustCapacity(list, .{ + .hyperlink_bytes = cap.grapheme_bytes * 2, + }); + } + + // Attempt to allocate the space that would be required + // for these graphemes, and if it's not available, then + // increase capacity. + if (self.page.grapheme_alloc.alloc( + u21, + self.page.memory, + cps.len, + )) |slice| { + self.page.grapheme_alloc.free(self.page.memory, slice); + } else |_| { + // Grow our capacity until we can + // definitely fit the extra bytes. + const required = cps.len * @sizeOf(u21); + var new_grapheme_capacity: usize = cap.grapheme_bytes; + while (new_grapheme_capacity - cap.grapheme_bytes < required) { + new_grapheme_capacity *= 2; } + try self.adjustCapacity(list, .{ + .grapheme_bytes = new_grapheme_capacity, + }); } // This shouldn't fail since we made sure we have space above. From ac308b0418054332e389307b9babcf821bb9c12f Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 19:18:19 -0600 Subject: [PATCH 19/70] test(PageList): add failing test for reflow style OOM --- src/terminal/PageList.zig | 104 +++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 8 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 8571912d5..310118051 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -7442,10 +7442,12 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" rac.row, rac.cell, &@as( - [@divFloor( - pagepkg.grapheme_bytes_default - 1, - @sizeOf(u21), - )]u21, + [ + @divFloor( + pagepkg.grapheme_bytes_default - 1, + @sizeOf(u21), + ) + ]u21, @splat('a'), ), ); @@ -7473,10 +7475,12 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" rac.row, rac.cell, &@as( - [@divFloor( - pagepkg.grapheme_bytes_default - 1, - @sizeOf(u21), - )]u21, + [ + @divFloor( + pagepkg.grapheme_bytes_default - 1, + @sizeOf(u21), + ) + ]u21, @splat('a'), ), ); @@ -7494,6 +7498,90 @@ test "PageList resize reflow exceeds grapheme memory forcing capacity increase" try s.resize(.{ .cols = s.cols + 1, .reflow = true }); } +test "PageList resize reflow exceeds style memory forcing capacity increase" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, pagepkg.std_capacity.styles - 1, 10, 0); + defer s.deinit(); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + + // Grow to the capacity of the first page and add + // one more row so that we have two pages total. + { + const page = &s.pages.first.?.data; + page.pauseIntegrityChecks(true); + for (page.size.rows..page.capacity.rows) |_| { + _ = try s.grow(); + } + page.pauseIntegrityChecks(false); + try testing.expectEqual(@as(usize, 1), s.totalPages()); + try s.growRows(1); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + + // We now have two pages. + try std.testing.expect(s.pages.first.? != s.pages.last.?); + try std.testing.expectEqual(s.pages.last.?, s.pages.first.?.next); + } + + // Give each cell in the final row of the first page a unique style. + // Mark the final row as wrapped. + { + const page = &s.pages.first.?.data; + for (0..s.cols) |x| { + const id = page.styles.add( + page.memory, + .{ + .bg_color = .{ .rgb = .{ + .r = @truncate(x), + .g = @truncate(x >> 8), + .b = @truncate(x >> 16), + } }, + }, + ) catch break; + + const rac = page.getRowAndCell(x, page.size.rows - 1); + rac.row.wrap = true; + rac.row.styled = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + .style_id = id, + }; + } + } + + // Do the same for the first row of the second page. + // Mark the first row as a wrap continuation. + { + const page = &s.pages.last.?.data; + for (0..s.cols) |x| { + const id = page.styles.add( + page.memory, + .{ + .fg_color = .{ .rgb = .{ + .r = @truncate(x), + .g = @truncate(x >> 8), + .b = @truncate(x >> 16), + } }, + }, + ) catch break; + + const rac = page.getRowAndCell(x, 0); + rac.row.wrap_continuation = true; + rac.row.styled = true; + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'X' }, + .style_id = id, + }; + } + } + + // Resize to twice as wide, fully unwrapping the row. + try s.resize(.{ .cols = s.cols * 2, .reflow = true }); +} + test "PageList resize reflow more cols unwrap wide spacer head" { const testing = std.testing; const alloc = testing.allocator; From a53ec1e567341b4e6dff985a6ab2609ea4813dd8 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 18 Aug 2025 18:49:25 -0600 Subject: [PATCH 20/70] PageList: increase capacity for style OOM during reflow --- src/terminal/PageList.zig | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 310118051..3b329d7cf 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1128,14 +1128,23 @@ const ReflowCursor = struct { self.page.memory, style, cell.style_id, - ) catch id: { - // We have no space for this style, - // so make a new page for this row. - try self.moveLastRowToNewPage(list, cap); + ) catch |err| id: { + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by adjusting capacity, either with + // no actual change or with an increased style cap. + try self.adjustCapacity(list, switch (err) { + error.OutOfMemory => .{ + .styles = cap.styles * 2, + }, + error.NeedsRehash => .{}, + }); - break :id try self.page.styles.add( + // We assume this one will succeed. + break :id try self.page.styles.addWithId( self.page.memory, style, + cell.style_id, ); } orelse cell.style_id; From f430c03ff3e094fd917f10a955160884579aa143 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 19 Aug 2025 10:35:05 -0400 Subject: [PATCH 21/70] zsh: add tests for setupZsh --- src/termio/shell_integration.zig | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/termio/shell_integration.zig b/src/termio/shell_integration.zig index 438c2a0ea..30519b6e2 100644 --- a/src/termio/shell_integration.zig +++ b/src/termio/shell_integration.zig @@ -670,3 +670,27 @@ fn setupZsh( ); try env.put("ZDOTDIR", integ_dir); } + +test "zsh" { + const testing = std.testing; + + var env = EnvMap.init(testing.allocator); + defer env.deinit(); + + try setupZsh(".", &env); + try testing.expectEqualStrings("./shell-integration/zsh", env.get("ZDOTDIR").?); + try testing.expect(env.get("GHOSTTY_ZSH_ZDOTDIR") == null); +} + +test "zsh: ZDOTDIR" { + const testing = std.testing; + + var env = EnvMap.init(testing.allocator); + defer env.deinit(); + + try env.put("ZDOTDIR", "$HOME/.config/zsh"); + + try setupZsh(".", &env); + try testing.expectEqualStrings("./shell-integration/zsh", env.get("ZDOTDIR").?); + try testing.expectEqualStrings("$HOME/.config/zsh", env.get("GHOSTTY_ZSH_ZDOTDIR").?); +} From e8a60a375c2902caa4727aafa3cc61345f4085a3 Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 19 Aug 2025 10:36:26 -0400 Subject: [PATCH 22/70] zsh: unset _ghostty_file in the early exit path If we're running a too-old version of zsh, we exit early. This skipped the _ghostty_file cleanup path below. --- src/shell-integration/zsh/.zshenv | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shell-integration/zsh/.zshenv b/src/shell-integration/zsh/.zshenv index 7fbfad659..3941e0c30 100644 --- a/src/shell-integration/zsh/.zshenv +++ b/src/shell-integration/zsh/.zshenv @@ -45,6 +45,7 @@ fi 'builtin' 'autoload' '--' 'is-at-least' 'is-at-least' "5.1" || { builtin echo "ZSH ${ZSH_VERSION} is too old for ghostty shell integration" > /dev/stderr + 'builtin' 'unset' '_ghostty_file' return } # ${(%):-%x} is the path to the current file. From 8300512a9118b7757958b95703914998b5111f9b Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Tue, 19 Aug 2025 10:39:09 -0400 Subject: [PATCH 23/70] zsh: clarify that an unset ZDOTDIR defaults to HOME This fixes the incorrect comment and uses $HOME (rather than ~) to be a little bit more explicit. Also, our script is named ghostty-integration, not ghostty.zsh, so update that part of the comment, too. --- src/shell-integration/zsh/.zshenv | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/shell-integration/zsh/.zshenv b/src/shell-integration/zsh/.zshenv index 3941e0c30..3332b1c1f 100644 --- a/src/shell-integration/zsh/.zshenv +++ b/src/shell-integration/zsh/.zshenv @@ -29,14 +29,15 @@ fi # Use try-always to have the right error code. { - # Zsh treats empty $ZDOTDIR as if it was "/". We do the same. + # Zsh treats unset ZDOTDIR as if it was HOME. We do the same. # - # Source the user's zshenv before sourcing ghostty.zsh because the former - # might set fpath and other things without which ghostty.zsh won't work. + # Source the user's .zshenv before sourcing ghostty-integration because the + # former might set fpath and other things without which ghostty-integration + # won't work. # # Use typeset in case we are in a function with warn_create_global in # effect. Unlikely but better safe than sorry. - 'builtin' 'typeset' _ghostty_file=${ZDOTDIR-~}"/.zshenv" + 'builtin' 'typeset' _ghostty_file=${ZDOTDIR-$HOME}"/.zshenv" # Zsh ignores unreadable rc files. We do the same. # Zsh ignores rc files that are directories, and so does source. [[ ! -r "$_ghostty_file" ]] || 'builtin' 'source' '--' "$_ghostty_file" From 2421132d80a9114ebc937d923603128314c97c51 Mon Sep 17 00:00:00 2001 From: Leah Amelia Chen Date: Mon, 18 Aug 2025 11:41:31 +0800 Subject: [PATCH 24/70] config: fix accidental codeblock indents In our webgen we treat 4 consecutive spaces as a code block, which is often triggered by mistake when a paragraph is encased within a list. We should probably fix this more thoroughly at some point since I don't think actual Markdown parsers have the same behavior, but for now we just fall back to using 3-space indents. --- src/config/Config.zig | 148 +++++++++++++++++++++++------------------- 1 file changed, 81 insertions(+), 67 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 2f6643c7d..05b5828f0 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -592,24 +592,24 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// /// * `contain` /// -/// Preserving the aspect ratio, scale the background image to the largest -/// size that can still be contained within the terminal, so that the whole -/// image is visible. +/// Preserving the aspect ratio, scale the background image to the largest +/// size that can still be contained within the terminal, so that the whole +/// image is visible. /// /// * `cover` /// -/// Preserving the aspect ratio, scale the background image to the smallest -/// size that can completely cover the terminal. This may result in one or -/// more edges of the image being clipped by the edge of the terminal. +/// Preserving the aspect ratio, scale the background image to the smallest +/// size that can completely cover the terminal. This may result in one or +/// more edges of the image being clipped by the edge of the terminal. /// /// * `stretch` /// -/// Stretch the background image to the full size of the terminal, without -/// preserving the aspect ratio. +/// Stretch the background image to the full size of the terminal, without +/// preserving the aspect ratio. /// /// * `none` /// -/// Don't scale the background image. +/// Don't scale the background image. /// /// The default value is `contain`. /// @@ -1330,53 +1330,59 @@ class: ?[:0]const u8 = null, /// The keybind trigger can be prefixed with some special values to change /// the behavior of the keybind. These are: /// -/// * `all:` - Make the keybind apply to all terminal surfaces. By default, -/// keybinds only apply to the focused terminal surface. If this is true, -/// then the keybind will be sent to all terminal surfaces. This only -/// applies to actions that are surface-specific. For actions that -/// are already global (e.g. `quit`), this prefix has no effect. +/// * `all:` /// -/// Available since: 1.0.0 +/// Make the keybind apply to all terminal surfaces. By default, +/// keybinds only apply to the focused terminal surface. If this is true, +/// then the keybind will be sent to all terminal surfaces. This only +/// applies to actions that are surface-specific. For actions that +/// are already global (e.g. `quit`), this prefix has no effect. /// -/// * `global:` - Make the keybind global. By default, keybinds only work -/// within Ghostty and under the right conditions (application focused, -/// sometimes terminal focused, etc.). If you want a keybind to work -/// globally across your system (e.g. even when Ghostty is not focused), -/// specify this prefix. This prefix implies `all:`. Note: this does not -/// work in all environments; see the additional notes below for more -/// information. +/// Available since: 1.0.0 /// -/// Available since: 1.0.0 (on macOS) -/// Available since: 1.2.0 (on GTK) +/// * `global:` /// -/// * `unconsumed:` - Do not consume the input. By default, a keybind -/// will consume the input, meaning that the associated encoding (if -/// any) will not be sent to the running program in the terminal. If -/// you wish to send the encoded value to the program, specify the -/// `unconsumed:` prefix before the entire keybind. For example: -/// `unconsumed:ctrl+a=reload_config`. `global:` and `all:`-prefixed -/// keybinds will always consume the input regardless of this setting. -/// Since they are not associated with a specific terminal surface, -/// they're never encoded. +/// Make the keybind global. By default, keybinds only work within Ghostty +/// and under the right conditions (application focused, sometimes terminal +/// focused, etc.). If you want a keybind to work globally across your system +/// (e.g. even when Ghostty is not focused), specify this prefix. +/// This prefix implies `all:`. /// -/// Available since: 1.0.0 +/// Note: this does not work in all environments; see the additional notes +/// below for more information. /// -/// * `performable:` - Only consume the input if the action is able to be -/// performed. For example, the `copy_to_clipboard` action will only -/// consume the input if there is a selection to copy. If there is no -/// selection, Ghostty behaves as if the keybind was not set. This has -/// no effect with `global:` or `all:`-prefixed keybinds. For key -/// sequences, this will reset the sequence if the action is not -/// performable (acting identically to not having a keybind set at -/// all). +/// Available since: 1.0.0 on macOS, 1.2.0 on GTK /// -/// Performable keybinds will not appear as menu shortcuts in the -/// application menu. This is because the menu shortcuts force the -/// action to be performed regardless of the state of the terminal. -/// Performable keybinds will still work, they just won't appear as -/// a shortcut label in the menu. +/// * `unconsumed:` /// -/// Available since: 1.1.0 +/// Do not consume the input. By default, a keybind will consume the input, +/// meaning that the associated encoding (if any) will not be sent to the +/// running program in the terminal. If you wish to send the encoded value +/// to the program, specify the `unconsumed:` prefix before the entire +/// keybind. For example: `unconsumed:ctrl+a=reload_config`. `global:` and +/// `all:`-prefixed keybinds will always consume the input regardless of +/// this setting. Since they are not associated with a specific terminal +/// surface, they're never encoded. +/// +/// Available since: 1.0.0 +/// +/// * `performable:` +/// +/// Only consume the input if the action is able to be performed. +/// For example, the `copy_to_clipboard` action will only consume the input +/// if there is a selection to copy. If there is no selection, Ghostty +/// behaves as if the keybind was not set. This has no effect with `global:` +/// or `all:`-prefixed keybinds. For key sequences, this will reset the +/// sequence if the action is not performable (acting identically to not +/// having a keybind set at all). +/// +/// Performable keybinds will not appear as menu shortcuts in the +/// application menu. This is because the menu shortcuts force the +/// action to be performed regardless of the state of the terminal. +/// Performable keybinds will still work, they just won't appear as +/// a shortcut label in the menu. +/// +/// Available since: 1.1.0 /// /// Keybind triggers are not unique per prefix combination. For example, /// `ctrl+a` and `global:ctrl+a` are not two separate keybinds. The keybind @@ -1522,28 +1528,36 @@ keybind: Keybinds = .{}, /// /// Valid values: /// -/// * `none` - All window decorations will be disabled. Titlebar, -/// borders, etc. will not be shown. On macOS, this will also disable -/// tabs (enforced by the system). +/// * `none` /// -/// * `auto` - Automatically decide to use either client-side or server-side -/// decorations based on the detected preferences of the current OS and -/// desktop environment. This option usually makes Ghostty look the most -/// "native" for your desktop. +/// All window decorations will be disabled. Titlebar, borders, etc. will +/// not be shown. On macOS, this will also disable tabs (enforced by the +/// system). /// -/// * `client` - Prefer client-side decorations. +/// * `auto` /// -/// Available since: 1.1.0 +/// Automatically decide to use either client-side or server-side +/// decorations based on the detected preferences of the current OS and +/// desktop environment. This option usually makes Ghostty look the most +/// "native" for your desktop. /// -/// * `server` - Prefer server-side decorations. This is only relevant -/// on Linux with GTK, either on X11, or Wayland on a compositor that -/// supports the `org_kde_kwin_server_decoration` protocol (e.g. KDE Plasma, -/// but almost any non-GNOME desktop supports this protocol). +/// * `client` /// -/// If `server` is set but the environment doesn't support server-side -/// decorations, client-side decorations will be used instead. +/// Prefer client-side decorations. /// -/// Available since: 1.1.0 +/// Available since: 1.1.0 +/// +/// * `server` +/// +/// Prefer server-side decorations. This is only relevant on Linux with GTK, +/// either on X11, or Wayland on a compositor that supports the +/// `org_kde_kwin_server_decoration` protocol (e.g. KDE Plasma, but almost +/// any non-GNOME desktop supports this protocol). +/// +/// If `server` is set but the environment doesn't support server-side +/// decorations, client-side decorations will be used instead. +/// +/// Available since: 1.1.0 /// /// The default value is `auto`. /// @@ -2316,9 +2330,9 @@ keybind: Keybinds = .{}, /// /// * `sampler2D iChannel0` - Input texture. /// -/// A texture containing the current terminal screen. If multiple custom -/// shaders are specified, the output of previous shaders is written to -/// this texture, to allow combining multiple effects. +/// A texture containing the current terminal screen. If multiple custom +/// shaders are specified, the output of previous shaders is written to +/// this texture, to allow combining multiple effects. /// /// * `vec3 iResolution` - Output texture size, `[width, height, 1]` (in px). /// From f3d8aac1e9b44b33a0e6bdf41107a9ccd5d45f86 Mon Sep 17 00:00:00 2001 From: Leah Amelia Chen Date: Wed, 20 Aug 2025 02:53:41 +0800 Subject: [PATCH 25/70] gtk-ng: fix toggle_window_decoration When window-decoration=none, setting the window decoration to null would just mean it would default to none again, creating a cycle of torment none can break out of... that sounds a bit too dramatic doesn't it Fixes #8274 --- src/apprt/gtk-ng/class/application.zig | 2 +- src/apprt/gtk-ng/class/window.zig | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk-ng/class/application.zig b/src/apprt/gtk-ng/class/application.zig index bfecab3e1..29a124798 100644 --- a/src/apprt/gtk-ng/class/application.zig +++ b/src/apprt/gtk-ng/class/application.zig @@ -2216,7 +2216,7 @@ const Action = struct { Window, surface.as(gtk.Widget), ) orelse { - log.warn("surface is not in a window, ignoring new_tab", .{}); + log.warn("surface is not in a window, ignoring toggle_window_decorations", .{}); return false; }; diff --git a/src/apprt/gtk-ng/class/window.zig b/src/apprt/gtk-ng/class/window.zig index 18e9178ac..502b4dc78 100644 --- a/src/apprt/gtk-ng/class/window.zig +++ b/src/apprt/gtk-ng/class/window.zig @@ -780,9 +780,18 @@ pub const Window = extern struct { /// Toggle the window decorations for this window. pub fn toggleWindowDecorations(self: *Self) void { - self.setWindowDecoration(switch (self.getWindowDecoration()) { - // Null will force using the central config - .none => null, + const priv = self.private(); + + if (priv.window_decoration) |_| { + // Unset any previously set window decoration settings + self.setWindowDecoration(null); + return; + } + + const config = if (priv.config) |v| v.get() else return; + self.setWindowDecoration(switch (config.@"window-decoration") { + // Use auto when the decoration is initially none + .none => .auto, // Anything non-none to none .auto, .client, .server => .none, From 7977b3695ab04f96e83581a481f035a4cd6c60ef Mon Sep 17 00:00:00 2001 From: Leah Amelia Chen Date: Wed, 20 Aug 2025 03:23:35 +0800 Subject: [PATCH 26/70] gtk-ng: attach surface size callbacks AFTER realize The `gdk.Surface` is only ever available *after* the window had been first presented and mapped. Trying to get the surface during `init` like what we had previously done will **always** return null. --- src/apprt/gtk-ng/class/window.zig | 37 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/apprt/gtk-ng/class/window.zig b/src/apprt/gtk-ng/class/window.zig index 18e9178ac..a513fd287 100644 --- a/src/apprt/gtk-ng/class/window.zig +++ b/src/apprt/gtk-ng/class/window.zig @@ -301,24 +301,6 @@ pub const Window = extern struct { // Initialize our actions self.initActionMap(); - // We need to setup resize notifications on our surface - if (self.as(gtk.Native).getSurface()) |gdk_surface| { - _ = gobject.Object.signals.notify.connect( - gdk_surface, - *Self, - propGdkSurfaceWidth, - self, - .{ .detail = "width" }, - ); - _ = gobject.Object.signals.notify.connect( - gdk_surface, - *Self, - propGdkSurfaceHeight, - self, - .{ .detail = "height" }, - ); - } - // Start states based on config. if (priv.config) |config_obj| { const config = config_obj.get(); @@ -1124,6 +1106,25 @@ pub const Window = extern struct { return; } + // We need to setup resize notifications on our surface, + // which is only available after the window had been realized. + if (self.as(gtk.Native).getSurface()) |gdk_surface| { + _ = gobject.Object.signals.notify.connect( + gdk_surface, + *Self, + propGdkSurfaceWidth, + self, + .{ .detail = "width" }, + ); + _ = gobject.Object.signals.notify.connect( + gdk_surface, + *Self, + propGdkSurfaceHeight, + self, + .{ .detail = "height" }, + ); + } + // When we are realized we always setup our appearance since this // calls some winproto functions. self.syncAppearance(); From 54b7e1838c0df472324f93a79d5947e75ea2fea9 Mon Sep 17 00:00:00 2001 From: Luzian Bieri Date: Fri, 15 Aug 2025 23:28:43 +0200 Subject: [PATCH 27/70] feat: add right-click action configuration --- src/Surface.zig | 54 ++++++++++++++++++++++++++++++++++--------- src/config.zig | 1 + src/config/Config.zig | 32 +++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index f45185b94..9d8ceafb0 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -247,6 +247,7 @@ const DerivedConfig = struct { clipboard_paste_protection: bool, clipboard_paste_bracketed_safe: bool, copy_on_select: configpkg.CopyOnSelect, + right_click_action: configpkg.RightClickAction, confirm_close_surface: configpkg.ConfirmCloseSurface, cursor_click_to_move: bool, desktop_notifications: bool, @@ -314,6 +315,7 @@ const DerivedConfig = struct { .clipboard_paste_protection = config.@"clipboard-paste-protection", .clipboard_paste_bracketed_safe = config.@"clipboard-paste-bracketed-safe", .copy_on_select = config.@"copy-on-select", + .right_click_action = config.@"right-click-action", .confirm_close_surface = config.@"confirm-close-surface", .cursor_click_to_move = config.@"cursor-click-to-move", .desktop_notifications = config.@"desktop-notifications", @@ -1881,8 +1883,7 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { // Both standard and selection clipboards are set. .clipboard => { - const clipboards: []const apprt.Clipboard = &.{ .standard, .selection }; - copySelectionToClipboards(self, sel, clipboards); + self.copySelectionToClipboards(sel, &.{ .standard, .selection }); }, // The selection clipboard is set if supported, otherwise the standard. @@ -1891,7 +1892,7 @@ fn setSelection(self: *Surface, sel_: ?terminal.Selection) !void { .selection else .standard; - copySelectionToClipboards(self, sel, &.{clipboard}); + self.copySelectionToClipboards(sel, &.{clipboard}); }, } } @@ -3578,18 +3579,49 @@ pub fn mouseButtonCallback( break :pin pin; }; - // If we already have a selection and the selection contains - // where we clicked then we don't want to modify the selection. - if (self.io.terminal.screen.selection) |prev_sel| { - if (prev_sel.contains(screen, pin)) break :sel; + switch (self.config.right_click_action) { + .ignore => { + // Return early to skip clearing the selection. + try self.queueRender(); + return true; + }, + .copy => { + if (self.io.terminal.screen.selection) |sel| { + self.copySelectionToClipboards(sel, &.{.standard}); + } + }, + .@"copy-or-paste" => { + if (self.io.terminal.screen.selection) |sel| { + self.copySelectionToClipboards(sel, &.{.standard}); + } else { + try self.startClipboardRequest(.standard, .paste); + } + }, + .paste => { + try self.startClipboardRequest(.standard, .paste); + }, + .@"context-menu" => { + // If we already have a selection and the selection contains + // where we clicked then we don't want to modify the selection. + if (self.io.terminal.screen.selection) |prev_sel| { + if (prev_sel.contains(screen, pin)) break :sel; - // The selection doesn't contain our pin, so we create a new - // word selection where we clicked. + // The selection doesn't contain our pin, so we create a new + // word selection where we clicked. + } + + const sel = screen.selectWord(pin) orelse break :sel; + try self.setSelection(sel); + try self.queueRender(); + return false; + }, } - const sel = screen.selectWord(pin) orelse break :sel; - try self.setSelection(sel); + try self.setSelection(null); try self.queueRender(); + + // Consume the event such that the context menu is not displayed. + return true; } return false; diff --git a/src/config.zig b/src/config.zig index df4eee791..bcb48214d 100644 --- a/src/config.zig +++ b/src/config.zig @@ -19,6 +19,7 @@ pub const ClipboardAccess = Config.ClipboardAccess; pub const Command = Config.Command; pub const ConfirmCloseSurface = Config.ConfirmCloseSurface; pub const CopyOnSelect = Config.CopyOnSelect; +pub const RightClickAction = Config.RightClickAction; pub const CustomShaderAnimation = Config.CustomShaderAnimation; pub const FontSyntheticStyle = Config.FontSyntheticStyle; pub const FontShapingBreak = Config.FontShapingBreak; diff --git a/src/config/Config.zig b/src/config/Config.zig index 2f6643c7d..1dad7708a 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1886,6 +1886,19 @@ keybind: Keybinds = .{}, else => .false, }, +/// The action to take when the user right-clicks on the terminal surface. +/// +/// Valid values: +/// * `context-menu` - Show the context menu. +/// * `paste` - Paste the contents of the clipboard. +/// * `copy` - Copy the selected text to the clipboard. +/// * `copy-or-paste` - If there is a selection, copy the selected text to +/// the clipboard; otherwise, paste the contents of the clipboard. +/// * `ignore` - Do nothing, ignore the right-click. +/// +/// The default value is `context-menu`. +@"right-click-action": RightClickAction = .@"context-menu", + /// The time in milliseconds between clicks to consider a click a repeat /// (double, triple, etc.) or an entirely new single click. A value of zero will /// use a platform-specific default. The default on macOS is determined by the @@ -6695,6 +6708,25 @@ pub const CopyOnSelect = enum { clipboard, }; +/// Options for right-click actions. +pub const RightClickAction = enum { + /// No action is taken on right-click. + ignore, + + /// Pastes from the system clipboard. + paste, + + /// Copies the selected text to the system clipboard. + copy, + + /// Copies the selected text to the system clipboard and + /// pastes the clipboard if no text is selected. + @"copy-or-paste", + + /// Shows a context menu with options. + @"context-menu", +}; + /// Shell integration values pub const ShellIntegration = enum { none, From 1c96870c1777eea6a2ae4012654f06438fe2eda5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 19 Aug 2025 14:10:28 -0700 Subject: [PATCH 28/70] macos: show copy menu item if selection start is outside viewport Fixes #8187 This properly handles the scenario with our `read_text` C API when the selection start is outside the viewport. Previously we'd report null (no text) which would cascade into things like the right click menu not showing a copy option. --- src/Surface.zig | 135 ++++++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 80 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 9d8ceafb0..2ab265cda 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1531,11 +1531,6 @@ pub const Text = struct { /// The viewport information about this text, if it is visible in /// the viewport. - /// - /// NOTE(mitchellh): This will only be non-null currently if the entirety - /// of the selection is contained within the viewport. We don't have a - /// use case currently for partial bounds but we should support this - /// eventually. viewport: ?Viewport = null, pub const Viewport = struct { @@ -1546,6 +1541,13 @@ pub const Text = struct { /// The linear offset of the start of the selection and the length. /// This is "linear" in the sense that it is the offset in the /// flattened viewport as a single array of text. + /// + /// Note: these values are currently wrong if there is a partially + /// visible selection in the viewport (i.e. the top-left or + /// bottom-right of the selection is outside the viewport). But the + /// apprt usecase we have right now doesn't require these to be + /// correct so... let's fix this later. The wrong values will always + /// be within the text bounds so we aren't risking an overflow. offset_start: u32, offset_len: u32, }; @@ -1587,17 +1589,57 @@ pub fn dumpTextLocked( // Calculate our viewport info if we can. const vp: ?Text.Viewport = viewport: { - // If our tl or br is not in the viewport then we don't - // have a viewport. One day we should extend this to support - // partial selections that are in the viewport. - const tl_pt = self.io.terminal.screen.pages.pointFromPin( + // If our bottom right pin is before the viewport, then we can't + // possibly have this text be within the viewport. + const vp_tl_pin = self.io.terminal.screen.pages.getTopLeft(.viewport); + const br_pin = sel.bottomRight(&self.io.terminal.screen); + if (br_pin.before(vp_tl_pin)) break :viewport null; + + // If our top-left pin is after the viewport, then we can't possibly + // have this text be within the viewport. + const vp_br_pin = self.io.terminal.screen.pages.getBottomRight(.viewport) orelse { + // I don't think this is possible but I don't want to crash on + // that assertion so let's just break out... + log.warn("viewport bottom-right pin not found, bug?", .{}); + break :viewport null; + }; + const tl_pin = sel.topLeft(&self.io.terminal.screen); + if (vp_br_pin.before(tl_pin)) break :viewport null; + + // We established that our top-left somewhere before the viewport + // bottom-right and that our bottom-right is somewhere after + // the top-left. This means that at least some portion of our + // selection is within the viewport. + + // Our top-left point. If it doesn't exist in the viewport it must + // be before and we can return (0,0). + const tl_pt: terminal.Point = self.io.terminal.screen.pages.pointFromPin( .viewport, - sel.topLeft(&self.io.terminal.screen), - ) orelse break :viewport null; + tl_pin, + ) orelse tl: { + if (comptime std.debug.runtime_safety) { + assert(tl_pin.before(vp_tl_pin)); + } + + break :tl .{ .viewport = .{} }; + }; + + // Our bottom-right point. If it doesn't exist in the viewport + // it must be the bottom-right of the viewport. const br_pt = self.io.terminal.screen.pages.pointFromPin( .viewport, - sel.bottomRight(&self.io.terminal.screen), - ) orelse break :viewport null; + br_pin, + ) orelse br: { + if (comptime std.debug.runtime_safety) { + assert(vp_br_pin.before(br_pin)); + } + + break :br self.io.terminal.screen.pages.pointFromPin( + .viewport, + vp_br_pin, + ).?; + }; + const tl_coord = tl_pt.coord(); const br_coord = br_pt.coord(); @@ -1668,73 +1710,6 @@ pub fn selectionString(self: *Surface, alloc: Allocator) !?[:0]const u8 { }); } -/// Return the apprt selection metadata used by apprt's for implementing -/// things like contextual information on right click and so on. -/// -/// This only returns non-null if the selection is fully contained within -/// the viewport. The use case for this function at the time of authoring -/// it is for apprt's to implement right-click contextual menus and -/// those only make sense for selections fully contained within the -/// viewport. We don't handle the case where you right click a word-wrapped -/// word at the end of the viewport yet. -pub fn selectionInfo(self: *const Surface) ?apprt.Selection { - self.renderer_state.mutex.lock(); - defer self.renderer_state.mutex.unlock(); - const sel = self.io.terminal.screen.selection orelse return null; - - // Get the TL/BR pins for the selection and convert to viewport. - const tl = sel.topLeft(&self.io.terminal.screen); - const br = sel.bottomRight(&self.io.terminal.screen); - const tl_pt = self.io.terminal.screen.pages.pointFromPin(.viewport, tl) orelse return null; - const br_pt = self.io.terminal.screen.pages.pointFromPin(.viewport, br) orelse return null; - const tl_coord = tl_pt.coord(); - const br_coord = br_pt.coord(); - - // Utilize viewport sizing to convert to offsets - const start = tl_coord.y * self.io.terminal.screen.pages.cols + tl_coord.x; - const end = br_coord.y * self.io.terminal.screen.pages.cols + br_coord.x; - - // Our sizes are all scaled so we need to send the unscaled values back. - const content_scale = self.rt_surface.getContentScale() catch .{ .x = 1, .y = 1 }; - - const x: f64 = x: { - // Simple x * cell width gives the left - var x: f64 = @floatFromInt(tl_coord.x * self.size.cell.width); - - // Add padding - x += @floatFromInt(self.size.padding.left); - - // Scale - x /= content_scale.x; - - break :x x; - }; - - const y: f64 = y: { - // Simple y * cell height gives the top - var y: f64 = @floatFromInt(tl_coord.y * self.size.cell.height); - - // We want the text baseline - y += @floatFromInt(self.size.cell.height); - y -= @floatFromInt(self.font_metrics.cell_baseline); - - // Add padding - y += @floatFromInt(self.size.padding.top); - - // Scale - y /= content_scale.y; - - break :y y; - }; - - return .{ - .tl_x_px = x, - .tl_y_px = y, - .offset_start = start, - .offset_len = end - start, - }; -} - /// Returns the pwd of the terminal, if any. This is always copied because /// the pwd can change at any point from termio. If we are calling from the IO /// thread you should just check the terminal directly. From 63ca777e0f48bca715083f5c98f6246d5e5952ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 19 Aug 2025 14:23:47 -0700 Subject: [PATCH 29/70] macos: show the copy menu item if we have any text selected --- macos/Sources/Ghostty/SurfaceView_AppKit.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 4b47949e2..97637e737 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -1327,7 +1327,7 @@ extension Ghostty { var item: NSMenuItem // If we have a selection, add copy - if self.selectedRange().length > 0 { + if let text = self.accessibilitySelectedText(), text.count > 0 { menu.addItem(withTitle: "Copy", action: #selector(copy(_:)), keyEquivalent: "") } menu.addItem(withTitle: "Paste", action: #selector(paste(_:)), keyEquivalent: "") From babe923c8c0460563a5f49e50e361fdab634ec47 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 19 Aug 2025 14:40:42 -0700 Subject: [PATCH 30/70] AI tooling must be disclosed for contributions --- CONTRIBUTING.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 18cd7ca90..880dc37d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,40 @@ we can always convert that to an issue later. > time to fixing bugs, maintaining features, and reviewing code, I do kindly > ask you spend a few minutes reading this document. Thank you. ❤️ +## AI Assistance Notice + +> [!IMPORTANT] +> +> If you are using **any kind of AI assistance** to contribute to Ghostty, +> it must be disclosed in the pull request. + +If you are using any kind of AI assistance while contributing to Ghostty, +**this must be disclosed in the pull request**, along with the extent to +which AI assistance was used (e.g. docs only vs. code generation). +If PR responses are being generated by an AI, disclose that as well. +As a small exception, trivial tab-completion doesn't need to be disclosed, +so long as it is limited to single keywords or short phrases. + +An example disclosure: + +> This PR was written primarily by Claude Code. + +Or a more detailed disclosure: + +> I consulted ChatGPT to understand the codebase but the solution +> was fully authored manually by myself. + +Failure to disclose this is first and foremost rude to the human operators +on the other end of the pull request, but it also makes it difficult to +determine how much scrutiny to apply to the contribution. + +In a perfect world, AI assistance would produce equal or higher quality +work than any human. That isn't the world we live in today, and in most cases +it's generating slop. I say this despite being a fan of and using them +successfully myself (with heavy supervision)! + +Please be respectful to maintainers and disclose AI assistance. + ## Quick Guide **I'd like to contribute!** From b4833c83cc2ff974cbac40f16e36d399f66ef759 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 19 Aug 2025 15:29:05 -0700 Subject: [PATCH 31/70] ci: workaround snap builder issues Workaround produced by Namespace support. Thanks! --- .github/workflows/test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c00816b38..8ba47a99a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -426,6 +426,11 @@ jobs: _LXD_SNAP_DEVCGROUP_CONFIG="/var/lib/snapd/cgroup/snap.lxd.device" sudo mkdir -p /var/lib/snapd/cgroup echo 'self-managed=true' | sudo tee "${_LXD_SNAP_DEVCGROUP_CONFIG}" + # Workaround from a change introduced between 8.10.2 and 8.11.1 where the + # user runtime directory was changed. + - run: | + sudo mkdir /run/user/1001 + sudo chown 1001 /run/user/1001 - uses: snapcore/action-build@3bdaa03e1ba6bf59a65f84a751d943d549a54e79 # v1.3.0 with: path: dist From aa26f8fd3423b23128f6875ef80704287ad6b405 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Tue, 19 Aug 2025 21:49:05 -0500 Subject: [PATCH 32/70] snap: remove workaround for build failures --- .github/workflows/test.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8ba47a99a..c00816b38 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -426,11 +426,6 @@ jobs: _LXD_SNAP_DEVCGROUP_CONFIG="/var/lib/snapd/cgroup/snap.lxd.device" sudo mkdir -p /var/lib/snapd/cgroup echo 'self-managed=true' | sudo tee "${_LXD_SNAP_DEVCGROUP_CONFIG}" - # Workaround from a change introduced between 8.10.2 and 8.11.1 where the - # user runtime directory was changed. - - run: | - sudo mkdir /run/user/1001 - sudo chown 1001 /run/user/1001 - uses: snapcore/action-build@3bdaa03e1ba6bf59a65f84a751d943d549a54e79 # v1.3.0 with: path: dist From 60327320015bcf527a6a3ba76ebc171a2a23baa0 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Wed, 20 Aug 2025 09:03:48 -0500 Subject: [PATCH 33/70] contributing: add some notes about running valgrind --- CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++++++++++++++ flake.nix | 13 +++++++++++++ 2 files changed, 54 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 880dc37d9..a525b5d54 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -133,6 +133,47 @@ pull request will be accepted with a high degree of certainty. See the [Contributor's Guide](po/README_CONTRIBUTORS.md) for more details. +## Checking for Memory Leaks + +While Zig does an amazing job of finding and preventing memory leaks, +Ghostty uses many third-party libraries that are written in C. Improper usage +of those libraries or bugs in those libraries can cause memory leaks that +Zig cannot detect by itself. + +### On Linux + +On Linux the recommended tool to check for memory leaks is Valgrind. We supply +a file containing suppressions for false positives and known leaks in 3rd party +libraries. The recommended way to run Valgrind is: + +``` +zig build -Dcpu=baseline -Doptimize=Debug +valgrind \ + --leak-check=full \ + --num-callers=50 \ + --suppressions=valgrind.supp \ + ./zig-out/bin/ghostty +``` + +> [!NOTE] +> +> `-Dcpu=baseline` may not be needed depending on your CPU, but Valgrind cannot +> deal with some instructions on certain newer CPUs so using `-Dcpu=baseline` +> doesn't hurt. + +Any leaks found by Valgrind should be investigated. + +If you use Nix, you can use the following commands to run Valgrind so that you +don't need to look up the Valgrind invocation every time: + +``` +nix develop +nix run .#valgrind -- --config-default-files=true +``` + +You can add any Ghostty CLI arguments after the `--` and they will be passed to +the invocation of Ghostty. + ## Input Stack Testing The input stack is the part of the codebase that starts with a diff --git a/flake.nix b/flake.nix index 7cf58b27c..1c36b64ac 100644 --- a/flake.nix +++ b/flake.nix @@ -94,6 +94,19 @@ x11-gnome = runVM ./nix/vm/x11-gnome.nix; x11-plasma6 = runVM ./nix/vm/x11-plasma6.nix; x11-xfce = runVM ./nix/vm/x11-xfce.nix; + valgrind = let + script = pkgs.writeShellScript "valgrind" '' + zig build -Dcpu=baseline -Doptimize=Debug + valgrind \ + --leak-check=full \ + --num-callers=50 \ + --suppressions=valgrind.supp \ + ./zig-out/bin/ghostty "$@" + ''; + in { + type = "app"; + program = "${script}"; + }; }; } # Our supported systems are the same supported systems as the Zig binaries. From 3cce5d26d79378eb1dafee6e9c643ea020cbcf6f Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Wed, 20 Aug 2025 10:08:24 -0500 Subject: [PATCH 34/70] wuffs: simplify the build --- pkg/wuffs/build.zig | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pkg/wuffs/build.zig b/pkg/wuffs/build.zig index b151892ed..57d89e6b6 100644 --- a/pkg/wuffs/build.zig +++ b/pkg/wuffs/build.zig @@ -13,11 +13,7 @@ pub fn build(b: *std.Build) !void { const unit_tests = b.addTest(.{ .name = "test", - .root_module = b.createModule(.{ - .root_source_file = b.path("src/main.zig"), - .target = target, - .optimize = optimize, - }), + .root_module = module, }); unit_tests.linkLibC(); @@ -34,12 +30,6 @@ pub fn build(b: *std.Build) !void { .file = wuffs_dep.path("release/c/wuffs-v0.4.c"), .flags = flags.items, }); - - unit_tests.addIncludePath(wuffs_dep.path("release/c")); - unit_tests.addCSourceFile(.{ - .file = wuffs_dep.path("release/c/wuffs-v0.4.c"), - .flags = flags.items, - }); } if (b.lazyDependency("pixels", .{})) |pixels_dep| { From f87213c2f68867c7cca2befdd0a75d94f289e2b6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 11:09:05 -0700 Subject: [PATCH 35/70] build: add run-valgrind and test-valgrind steps This adds two explicit `zig build` steps: `run-valgrind` and `test-valgrind` to run the Ghostty exe or tests under Valgrind, respectively. This simplifies the manual Valgrind calls in a few ways: 1. It automatically sets the CPU to baseline, which is a frequent and requirement for Valgrind on newer CPUs, and generally safe. 2. It sets up the rather complicated set of flags to call Valgrind with, importantly setting up our suppressions. 3. It enables pairing it with the typical and comfortable workflow of specifying extra args (with `--`) or flags like `-Dtest-filter` for tests. --- CONTRIBUTING.md | 35 ++++++-------------------- build.zig | 58 ++++++++++++++++++++++++++++++++++++++++---- flake.nix | 13 ---------- src/build/Config.zig | 23 ++++++++++++++++++ 4 files changed, 84 insertions(+), 45 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a525b5d54..0e988704b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -142,37 +142,18 @@ Zig cannot detect by itself. ### On Linux -On Linux the recommended tool to check for memory leaks is Valgrind. We supply -a file containing suppressions for false positives and known leaks in 3rd party -libraries. The recommended way to run Valgrind is: +On Linux the recommended tool to check for memory leaks is Valgrind. The +recommended way to run Valgrind is via `zig build`: -``` -zig build -Dcpu=baseline -Doptimize=Debug -valgrind \ - --leak-check=full \ - --num-callers=50 \ - --suppressions=valgrind.supp \ - ./zig-out/bin/ghostty +```sh +zig build run-valgrind ``` -> [!NOTE] -> -> `-Dcpu=baseline` may not be needed depending on your CPU, but Valgrind cannot -> deal with some instructions on certain newer CPUs so using `-Dcpu=baseline` -> doesn't hurt. +This builds a Ghostty executable with Valgrind support and runs Valgrind +with the proper flags to ensure we're suppressing known false positives. -Any leaks found by Valgrind should be investigated. - -If you use Nix, you can use the following commands to run Valgrind so that you -don't need to look up the Valgrind invocation every time: - -``` -nix develop -nix run .#valgrind -- --config-default-files=true -``` - -You can add any Ghostty CLI arguments after the `--` and they will be passed to -the invocation of Ghostty. +You can combine the same build args with `run-valgrind` that you can with +`run`, such as specifying additional configurations after a trailing `--`. ## Input Stack Testing diff --git a/build.zig b/build.zig index 4acca53cc..38cfd0e56 100644 --- a/build.zig +++ b/build.zig @@ -19,7 +19,15 @@ pub fn build(b: *std.Build) !void { // All our steps which we'll hook up later. The steps are shown // up here just so that they are more self-documenting. const run_step = b.step("run", "Run the app"); - const test_step = b.step("test", "Run all tests"); + const run_valgrind_step = b.step( + "run-valgrind", + "Run the app under valgrind", + ); + const test_step = b.step("test", "Run tests"); + const test_valgrind_step = b.step( + "test-valgrind", + "Run tests under valgrind", + ); const translations_step = b.step( "update-translations", "Update translation files", @@ -77,9 +85,11 @@ pub fn build(b: *std.Build) !void { // Runtime "none" is libghostty, anything else is an executable. if (config.app_runtime != .none) { - exe.install(); - resources.install(); - if (i18n) |v| v.install(); + if (config.emit_exe) { + exe.install(); + resources.install(); + if (i18n) |v| v.install(); + } } else { // Libghostty // @@ -181,6 +191,31 @@ pub fn build(b: *std.Build) !void { } } + // Valgrind + if (config.app_runtime != .none) { + // We need to rebuild Ghostty with a baseline CPU target. + const valgrind_exe = exe: { + var valgrind_config = config; + valgrind_config.target = valgrind_config.baselineTarget(); + break :exe try buildpkg.GhosttyExe.init( + b, + &valgrind_config, + &deps, + ); + }; + + const run_cmd = b.addSystemCommand(&.{ + "valgrind", + "--leak-check=full", + "--num-callers=50", + b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}), + "--gen-suppressions=all", + }); + run_cmd.addArtifactArg(valgrind_exe.exe); + if (b.args) |args| run_cmd.addArgs(args); + run_valgrind_step.dependOn(&run_cmd.step); + } + // Tests { const test_exe = b.addTest(.{ @@ -188,7 +223,7 @@ pub fn build(b: *std.Build) !void { .filters = if (test_filter) |v| &.{v} else &.{}, .root_module = b.createModule(.{ .root_source_file = b.path("src/main.zig"), - .target = config.target, + .target = config.baselineTarget(), .optimize = .Debug, .strip = false, .omit_frame_pointer = false, @@ -198,8 +233,21 @@ pub fn build(b: *std.Build) !void { if (config.emit_test_exe) b.installArtifact(test_exe); _ = try deps.add(test_exe); + + // Normal test running const test_run = b.addRunArtifact(test_exe); test_step.dependOn(&test_run.step); + + // Valgrind test running + const valgrind_run = b.addSystemCommand(&.{ + "valgrind", + "--leak-check=full", + "--num-callers=50", + b.fmt("--suppressions={s}", .{b.pathFromRoot("valgrind.supp")}), + "--gen-suppressions=all", + }); + valgrind_run.addArtifactArg(test_exe); + test_valgrind_step.dependOn(&valgrind_run.step); } // update-translations does what it sounds like and updates the "pot" diff --git a/flake.nix b/flake.nix index 1c36b64ac..7cf58b27c 100644 --- a/flake.nix +++ b/flake.nix @@ -94,19 +94,6 @@ x11-gnome = runVM ./nix/vm/x11-gnome.nix; x11-plasma6 = runVM ./nix/vm/x11-plasma6.nix; x11-xfce = runVM ./nix/vm/x11-xfce.nix; - valgrind = let - script = pkgs.writeShellScript "valgrind" '' - zig build -Dcpu=baseline -Doptimize=Debug - valgrind \ - --leak-check=full \ - --num-callers=50 \ - --suppressions=valgrind.supp \ - ./zig-out/bin/ghostty "$@" - ''; - in { - type = "app"; - program = "${script}"; - }; }; } # Our supported systems are the same supported systems as the Zig binaries. diff --git a/src/build/Config.zig b/src/build/Config.zig index 175745dc6..fd892f16c 100644 --- a/src/build/Config.zig +++ b/src/build/Config.zig @@ -53,6 +53,7 @@ patch_rpath: ?[]const u8 = null, flatpak: bool = false, emit_bench: bool = false, emit_docs: bool = false, +emit_exe: bool = false, emit_helpgen: bool = false, emit_macos_app: bool = false, emit_terminfo: bool = false, @@ -286,6 +287,12 @@ pub fn init(b: *std.Build) !Config { //--------------------------------------------------------------- // Artifacts to Emit + config.emit_exe = b.option( + bool, + "emit-exe", + "Build and install main executables with 'build'", + ) orelse true; + config.emit_test_exe = b.option( bool, "emit-test-exe", @@ -460,6 +467,22 @@ pub fn addOptions(self: *const Config, step: *std.Build.Step.Options) !void { ); } +/// Returns a baseline CPU target retaining all the other CPU configs. +pub fn baselineTarget(self: *const Config) std.Build.ResolvedTarget { + // Set our cpu model as baseline. There may need to be other modifications + // we need to make such as resetting CPU features but for now this works. + var q = self.target.query; + q.cpu_model = .baseline; + + // Same logic as build.resolveTargetQuery but we don't need to + // handle the native case. + return .{ + .query = q, + .result = std.zig.system.resolveTargetQuery(q) catch + @panic("unable to resolve baseline query"), + }; +} + /// Rehydrate our Config from the comptime options. Note that not all /// options are available at comptime, so look closely at this implementation /// to see what is and isn't available. From 131f170f8985cc89c960a02b1b3aae8f3ad9e7a5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 12:06:09 -0700 Subject: [PATCH 36/70] terminal: change OSC parser to explicit init to set undefined This works around: https://github.com/ziglang/zig/issues/19148 This lets our `test-valgrind` command catch some issues. We'll have to follow this pattern in more places but I want to do it incrementally so things keep passing. I **do not** want to blindly follow this pattern everywhere. I want to start by focusing in only on the structs that set `undefined` as default fields that we're also about to test in isolation with Valgrind. Its just too much noise otherwise and not a general style I'm sure of; it's worth it for Valgrind though. --- .github/workflows/test.yml | 52 +++---- src/benchmark/TerminalParser.zig | 2 +- src/benchmark/TerminalStream.zig | 2 +- src/inspector/Inspector.zig | 11 +- src/synthetic/Osc.zig | 4 +- src/terminal/Parser.zig | 6 +- src/terminal/osc.zig | 234 ++++++++++++++++++------------- src/terminal/stream.zig | 82 ++++++----- src/termio/Termio.zig | 15 +- 9 files changed, 222 insertions(+), 186 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c00816b38..44e1e30fc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,7 +22,6 @@ jobs: - build-macos-matrix - build-windows - flatpak-check-zig-cache - - flatpak - test - test-gtk - test-gtk-ng @@ -1013,28 +1012,29 @@ jobs: - name: Check Flatpak Zig Dependencies run: nix develop -c ./flatpak/build-support/check-zig-cache.sh - flatpak: - if: github.repository == 'ghostty-org/ghostty' - name: "Flatpak" - container: - image: ghcr.io/flathub-infra/flatpak-github-actions:gnome-47 - options: --privileged - strategy: - fail-fast: false - matrix: - variant: - - arch: x86_64 - runner: namespace-profile-ghostty-md - - arch: aarch64 - runner: namespace-profile-ghostty-md-arm64 - runs-on: ${{ matrix.variant.runner }} - needs: [flatpak-check-zig-cache, test] - steps: - - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - - uses: flatpak/flatpak-github-actions/flatpak-builder@10a3c29f0162516f0f68006be14c92f34bd4fa6c # v6.5 - with: - bundle: com.mitchellh.ghostty - manifest-path: flatpak/com.mitchellh.ghostty.yml - cache-key: flatpak-builder-${{ github.sha }} - arch: ${{ matrix.variant.arch }} - verbose: true + # Disabled until we update to Zig 0.15 or if we can pin this to Zig 0.14 + # flatpak: + # if: github.repository == 'ghostty-org/ghostty' + # name: "Flatpak" + # container: + # image: ghcr.io/flathub-infra/flatpak-github-actions:gnome-47 + # options: --privileged + # strategy: + # fail-fast: false + # matrix: + # variant: + # - arch: x86_64 + # runner: namespace-profile-ghostty-md + # - arch: aarch64 + # runner: namespace-profile-ghostty-md-arm64 + # runs-on: ${{ matrix.variant.runner }} + # needs: [flatpak-check-zig-cache, test] + # steps: + # - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + # - uses: flatpak/flatpak-github-actions/flatpak-builder@10a3c29f0162516f0f68006be14c92f34bd4fa6c # v6.5 + # with: + # bundle: com.mitchellh.ghostty + # manifest-path: flatpak/com.mitchellh.ghostty.yml + # cache-key: flatpak-builder-${{ github.sha }} + # arch: ${{ matrix.variant.arch }} + # verbose: true diff --git a/src/benchmark/TerminalParser.zig b/src/benchmark/TerminalParser.zig index 9107d4555..002af4831 100644 --- a/src/benchmark/TerminalParser.zig +++ b/src/benchmark/TerminalParser.zig @@ -77,7 +77,7 @@ fn step(ptr: *anyopaque) Benchmark.Error!void { const f = self.data_f orelse return; var r = std.io.bufferedReader(f.reader()); - var p: terminalpkg.Parser = .{}; + var p: terminalpkg.Parser = .init(); var buf: [4096]u8 = undefined; while (true) { diff --git a/src/benchmark/TerminalStream.zig b/src/benchmark/TerminalStream.zig index 5d235c4ee..28a95226c 100644 --- a/src/benchmark/TerminalStream.zig +++ b/src/benchmark/TerminalStream.zig @@ -62,7 +62,7 @@ pub fn create( .cols = opts.@"terminal-cols", }), .handler = .{ .t = &ptr.terminal }, - .stream = .{ .handler = &ptr.handler }, + .stream = .init(&ptr.handler), }; return ptr; diff --git a/src/inspector/Inspector.zig b/src/inspector/Inspector.zig index d3e7fcaaa..27abb8657 100644 --- a/src/inspector/Inspector.zig +++ b/src/inspector/Inspector.zig @@ -172,13 +172,10 @@ pub fn init(surface: *Surface) !Inspector { .surface = surface, .key_events = key_buf, .vt_events = vt_events, - .vt_stream = .{ - .handler = vt_handler, - .parser = .{ - .osc_parser = .{ - .alloc = surface.alloc, - }, - }, + .vt_stream = stream: { + var s: inspector.termio.Stream = .init(vt_handler); + s.parser.osc_parser.alloc = surface.alloc; + break :stream s; }, }; } diff --git a/src/synthetic/Osc.zig b/src/synthetic/Osc.zig index e0a6b42a0..8d5d7d3a2 100644 --- a/src/synthetic/Osc.zig +++ b/src/synthetic/Osc.zig @@ -196,7 +196,7 @@ test "OSC generator valid" { }; for (0..50) |_| { const seq = try gen.next(&buf); - var parser: terminal.osc.Parser = .{}; + var parser: terminal.osc.Parser = .init(); for (seq[2 .. seq.len - 1]) |c| parser.next(c); try testing.expect(parser.end(null) != null); } @@ -214,7 +214,7 @@ test "OSC generator invalid" { }; for (0..50) |_| { const seq = try gen.next(&buf); - var parser: terminal.osc.Parser = .{}; + var parser: terminal.osc.Parser = .init(); for (seq[2 .. seq.len - 1]) |c| parser.next(c); try testing.expect(parser.end(null) == null); } diff --git a/src/terminal/Parser.zig b/src/terminal/Parser.zig index ec3f322f6..ed099ee47 100644 --- a/src/terminal/Parser.zig +++ b/src/terminal/Parser.zig @@ -223,10 +223,12 @@ param_acc: u16 = 0, param_acc_idx: u8 = 0, /// Parser for OSC sequences -osc_parser: osc.Parser = .{}, +osc_parser: osc.Parser, pub fn init() Parser { - return .{}; + return .{ + .osc_parser = .init(), + }; } pub fn deinit(self: *Parser) void { diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index 7619c82c1..00ede49ce 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -282,23 +282,23 @@ pub const Parser = struct { /// Optional allocator used to accept data longer than MAX_BUF. /// This only applies to some commands (e.g. OSC 52) that can /// reasonably exceed MAX_BUF. - alloc: ?Allocator = null, + alloc: ?Allocator, /// Current state of the parser. - state: State = .empty, + state: State, /// Current command of the parser, this accumulates. - command: Command = undefined, + command: Command, /// Buffer that stores the input we see for a single OSC command. /// Slices in Command are offsets into this buffer. - buf: [MAX_BUF]u8 = undefined, - buf_start: usize = 0, - buf_idx: usize = 0, - buf_dynamic: ?*std.ArrayListUnmanaged(u8) = null, + buf: [MAX_BUF]u8, + buf_start: usize, + buf_idx: usize, + buf_dynamic: ?*std.ArrayListUnmanaged(u8), /// True when a command is complete/valid to return. - complete: bool = false, + complete: bool, /// Temporary state that is dependent on the current state. temp_state: union { @@ -310,7 +310,7 @@ pub const Parser = struct { /// Temporary state for key/value pairs key: []const u8, - } = undefined, + }, // Maximum length of a single OSC command. This is the full OSC command // sequence length (excluding ESC ]). This is arbitrary, I couldn't find @@ -429,6 +429,38 @@ pub const Parser = struct { conemu_progress_value, }; + pub fn init() Parser { + var result: Parser = .{ + .alloc = null, + .state = .empty, + .buf_start = 0, + .buf_idx = 0, + .buf_dynamic = null, + .complete = false, + + // Keeping all our undefined values together so we can + // visually easily duplicate them in the Valgrind check below. + .command = undefined, + .buf = undefined, + .temp_state = undefined, + }; + if (std.valgrind.runningOnValgrind() > 0) { + // Initialize our undefined fields so Valgrind can catch it. + // https://github.com/ziglang/zig/issues/19148 + result.command = undefined; + result.buf = undefined; + result.temp_state = undefined; + } + + return result; + } + + pub fn initAlloc(alloc: Allocator) Parser { + var result: Parser = .init(); + result.alloc = alloc; + return result; + } + /// This must be called to clean up any allocated memory. pub fn deinit(self: *Parser) void { self.reset(); @@ -1590,7 +1622,7 @@ pub const Parser = struct { test "OSC: change_window_title" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); p.next('0'); p.next(';'); p.next('a'); @@ -1603,7 +1635,7 @@ test "OSC: change_window_title" { test "OSC: change_window_title with 2" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); p.next('2'); p.next(';'); p.next('a'); @@ -1616,7 +1648,7 @@ test "OSC: change_window_title with 2" { test "OSC: change_window_title with utf8" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); p.next('2'); p.next(';'); // '—' EM DASH U+2014 (E2 80 94) @@ -1638,7 +1670,7 @@ test "OSC: change_window_title with utf8" { test "OSC: change_window_title empty" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); p.next('2'); p.next(';'); const cmd = p.end(null).?; @@ -1649,7 +1681,7 @@ test "OSC: change_window_title empty" { test "OSC: change_window_icon" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); p.next('1'); p.next(';'); p.next('a'); @@ -1662,7 +1694,7 @@ test "OSC: change_window_icon" { test "OSC: prompt_start" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A"; for (input) |ch| p.next(ch); @@ -1676,7 +1708,7 @@ test "OSC: prompt_start" { test "OSC: prompt_start with single option" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A;aid=14"; for (input) |ch| p.next(ch); @@ -1689,7 +1721,7 @@ test "OSC: prompt_start with single option" { test "OSC: prompt_start with redraw disabled" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A;redraw=0"; for (input) |ch| p.next(ch); @@ -1702,7 +1734,7 @@ test "OSC: prompt_start with redraw disabled" { test "OSC: prompt_start with redraw invalid value" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A;redraw=42"; for (input) |ch| p.next(ch); @@ -1716,7 +1748,7 @@ test "OSC: prompt_start with redraw invalid value" { test "OSC: prompt_start with continuation" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A;k=c"; for (input) |ch| p.next(ch); @@ -1729,7 +1761,7 @@ test "OSC: prompt_start with continuation" { test "OSC: prompt_start with secondary" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;A;k=s"; for (input) |ch| p.next(ch); @@ -1742,7 +1774,7 @@ test "OSC: prompt_start with secondary" { test "OSC: end_of_command no exit code" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;D"; for (input) |ch| p.next(ch); @@ -1754,7 +1786,7 @@ test "OSC: end_of_command no exit code" { test "OSC: end_of_command with exit code" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;D;25"; for (input) |ch| p.next(ch); @@ -1767,7 +1799,7 @@ test "OSC: end_of_command with exit code" { test "OSC: prompt_end" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;B"; for (input) |ch| p.next(ch); @@ -1779,7 +1811,7 @@ test "OSC: prompt_end" { test "OSC: end_of_input" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "133;C"; for (input) |ch| p.next(ch); @@ -1791,7 +1823,7 @@ test "OSC: end_of_input" { test "OSC: OSC110: reset foreground color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "110"; @@ -1817,7 +1849,7 @@ test "OSC: OSC110: reset foreground color" { test "OSC: OSC111: reset background color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "111"; @@ -1843,7 +1875,7 @@ test "OSC: OSC111: reset background color" { test "OSC: OSC112: reset cursor color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "112"; @@ -1869,7 +1901,7 @@ test "OSC: OSC112: reset cursor color" { test "OSC: OSC112: reset cursor color with semicolon" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "112;"; @@ -1896,7 +1928,7 @@ test "OSC: OSC112: reset cursor color with semicolon" { test "OSC: get/set clipboard" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "52;s;?"; for (input) |ch| p.next(ch); @@ -1910,7 +1942,7 @@ test "OSC: get/set clipboard" { test "OSC: get/set clipboard (optional parameter)" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "52;;?"; for (input) |ch| p.next(ch); @@ -1924,7 +1956,7 @@ test "OSC: get/set clipboard (optional parameter)" { test "OSC: get/set clipboard with allocator" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "52;s;?"; @@ -1939,7 +1971,7 @@ test "OSC: get/set clipboard with allocator" { test "OSC: clear clipboard" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .init(); defer p.deinit(); const input = "52;;"; @@ -1954,7 +1986,7 @@ test "OSC: clear clipboard" { test "OSC: report pwd" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "7;file:///tmp/example"; for (input) |ch| p.next(ch); @@ -1967,7 +1999,7 @@ test "OSC: report pwd" { test "OSC: report pwd empty" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "7;"; for (input) |ch| p.next(ch); @@ -1979,7 +2011,7 @@ test "OSC: report pwd empty" { test "OSC: pointer cursor" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "22;pointer"; for (input) |ch| p.next(ch); @@ -1992,7 +2024,7 @@ test "OSC: pointer cursor" { test "OSC: longer than buffer" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "0;" ++ "a" ** (Parser.MAX_BUF + 2); for (input) |ch| p.next(ch); @@ -2004,7 +2036,7 @@ test "OSC: longer than buffer" { test "OSC: OSC10: report foreground color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "10;?"; @@ -2032,7 +2064,7 @@ test "OSC: OSC10: report foreground color" { test "OSC: OSC10: set foreground color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "10;rgbi:0.0/0.5/1.0"; @@ -2062,7 +2094,7 @@ test "OSC: OSC10: set foreground color" { test "OSC: OSC11: report background color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "11;?"; @@ -2090,7 +2122,7 @@ test "OSC: OSC11: report background color" { test "OSC: OSC11: set background color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "11;rgb:f/ff/ffff"; @@ -2120,7 +2152,7 @@ test "OSC: OSC11: set background color" { test "OSC: OSC12: report cursor color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "12;?"; @@ -2148,7 +2180,7 @@ test "OSC: OSC12: report cursor color" { test "OSC: OSC12: set cursor color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "12;rgb:f/ff/ffff"; @@ -2178,7 +2210,7 @@ test "OSC: OSC12: set cursor color" { test "OSC: OSC4: get palette color 1" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;1;?"; @@ -2204,7 +2236,7 @@ test "OSC: OSC4: get palette color 1" { test "OSC: OSC4: get palette color 2" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;1;?;2;?"; @@ -2238,7 +2270,7 @@ test "OSC: OSC4: get palette color 2" { test "OSC: OSC4: set palette color 1" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;17;rgb:aa/bb/cc"; @@ -2267,7 +2299,7 @@ test "OSC: OSC4: set palette color 1" { test "OSC: OSC4: set palette color 2" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;17;rgb:aa/bb/cc;1;rgb:00/11/22"; @@ -2308,7 +2340,7 @@ test "OSC: OSC4: set palette color 2" { test "OSC: OSC4: get with invalid index 1" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;1111;?;1;?"; @@ -2333,7 +2365,7 @@ test "OSC: OSC4: get with invalid index 1" { test "OSC: OSC4: get with invalid index 2" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;5;?;1111;?;1;?"; @@ -2367,7 +2399,7 @@ test "OSC: OSC4: get with invalid index 2" { test "OSC: OSC4: multiple get 8a" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;0;?;1;?;2;?;3;?;4;?;5;?;6;?;7;?"; @@ -2449,7 +2481,7 @@ test "OSC: OSC4: multiple get 8a" { test "OSC: OSC4: multiple get 8b" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;8;?;9;?;10;?;11;?;12;?;13;?;14;?;15;?"; @@ -2530,7 +2562,7 @@ test "OSC: OSC4: multiple get 8b" { test "OSC: OSC4: set with invalid index" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;256;#ffffff;1;#aabbcc"; @@ -2559,7 +2591,7 @@ test "OSC: OSC4: set with invalid index" { test "OSC: OSC4: mix get/set palette color" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;17;rgb:aa/bb/cc;254;?"; @@ -2596,7 +2628,7 @@ test "OSC: OSC4: mix get/set palette color" { test "OSC: OSC4: incomplete color/spec 1" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;17"; @@ -2613,7 +2645,7 @@ test "OSC: OSC4: incomplete color/spec 1" { test "OSC: OSC4: incomplete color/spec 2" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "4;17;?;42"; @@ -2638,7 +2670,7 @@ test "OSC: OSC4: incomplete color/spec 2" { test "OSC: OSC104: reset palette color 1" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "104;17"; @@ -2663,7 +2695,7 @@ test "OSC: OSC104: reset palette color 1" { test "OSC: OSC104: reset palette color 2" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "104;17;111"; @@ -2696,7 +2728,7 @@ test "OSC: OSC104: reset palette color 2" { test "OSC: OSC104: invalid palette index" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "104;ffff;111"; @@ -2721,7 +2753,7 @@ test "OSC: OSC104: invalid palette index" { test "OSC: OSC104: empty palette index" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "104;;111"; @@ -2746,7 +2778,7 @@ test "OSC: OSC104: empty palette index" { test "OSC: conemu sleep" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;1;420"; for (input) |ch| p.next(ch); @@ -2760,7 +2792,7 @@ test "OSC: conemu sleep" { test "OSC: conemu sleep with no value default to 100ms" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;1;"; for (input) |ch| p.next(ch); @@ -2774,7 +2806,7 @@ test "OSC: conemu sleep with no value default to 100ms" { test "OSC: conemu sleep cannot exceed 10000ms" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;1;12345"; for (input) |ch| p.next(ch); @@ -2788,7 +2820,7 @@ test "OSC: conemu sleep cannot exceed 10000ms" { test "OSC: conemu sleep invalid input" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;1;foo"; for (input) |ch| p.next(ch); @@ -2802,7 +2834,7 @@ test "OSC: conemu sleep invalid input" { test "OSC: show desktop notification" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;Hello world"; for (input) |ch| p.next(ch); @@ -2816,7 +2848,7 @@ test "OSC: show desktop notification" { test "OSC: show desktop notification with title" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "777;notify;Title;Body"; for (input) |ch| p.next(ch); @@ -2830,7 +2862,7 @@ test "OSC: show desktop notification with title" { test "OSC: conemu message box" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;2;hello world"; for (input) |ch| p.next(ch); @@ -2843,7 +2875,7 @@ test "OSC: conemu message box" { test "OSC: conemu message box invalid input" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;2"; for (input) |ch| p.next(ch); @@ -2855,7 +2887,7 @@ test "OSC: conemu message box invalid input" { test "OSC: conemu message box empty message" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;2;"; for (input) |ch| p.next(ch); @@ -2868,7 +2900,7 @@ test "OSC: conemu message box empty message" { test "OSC: conemu message box spaces only message" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;2; "; for (input) |ch| p.next(ch); @@ -2881,7 +2913,7 @@ test "OSC: conemu message box spaces only message" { test "OSC: conemu change tab title" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;3;foo bar"; for (input) |ch| p.next(ch); @@ -2894,7 +2926,7 @@ test "OSC: conemu change tab title" { test "OSC: conemu change tab reset title" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;3;"; for (input) |ch| p.next(ch); @@ -2908,7 +2940,7 @@ test "OSC: conemu change tab reset title" { test "OSC: conemu change tab spaces only title" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;3; "; for (input) |ch| p.next(ch); @@ -2922,7 +2954,7 @@ test "OSC: conemu change tab spaces only title" { test "OSC: conemu change tab invalid input" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;3"; for (input) |ch| p.next(ch); @@ -2934,7 +2966,7 @@ test "OSC: conemu change tab invalid input" { test "OSC: OSC9 progress set" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;1;100"; for (input) |ch| p.next(ch); @@ -2948,7 +2980,7 @@ test "OSC: OSC9 progress set" { test "OSC: OSC9 progress set overflow" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;1;900"; for (input) |ch| p.next(ch); @@ -2962,7 +2994,7 @@ test "OSC: OSC9 progress set overflow" { test "OSC: OSC9 progress set single digit" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;1;9"; for (input) |ch| p.next(ch); @@ -2976,7 +3008,7 @@ test "OSC: OSC9 progress set single digit" { test "OSC: OSC9 progress set double digit" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;1;94"; for (input) |ch| p.next(ch); @@ -2990,7 +3022,7 @@ test "OSC: OSC9 progress set double digit" { test "OSC: OSC9 progress set extra semicolon ignored" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;1;100"; for (input) |ch| p.next(ch); @@ -3004,7 +3036,7 @@ test "OSC: OSC9 progress set extra semicolon ignored" { test "OSC: OSC9 progress remove with no progress" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;0;"; for (input) |ch| p.next(ch); @@ -3018,7 +3050,7 @@ test "OSC: OSC9 progress remove with no progress" { test "OSC: OSC9 progress remove with double semicolon" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;0;;"; for (input) |ch| p.next(ch); @@ -3032,7 +3064,7 @@ test "OSC: OSC9 progress remove with double semicolon" { test "OSC: OSC9 progress remove ignores progress" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;0;100"; for (input) |ch| p.next(ch); @@ -3046,7 +3078,7 @@ test "OSC: OSC9 progress remove ignores progress" { test "OSC: OSC9 progress remove extra semicolon" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;0;100;"; for (input) |ch| p.next(ch); @@ -3059,7 +3091,7 @@ test "OSC: OSC9 progress remove extra semicolon" { test "OSC: OSC9 progress error" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;2"; for (input) |ch| p.next(ch); @@ -3073,7 +3105,7 @@ test "OSC: OSC9 progress error" { test "OSC: OSC9 progress error with progress" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;2;100"; for (input) |ch| p.next(ch); @@ -3087,7 +3119,7 @@ test "OSC: OSC9 progress error with progress" { test "OSC: OSC9 progress pause" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;4"; for (input) |ch| p.next(ch); @@ -3101,7 +3133,7 @@ test "OSC: OSC9 progress pause" { test "OSC: OSC9 progress pause with progress" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;4;4;100"; for (input) |ch| p.next(ch); @@ -3115,7 +3147,7 @@ test "OSC: OSC9 progress pause with progress" { test "OSC: OSC9 conemu wait input" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;5"; for (input) |ch| p.next(ch); @@ -3127,7 +3159,7 @@ test "OSC: OSC9 conemu wait input" { test "OSC: OSC9 conemu wait ignores trailing characters" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "9;5;foo"; for (input) |ch| p.next(ch); @@ -3139,7 +3171,7 @@ test "OSC: OSC9 conemu wait ignores trailing characters" { test "OSC: empty param" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "4;;"; for (input) |ch| p.next(ch); @@ -3151,7 +3183,7 @@ test "OSC: empty param" { test "OSC: hyperlink" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;;http://example.com"; for (input) |ch| p.next(ch); @@ -3164,7 +3196,7 @@ test "OSC: hyperlink" { test "OSC: hyperlink with id set" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;id=foo;http://example.com"; for (input) |ch| p.next(ch); @@ -3178,7 +3210,7 @@ test "OSC: hyperlink with id set" { test "OSC: hyperlink with empty id" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;id=;http://example.com"; for (input) |ch| p.next(ch); @@ -3192,7 +3224,7 @@ test "OSC: hyperlink with empty id" { test "OSC: hyperlink with incomplete key" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;id;http://example.com"; for (input) |ch| p.next(ch); @@ -3206,7 +3238,7 @@ test "OSC: hyperlink with incomplete key" { test "OSC: hyperlink with empty key" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;=value;http://example.com"; for (input) |ch| p.next(ch); @@ -3220,7 +3252,7 @@ test "OSC: hyperlink with empty key" { test "OSC: hyperlink with empty key and id" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;=value:id=foo;http://example.com"; for (input) |ch| p.next(ch); @@ -3234,7 +3266,7 @@ test "OSC: hyperlink with empty key and id" { test "OSC: hyperlink with empty uri" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;id=foo;"; for (input) |ch| p.next(ch); @@ -3246,7 +3278,7 @@ test "OSC: hyperlink with empty uri" { test "OSC: hyperlink end" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); const input = "8;;"; for (input) |ch| p.next(ch); @@ -3259,7 +3291,7 @@ test "OSC: kitty color protocol" { const testing = std.testing; const Kind = kitty.color.Kind; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "21;foreground=?;background=rgb:f0/f8/ff;cursor=aliceblue;cursor_text;visual_bell=;selection_foreground=#xxxyyzz;selection_background=?;selection_background=#aabbcc;2=?;3=rgbi:1.0/1.0/1.0"; @@ -3330,7 +3362,7 @@ test "OSC: kitty color protocol" { test "OSC: kitty color protocol without allocator" { const testing = std.testing; - var p: Parser = .{}; + var p: Parser = .init(); defer p.deinit(); const input = "21;foreground=?"; @@ -3341,7 +3373,7 @@ test "OSC: kitty color protocol without allocator" { test "OSC: kitty color protocol double reset" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "21;foreground=?;background=rgb:f0/f8/ff;cursor=aliceblue;cursor_text;visual_bell=;selection_foreground=#xxxyyzz;selection_background=?;selection_background=#aabbcc;2=?;3=rgbi:1.0/1.0/1.0"; @@ -3357,7 +3389,7 @@ test "OSC: kitty color protocol double reset" { test "OSC: kitty color protocol reset after invalid" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "21;foreground=?;background=rgb:f0/f8/ff;cursor=aliceblue;cursor_text;visual_bell=;selection_foreground=#xxxyyzz;selection_background=?;selection_background=#aabbcc;2=?;3=rgbi:1.0/1.0/1.0"; @@ -3378,7 +3410,7 @@ test "OSC: kitty color protocol reset after invalid" { test "OSC: kitty color protocol no key" { const testing = std.testing; - var p: Parser = .{ .alloc = testing.allocator }; + var p: Parser = .initAlloc(testing.allocator); defer p.deinit(); const input = "21;"; diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index ec7296490..5af446ebb 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -47,8 +47,16 @@ pub fn Stream(comptime Handler: type) type { }; handler: Handler, - parser: Parser = .{}, - utf8decoder: UTF8Decoder = .{}, + parser: Parser, + utf8decoder: UTF8Decoder, + + pub fn init(h: Handler) Self { + return .{ + .handler = h, + .parser = .init(), + .utf8decoder = .{}, + }; + } pub fn deinit(self: *Self) void { self.parser.deinit(); @@ -1842,7 +1850,7 @@ test "stream: print" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.next('x'); try testing.expectEqual(@as(u21, 'x'), s.handler.c.?); } @@ -1856,7 +1864,7 @@ test "simd: print invalid utf-8" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice(&.{0xFF}); try testing.expectEqual(@as(u21, 0xFFFD), s.handler.c.?); } @@ -1870,7 +1878,7 @@ test "simd: complete incomplete utf-8" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice(&.{0xE0}); // 3 byte try testing.expect(s.handler.c == null); try s.nextSlice(&.{0xA0}); // still incomplete @@ -1888,7 +1896,7 @@ test "stream: cursor right (CUF)" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[C"); try testing.expectEqual(@as(u16, 1), s.handler.amount); @@ -1913,7 +1921,7 @@ test "stream: dec set mode (SM) and reset mode (RM)" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[?6h"); try testing.expectEqual(@as(modes.Mode, .origin), s.handler.mode); @@ -1935,7 +1943,7 @@ test "stream: ansi set mode (SM) and reset mode (RM)" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[4h"); try testing.expectEqual(@as(modes.Mode, .insert), s.handler.mode.?); @@ -1957,7 +1965,7 @@ test "stream: ansi set mode (SM) and reset mode (RM) with unknown value" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[6h"); try testing.expect(s.handler.mode == null); @@ -1977,7 +1985,7 @@ test "stream: restore mode" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); for ("\x1B[?42r") |c| try s.next(c); try testing.expect(!s.handler.called); } @@ -1992,7 +2000,7 @@ test "stream: pop kitty keyboard with no params defaults to 1" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); for ("\x1B[2s"); try testing.expect(s.handler.escape == null); @@ -2245,13 +2253,13 @@ test "stream: change window title with invalid utf-8" { }; { - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b]2;abc\x1b\\"); try testing.expect(s.handler.seen); } { - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b]2;abc\xc0\x1b\\"); try testing.expect(!s.handler.seen); } @@ -2268,7 +2276,7 @@ test "stream: insert characters" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); for ("\x1B[42@") |c| try s.next(c); try testing.expect(s.handler.called); @@ -2294,7 +2302,7 @@ test "stream: SCOSC" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); for ("\x1B[s") |c| try s.next(c); try testing.expect(s.handler.called); } @@ -2309,7 +2317,7 @@ test "stream: SCORC" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); for ("\x1B[u") |c| try s.next(c); try testing.expect(s.handler.called); } @@ -2323,7 +2331,7 @@ test "stream: too many csi params" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[1;1;1;1;1;1;1;1;1;1;1;1;1;1;1;1;1C"); } @@ -2335,7 +2343,7 @@ test "stream: csi param too long" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1B[1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111C"); } @@ -2348,7 +2356,7 @@ test "stream: send report with CSI t" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[14t"); try testing.expectEqual(csi.SizeReportStyle.csi_14_t, s.handler.style); @@ -2372,7 +2380,7 @@ test "stream: invalid CSI t" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[19t"); try testing.expectEqual(null, s.handler.style); @@ -2387,7 +2395,7 @@ test "stream: CSI t push title" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[22;0t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2405,7 +2413,7 @@ test "stream: CSI t push title with explicit window" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[22;2t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2423,7 +2431,7 @@ test "stream: CSI t push title with explicit icon" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[22;1t"); try testing.expectEqual(null, s.handler.op); @@ -2438,7 +2446,7 @@ test "stream: CSI t push title with index" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[22;0;5t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2456,7 +2464,7 @@ test "stream: CSI t pop title" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[23;0t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2474,7 +2482,7 @@ test "stream: CSI t pop title with explicit window" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[23;2t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2492,7 +2500,7 @@ test "stream: CSI t pop title with explicit icon" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[23;1t"); try testing.expectEqual(null, s.handler.op); @@ -2507,7 +2515,7 @@ test "stream: CSI t pop title with index" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[23;0;5t"); try testing.expectEqual(csi.TitlePushPop{ @@ -2525,7 +2533,7 @@ test "stream CSI W clear tab stops" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[2W"); try testing.expectEqual(csi.TabClear.current, s.handler.op.?); @@ -2543,7 +2551,7 @@ test "stream CSI W tab set" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[W"); try testing.expect(s.handler.called); @@ -2570,7 +2578,7 @@ test "stream CSI ? W reset tab stops" { } }; - var s: Stream(H) = .{ .handler = .{} }; + var s: Stream(H) = .init(.{}); try s.nextSlice("\x1b[?2W"); try testing.expect(!s.handler.reset); diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 4b5b93641..e41fe33a9 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -313,15 +313,12 @@ pub fn init(self: *Termio, alloc: Allocator, opts: termio.Options) !void { .size = opts.size, .backend = backend, .mailbox = opts.mailbox, - .terminal_stream = .{ - .handler = handler, - .parser = .{ - .osc_parser = .{ - // Populate the OSC parser allocator (optional) because - // we want to support large OSC payloads such as OSC 52. - .alloc = alloc, - }, - }, + .terminal_stream = stream: { + var s: terminalpkg.Stream(StreamHandler) = .init(handler); + // Populate the OSC parser allocator (optional) because + // we want to support large OSC payloads such as OSC 52. + s.parser.osc_parser.alloc = alloc; + break :stream s; }, .thread_enter_state = thread_enter_state, }; From 108260100c0400bac26b50e5bf44600ed61d0e66 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 9 Aug 2025 23:09:59 -0500 Subject: [PATCH 37/70] gtk-ng: add a helper to reduce boilerplate in GTK IPC --- src/apprt/gtk-ng/ipc/DBus.zig | 189 ++++++++++++++++++++++++++++ src/apprt/gtk-ng/ipc/new_window.zig | 170 +++++-------------------- 2 files changed, 220 insertions(+), 139 deletions(-) create mode 100644 src/apprt/gtk-ng/ipc/DBus.zig diff --git a/src/apprt/gtk-ng/ipc/DBus.zig b/src/apprt/gtk-ng/ipc/DBus.zig new file mode 100644 index 000000000..d14d86ce6 --- /dev/null +++ b/src/apprt/gtk-ng/ipc/DBus.zig @@ -0,0 +1,189 @@ +//! DBus helper for IPC +const Self = @This(); + +const std = @import("std"); +const Allocator = std.mem.Allocator; + +const gio = @import("gio"); +const glib = @import("glib"); + +const apprt = @import("../../../apprt.zig"); +const ApprtApp = @import("../App.zig"); + +/// The target for this IPC. +target: apprt.ipc.Target, + +/// Connection to the DBus session bus. +dbus: *gio.DBusConnection, + +/// The bus name of the Ghostty instance that we are calling. +bus_name: [:0]const u8, + +/// The object path of the Ghostty instance that we are calling. +object_path: [:0]const u8, + +/// Used to build the DBus payload. +payload_builder: *glib.VariantBuilder, + +/// Used to build the parameters for the IPC. +parameters_builder: *glib.VariantBuilder, + +/// Initialize the helper. +pub fn init(alloc: Allocator, target: apprt.ipc.Target, action: [:0]const u8) (Allocator.Error || std.posix.WriteError || apprt.ipc.Errors)!Self { + + // Get the appropriate bus name and object path for contacting the + // Ghostty instance we're interested in. + const bus_name: [:0]const u8, const object_path: [:0]const u8 = switch (target) { + .class => |class| result: { + // Force the usage of the class specified on the CLI to determine the + // bus name and object path. + const object_path = try std.fmt.allocPrintZ(alloc, "/{s}", .{class}); + + std.mem.replaceScalar(u8, object_path, '.', '/'); + std.mem.replaceScalar(u8, object_path, '-', '_'); + + break :result .{ class, object_path }; + }, + .detect => .{ ApprtApp.application_id, ApprtApp.object_path }, + }; + errdefer { + switch (target) { + .class => alloc.free(object_path), + .detect => {}, + } + } + + if (gio.Application.idIsValid(bus_name.ptr) == 0) { + const stderr = std.io.getStdErr().writer(); + try stderr.print("D-Bus bus name is not valid: {s}\n", .{bus_name}); + return error.IPCFailed; + } + + if (glib.Variant.isObjectPath(object_path.ptr) == 0) { + const stderr = std.io.getStdErr().writer(); + try stderr.print("D-Bus object path is not valid: {s}\n", .{object_path}); + return error.IPCFailed; + } + + // Get a connection to the DBus session bus. + const dbus = dbus: { + var err_: ?*glib.Error = null; + defer if (err_) |err| err.free(); + + const dbus_ = gio.busGetSync(.session, null, &err_); + if (err_) |err| { + const stderr = std.io.getStdErr().writer(); + try stderr.print( + "Unable to establish connection to D-Bus session bus: {s}\n", + .{err.f_message orelse "(unknown)"}, + ); + return error.IPCFailed; + } + + break :dbus dbus_ orelse { + const stderr = std.io.getStdErr().writer(); + try stderr.print("gio.busGetSync returned null\n", .{}); + return error.IPCFailed; + }; + }; + + // Set up the payload builder. + const payload_variant_type = glib.VariantType.new("(sava{sv})"); + defer glib.free(payload_variant_type); + + const payload_builder = glib.VariantBuilder.new(payload_variant_type); + + // Add the action name to the payload. + { + const s_variant_type = glib.VariantType.new("s"); + defer s_variant_type.free(); + + const bytes = glib.Bytes.new(action.ptr, action.len + 1); + defer bytes.unref(); + const value = glib.Variant.newFromBytes(s_variant_type, bytes, @intFromBool(true)); + + payload_builder.addValue(value); + } + + // Set up the parameter builder. + const parameters_variant_type = glib.VariantType.new("av"); + defer parameters_variant_type.free(); + + const parameters_builder = glib.VariantBuilder.new(parameters_variant_type); + + return .{ + .target = target, + .dbus = dbus, + .bus_name = bus_name, + .object_path = object_path, + .payload_builder = payload_builder, + .parameters_builder = parameters_builder, + }; +} + +/// Add a parameter to the IPC call. +pub fn addParameter(self: *Self, variant: *glib.Variant) void { + self.parameters_builder.add("v", variant); +} + +/// Send the IPC to the remote Ghostty. Once it completes, nothing further +/// should be done with this object other than call `deinit`. +pub fn send(self: *Self) (std.posix.WriteError || apprt.ipc.Errors)!void { + // finish building the parameters + const parameters = self.parameters_builder.end(); + + // Add the parameters to the payload. + self.payload_builder.addValue(parameters); + + // Add the platform data to the payload. + { + const platform_data_variant_type = glib.VariantType.new("a{sv}"); + defer platform_data_variant_type.free(); + + self.payload_builder.open(platform_data_variant_type); + defer self.payload_builder.close(); + + // We have no platform data. + } + + const payload = self.payload_builder.end(); + + { + var err_: ?*glib.Error = null; + defer if (err_) |err| err.free(); + + const result_ = self.dbus.callSync( + self.bus_name, + self.object_path, + "org.gtk.Actions", + "Activate", + payload, + null, // We don't care about the return type, we don't do anything with it. + .{}, // no flags + -1, // default timeout + null, // not cancellable + &err_, + ); + defer if (result_) |result| result.unref(); + + if (err_) |err| { + const stderr = std.io.getStdErr().writer(); + try stderr.print( + "D-Bus method call returned an error err={s}\n", + .{err.f_message orelse "(unknown)"}, + ); + return error.IPCFailed; + } + } +} + +/// Free/unref any data held by this instance. +pub fn deinit(self: *Self, alloc: Allocator) void { + switch (self.target) { + .class => alloc.free(self.object_path), + .detect => {}, + } + self.parameters_builder.unref(); + self.payload_builder.unref(); + self.dbus.unref(); +} diff --git a/src/apprt/gtk-ng/ipc/new_window.zig b/src/apprt/gtk-ng/ipc/new_window.zig index f67498ae1..55e2e0e01 100644 --- a/src/apprt/gtk-ng/ipc/new_window.zig +++ b/src/apprt/gtk-ng/ipc/new_window.zig @@ -1,11 +1,10 @@ const std = @import("std"); const Allocator = std.mem.Allocator; -const gio = @import("gio"); const glib = @import("glib"); const apprt = @import("../../../apprt.zig"); -const ApprtApp = @import("../App.zig"); +const DBus = @import("DBus.zig"); // Use a D-Bus method call to open a new window on GTK. // See: https://wiki.gnome.org/Projects/GLib/GApplication/DBusAPI @@ -22,149 +21,42 @@ const ApprtApp = @import("../App.zig"); // gdbus call --session --dest com.mitchellh.ghostty --object-path /com/mitchellh/ghostty --method org.gtk.Actions.Activate new-window-command '[<@as ["echo" "hello"]>]' [] // ``` pub fn newWindow(alloc: Allocator, target: apprt.ipc.Target, value: apprt.ipc.Action.NewWindow) (Allocator.Error || std.posix.WriteError || apprt.ipc.Errors)!bool { - const stderr = std.io.getStdErr().writer(); + var dbus = try DBus.init( + alloc, + target, + if (value.arguments == null) + "new-window" + else + "new-window-command", + ); + defer dbus.deinit(alloc); - // Get the appropriate bus name and object path for contacting the - // Ghostty instance we're interested in. - const bus_name: [:0]const u8, const object_path: [:0]const u8 = switch (target) { - .class => |class| result: { - // Force the usage of the class specified on the CLI to determine the - // bus name and object path. - const object_path = try std.fmt.allocPrintZ(alloc, "/{s}", .{class}); + if (value.arguments) |arguments| { + // If `-e` was specified on the command line, the first + // parameter is an array of strings that contain the arguments + // that came after `-e`, which will be interpreted as a command + // to run. + const as_variant_type = glib.VariantType.new("as"); + defer as_variant_type.free(); - std.mem.replaceScalar(u8, object_path, '.', '/'); - std.mem.replaceScalar(u8, object_path, '-', '_'); + const s_variant_type = glib.VariantType.new("s"); + defer s_variant_type.free(); - break :result .{ class, object_path }; - }, - .detect => .{ ApprtApp.application_id, ApprtApp.object_path }, - }; - defer { - switch (target) { - .class => alloc.free(object_path), - .detect => {}, + var command: glib.VariantBuilder = undefined; + command.init(as_variant_type); + errdefer command.clear(); + + for (arguments) |argument| { + const bytes = glib.Bytes.new(argument.ptr, argument.len + 1); + defer bytes.unref(); + const string = glib.Variant.newFromBytes(s_variant_type, bytes, @intFromBool(true)); + command.addValue(string); } + + dbus.addParameter(command.end()); } - if (gio.Application.idIsValid(bus_name.ptr) == 0) { - try stderr.print("D-Bus bus name is not valid: {s}\n", .{bus_name}); - return error.IPCFailed; - } - - if (glib.Variant.isObjectPath(object_path.ptr) == 0) { - try stderr.print("D-Bus object path is not valid: {s}\n", .{object_path}); - return error.IPCFailed; - } - - const dbus = dbus: { - var err_: ?*glib.Error = null; - defer if (err_) |err| err.free(); - - const dbus_ = gio.busGetSync(.session, null, &err_); - if (err_) |err| { - try stderr.print( - "Unable to establish connection to D-Bus session bus: {s}\n", - .{err.f_message orelse "(unknown)"}, - ); - return error.IPCFailed; - } - - break :dbus dbus_ orelse { - try stderr.print("gio.busGetSync returned null\n", .{}); - return error.IPCFailed; - }; - }; - defer dbus.unref(); - - // use a builder to create the D-Bus method call payload - const payload = payload: { - const payload_variant_type = glib.VariantType.new("(sava{sv})"); - defer glib.free(payload_variant_type); - - // Initialize our builder to build up our parameters - var builder: glib.VariantBuilder = undefined; - builder.init(payload_variant_type); - errdefer builder.clear(); - - // action - if (value.arguments == null) { - builder.add("s", "new-window"); - } else { - builder.add("s", "new-window-command"); - } - - // parameters - { - const av_variant_type = glib.VariantType.new("av"); - defer av_variant_type.free(); - - var parameters: glib.VariantBuilder = undefined; - parameters.init(av_variant_type); - errdefer parameters.clear(); - - if (value.arguments) |arguments| { - // If `-e` was specified on the command line, the first - // parameter is an array of strings that contain the arguments - // that came after `-e`, which will be interpreted as a command - // to run. - { - const as = glib.VariantType.new("as"); - defer as.free(); - - var command: glib.VariantBuilder = undefined; - command.init(as); - errdefer command.clear(); - - for (arguments) |argument| { - command.add("s", argument.ptr); - } - - parameters.add("v", command.end()); - } - } - - builder.addValue(parameters.end()); - } - - { - const platform_data_variant_type = glib.VariantType.new("a{sv}"); - defer platform_data_variant_type.free(); - - builder.open(platform_data_variant_type); - defer builder.close(); - - // we have no platform data - } - - break :payload builder.end(); - }; - - { - var err_: ?*glib.Error = null; - defer if (err_) |err| err.free(); - - const result_ = dbus.callSync( - bus_name, - object_path, - "org.gtk.Actions", - "Activate", - payload, - null, // We don't care about the return type, we don't do anything with it. - .{}, // no flags - -1, // default timeout - null, // not cancellable - &err_, - ); - defer if (result_) |result| result.unref(); - - if (err_) |err| { - try stderr.print( - "D-Bus method call returned an error err={s}\n", - .{err.f_message orelse "(unknown)"}, - ); - return error.IPCFailed; - } - } + try dbus.send(); return true; } From 42f0c05d7e4b99f3d101c4d834c83f679e6d1398 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 13:04:42 -0700 Subject: [PATCH 38/70] terminal: fix undefined memory access in OSC parser Fixes #8007 Verified with `test-valgrind -Dtest-filter="OSC"` which had cond access errors before, and none after this. Basically a copy of #8008. --- src/terminal/osc.zig | 31 +++++++++++++------------------ src/terminal/stream.zig | 6 ++++++ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index 00ede49ce..6090166da 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -16,6 +16,10 @@ const kitty = @import("kitty.zig"); const log = std.log.scoped(.osc); pub const Command = union(enum) { + /// This generally shouldn't ever be set except as an initial zero value. + /// Ignore it. + invalid, + /// Set the window title of the terminal /// /// If title mode 0 is set text is expect to be hex encoded (i.e. utf-8 @@ -433,6 +437,7 @@ pub const Parser = struct { var result: Parser = .{ .alloc = null, .state = .empty, + .command = .invalid, .buf_start = 0, .buf_idx = 0, .buf_dynamic = null, @@ -440,14 +445,12 @@ pub const Parser = struct { // Keeping all our undefined values together so we can // visually easily duplicate them in the Valgrind check below. - .command = undefined, .buf = undefined, .temp_state = undefined, }; if (std.valgrind.runningOnValgrind() > 0) { // Initialize our undefined fields so Valgrind can catch it. // https://github.com/ziglang/zig/issues/19148 - result.command = undefined; result.buf = undefined; result.temp_state = undefined; } @@ -478,9 +481,17 @@ pub const Parser = struct { return; } + // Some commands have their own memory management we need to clear. + switch (self.command) { + .kitty_color_protocol => |*v| v.list.deinit(), + .color_operation => |*v| v.operations.deinit(self.alloc.?), + else => {}, + } + self.state = .empty; self.buf_start = 0; self.buf_idx = 0; + self.command = .invalid; self.complete = false; if (self.buf_dynamic) |ptr| { const alloc = self.alloc.?; @@ -488,22 +499,6 @@ pub const Parser = struct { alloc.destroy(ptr); self.buf_dynamic = null; } - - // Some commands have their own memory management we need to clear. - // After cleaning up these command, we reset the command to - // some nonsense (but valid) command so we don't double free. - const default: Command = .{ .hyperlink_end = {} }; - switch (self.command) { - .kitty_color_protocol => |*v| { - v.list.deinit(); - self.command = default; - }, - .color_operation => |*v| { - v.operations.deinit(self.alloc.?); - self.command = default; - }, - else => {}, - } } /// Consume the next character c and advance the parser state. diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index 5af446ebb..f40fc4c94 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -1608,6 +1608,12 @@ pub fn Stream(comptime Handler: type) type { .sleep, .show_message_box, .change_conemu_tab_title, .wait_input => { log.warn("unimplemented OSC callback: {}", .{cmd}); }, + + .invalid => { + // This is an invalid internal state, not an invalid OSC + // string being parsed. We shouldn't see this. + log.warn("invalid OSC, should never happen", .{}); + }, } // Fall through for when we don't have a handler. From 5287b963c9d5b300411f0b103e012068dddccc26 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 13:19:21 -0700 Subject: [PATCH 39/70] ci: run valgrind in CI This runs Valgrind on our unit test suite in CI. Since we're not currently passing Valgrind, this will be incrementally updated with the filters for our passing tests. Ultimately, we'll remove the filters and run the full suite. Valgrind is slow and hungry so this is our first and only job currently on a large instance. --- .github/workflows/test.yml | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 44e1e30fc..61d0cde42 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,6 +36,7 @@ jobs: - blueprint-compiler - test-pkg-linux - test-debian-13 + - valgrind - zig-fmt steps: - id: status @@ -1038,3 +1039,42 @@ jobs: # cache-key: flatpak-builder-${{ github.sha }} # arch: ${{ matrix.variant.arch }} # verbose: true + + valgrind: + if: github.repository == 'ghostty-org/ghostty' + runs-on: namespace-profile-ghostty-lg + needs: test + env: + ZIG_LOCAL_CACHE_DIR: /zig/local-cache + ZIG_GLOBAL_CACHE_DIR: /zig/global-cache + steps: + - name: Checkout code + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + + - name: Setup Cache + uses: namespacelabs/nscloud-cache-action@305bfa7ea980a858d511af4899414a84847c7991 # v1.2.16 + with: + path: | + /nix + /zig + + # Install Nix and use that to run our tests so our environment matches exactly. + - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + with: + nix_path: nixpkgs=channel:nixos-unstable + - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 + with: + name: ghostty + authToken: "${{ secrets.CACHIX_AUTH_TOKEN }}" + + - name: valgrind deps + run: | + sudo apt update -y + sudo apt install -y valgrind libc6-dbg + + # Currently, the entire Ghostty test suite does not pass Valgrind. + # As we incrementally add areas that pass, we'll add more filters here. + # Ultimately, we'll remove the filter and run the full suite. + - name: valgrind + run: | + nix develop -c zig build test-valgrind -Dtest-filter="OSC" From 4fa7b412d400d73c53ef2e3edbc65ce8e2c32154 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 14:05:09 -0700 Subject: [PATCH 40/70] terminal: disable integrity checks under Valgrind --- .github/workflows/test.yml | 1 + src/terminal/Screen.zig | 5 +++++ src/terminal/page.zig | 5 +++++ src/terminal/search.zig | 5 +++++ 4 files changed, 16 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 61d0cde42..021fa3b32 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1078,3 +1078,4 @@ jobs: - name: valgrind run: | nix develop -c zig build test-valgrind -Dtest-filter="OSC" + nix develop -c zig build test-valgrind -Dtest-filter="Parser" diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 079df37db..67769923f 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -233,6 +233,11 @@ pub fn deinit(self: *Screen) void { /// ensure they're also calling page integrity checks if necessary. pub fn assertIntegrity(self: *const Screen) void { if (build_config.slow_runtime_safety) { + // We don't run integrity checks on Valgrind because its soooooo slow, + // Valgrind is our integrity checker, and we run these during unit + // tests (non-Valgrind) anyways so we're verifying anyways. + if (std.valgrind.runningOnValgrind() > 0) return; + assert(self.cursor.x < self.pages.cols); assert(self.cursor.y < self.pages.rows); diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 2a631ac78..292707263 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -346,6 +346,11 @@ pub const Page = struct { // used for the same reason as styles above. // + // We don't run integrity checks on Valgrind because its soooooo slow, + // Valgrind is our integrity checker, and we run these during unit + // tests (non-Valgrind) anyways so we're verifying anyways. + if (std.valgrind.runningOnValgrind() > 0) return; + if (build_config.slow_runtime_safety) { if (self.pause_integrity_checks > 0) return; } diff --git a/src/terminal/search.zig b/src/terminal/search.zig index 2f87f894b..b3c6494a3 100644 --- a/src/terminal/search.zig +++ b/src/terminal/search.zig @@ -454,6 +454,11 @@ const SlidingWindow = struct { fn assertIntegrity(self: *const SlidingWindow) void { if (comptime !std.debug.runtime_safety) return; + // We don't run integrity checks on Valgrind because its soooooo slow, + // Valgrind is our integrity checker, and we run these during unit + // tests (non-Valgrind) anyways so we're verifying anyways. + if (std.valgrind.runningOnValgrind() > 0) return; + // Integrity check: verify our data matches our metadata exactly. var meta_it = self.meta.iterator(.forward); var data_len: usize = 0; From 610ce94f2d5308392ea0fbf88a5a7c87b234040a Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 20 Aug 2025 15:26:16 -0600 Subject: [PATCH 41/70] font/CoreText: fix positioning for padded scaled glyphs When constraints increased or decreased the size significantly, the fractional position was getting messed up by the scale. This change separates that out so that it applies correctly. --- src/font/face/coretext.zig | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/font/face/coretext.zig b/src/font/face/coretext.zig index 1b1c559fb..cb6e6b1f7 100644 --- a/src/font/face/coretext.zig +++ b/src/font/face/coretext.zig @@ -408,18 +408,15 @@ pub const Face = struct { const px_x: i32 = @intFromFloat(@floor(x)); const px_y: i32 = @intFromFloat(@floor(y)); - // We offset our glyph by its bearings when we draw it, so that it's - // rendered fully inside our canvas area, but we make sure to keep the - // fractional pixel offset so that we rasterize with the appropriate - // sub-pixel position. + // We keep track of the fractional part of the pixel bearings, which + // we will add as an offset when rasterizing to make sure we get the + // correct sub-pixel position. const frac_x = x - @floor(x); const frac_y = y - @floor(y); - const draw_x = -rect.origin.x + frac_x; - const draw_y = -rect.origin.y + frac_y; // Add the fractional pixel to the width and height and take // the ceiling to get a canvas size that will definitely fit - // our drawn glyph. + // our drawn glyph, including the fractional offset. const px_width: u32 = @intFromFloat(@ceil(width + frac_x)); const px_height: u32 = @intFromFloat(@ceil(height + frac_y)); @@ -525,6 +522,17 @@ pub const Face = struct { context.setLineWidth(ctx, line_width); } + // Translate our drawing context so that when we draw our + // glyph the bottom/left edge is at the correct sub-pixel + // position. The bottom/left edges are guaranteed to be at + // exactly [0, 0] relative to this because when we call to + // `drawGlyphs`, we pass the negated bearings. + context.translateCTM( + ctx, + frac_x, + frac_y, + ); + // Scale the drawing context so that when we draw // our glyph it's stretched to the constrained size. context.scaleCTM( @@ -534,7 +542,15 @@ pub const Face = struct { ); // Draw our glyph. - self.font.drawGlyphs(&glyphs, &.{.{ .x = draw_x, .y = draw_y }}, ctx); + // + // We offset the position by the negated bearings so that the + // glyph is drawn at exactly [0, 0], which is then offset to + // the appropriate fractional position by the translation we + // did before scaling. + self.font.drawGlyphs(&glyphs, &.{.{ + .x = -rect.origin.x, + .y = -rect.origin.y, + }}, ctx); // Write our rasterized glyph to the atlas. const region = try atlas.reserve(alloc, px_width, px_height); From 1f7f6787457bec48621621719f6a75336ac856bc Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Wed, 20 Aug 2025 17:06:20 -0500 Subject: [PATCH 42/70] flatpak: manually install Zig 0.14.1 The SDK published on Flathub updated to Zig 0.15.1 which broke the Flathub build in CI. So let's install it ourselves so that we can control the version. --- .github/workflows/test.yml | 51 ++++++++++++------------- flatpak/com.mitchellh.ghostty-debug.yml | 4 +- flatpak/com.mitchellh.ghostty.yml | 4 +- flatpak/dependencies.yml | 18 +++++++++ 4 files changed, 45 insertions(+), 32 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 61d0cde42..27f63da34 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1013,32 +1013,31 @@ jobs: - name: Check Flatpak Zig Dependencies run: nix develop -c ./flatpak/build-support/check-zig-cache.sh - # Disabled until we update to Zig 0.15 or if we can pin this to Zig 0.14 - # flatpak: - # if: github.repository == 'ghostty-org/ghostty' - # name: "Flatpak" - # container: - # image: ghcr.io/flathub-infra/flatpak-github-actions:gnome-47 - # options: --privileged - # strategy: - # fail-fast: false - # matrix: - # variant: - # - arch: x86_64 - # runner: namespace-profile-ghostty-md - # - arch: aarch64 - # runner: namespace-profile-ghostty-md-arm64 - # runs-on: ${{ matrix.variant.runner }} - # needs: [flatpak-check-zig-cache, test] - # steps: - # - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - # - uses: flatpak/flatpak-github-actions/flatpak-builder@10a3c29f0162516f0f68006be14c92f34bd4fa6c # v6.5 - # with: - # bundle: com.mitchellh.ghostty - # manifest-path: flatpak/com.mitchellh.ghostty.yml - # cache-key: flatpak-builder-${{ github.sha }} - # arch: ${{ matrix.variant.arch }} - # verbose: true + flatpak: + if: github.repository == 'ghostty-org/ghostty' + name: "Flatpak" + container: + image: ghcr.io/flathub-infra/flatpak-github-actions:gnome-47 + options: --privileged + strategy: + fail-fast: false + matrix: + variant: + - arch: x86_64 + runner: namespace-profile-ghostty-md + - arch: aarch64 + runner: namespace-profile-ghostty-md-arm64 + runs-on: ${{ matrix.variant.runner }} + needs: [flatpak-check-zig-cache, test] + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: flatpak/flatpak-github-actions/flatpak-builder@10a3c29f0162516f0f68006be14c92f34bd4fa6c # v6.5 + with: + bundle: com.mitchellh.ghostty + manifest-path: flatpak/com.mitchellh.ghostty.yml + cache-key: flatpak-builder-${{ github.sha }} + arch: ${{ matrix.variant.arch }} + verbose: true valgrind: if: github.repository == 'ghostty-org/ghostty' diff --git a/flatpak/com.mitchellh.ghostty-debug.yml b/flatpak/com.mitchellh.ghostty-debug.yml index fe4722ef5..51c41931b 100644 --- a/flatpak/com.mitchellh.ghostty-debug.yml +++ b/flatpak/com.mitchellh.ghostty-debug.yml @@ -2,8 +2,6 @@ app-id: com.mitchellh.ghostty-debug runtime: org.gnome.Platform runtime-version: "48" sdk: org.gnome.Sdk -sdk-extensions: - - org.freedesktop.Sdk.Extension.ziglang default-branch: tip command: ghostty rename-icon: com.mitchellh.ghostty @@ -37,7 +35,7 @@ modules: - name: ghostty buildsystem: simple build-options: - append-path: /usr/lib/sdk/ziglang + append-path: /app/zig build-commands: - zig build -Doptimize=Debug diff --git a/flatpak/com.mitchellh.ghostty.yml b/flatpak/com.mitchellh.ghostty.yml index 1b119c11b..f5af4235d 100644 --- a/flatpak/com.mitchellh.ghostty.yml +++ b/flatpak/com.mitchellh.ghostty.yml @@ -2,8 +2,6 @@ app-id: com.mitchellh.ghostty runtime: org.gnome.Platform runtime-version: "48" sdk: org.gnome.Sdk -sdk-extensions: - - org.freedesktop.Sdk.Extension.ziglang default-branch: tip command: ghostty finish-args: @@ -36,7 +34,7 @@ modules: - name: ghostty buildsystem: simple build-options: - append-path: /usr/lib/sdk/ziglang + append-path: /app/zig build-commands: - zig build -Doptimize=ReleaseFast diff --git a/flatpak/dependencies.yml b/flatpak/dependencies.yml index efb5851e9..0ff0784c2 100644 --- a/flatpak/dependencies.yml +++ b/flatpak/dependencies.yml @@ -3,6 +3,24 @@ buildsystem: simple build-commands: - true modules: + - name: zig + buildsystem: simple + cleanup: + - "*" + build-commands: + - mkdir -p /app/zig + - cp -r ./* /app/zig + - chmod a+x /app/zig/zig + sources: + - type: archive + sha256: 24aeeec8af16c381934a6cd7d95c807a8cb2cf7df9fa40d359aa884195c4716c + url: https://ziglang.org/download/0.14.1/zig-x86_64-linux-0.14.1.tar.xz + only-arches: [x86_64] + - type: archive + sha256: f7a654acc967864f7a050ddacfaa778c7504a0eca8d2b678839c21eea47c992b + url: https://ziglang.org/download/0.14.1/zig-aarch64-linux-0.14.1.tar.xz + only-arches: [aarch64] + - name: bzip2-redirect buildsystem: simple build-commands: From 2cebc225c038e0a2245f840244b9b8063148ad10 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 19:46:36 -0700 Subject: [PATCH 43/70] ci: failing pagelist tests on purpose, so we can verify CI fails --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 021fa3b32..ea4448bf4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1079,3 +1079,4 @@ jobs: run: | nix develop -c zig build test-valgrind -Dtest-filter="OSC" nix develop -c zig build test-valgrind -Dtest-filter="Parser" + nix develop -c zig build test-valgrind -Dtest-filter="PageList" From 3ce043123b5c651ed71bbbdb935feb7e5d30aa4e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 19:51:36 -0700 Subject: [PATCH 44/70] terminal: fix undefined memory access in PageList eraseRows --- src/terminal/PageList.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 3b329d7cf..a4136d7f3 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2395,8 +2395,8 @@ pub fn eraseRows( break; } - self.erasePage(chunk.node); erased += chunk.node.data.size.rows; + self.erasePage(chunk.node); continue; } From be51f3e729fce6017caa624b23e7313b8c53a052 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 20:15:47 -0700 Subject: [PATCH 45/70] terminal: fix uninitialized memory in Cell init --- src/terminal/page.zig | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 292707263..5bc724c4c 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -2043,10 +2043,13 @@ pub const Cell = packed struct(u64) { /// Helper to make a cell that just has a codepoint. pub fn init(cp: u21) Cell { - return .{ - .content_tag = .codepoint, - .content = .{ .codepoint = cp }, - }; + // We have to use this bitCast here to ensure that our memory is + // zeroed. Otherwise, the content below will leave some uninitialized + // memory in the packed union. Valgrind verifies this. + var cell: Cell = @bitCast(@as(u64, 0)); + cell.content_tag = .codepoint; + cell.content = .{ .codepoint = cp }; + return cell; } pub fn isZero(self: Cell) bool { From 566062c0a57832026807bbf04a8eecb478278cc5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 20:44:32 -0700 Subject: [PATCH 46/70] terminal: fix undefined memory in Tabstops code --- src/font/Collection.zig | 3 +++ src/terminal/Tabstops.zig | 1 + src/terminal/page.zig | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/src/font/Collection.zig b/src/font/Collection.zig index ef508b346..2b5f591a5 100644 --- a/src/font/Collection.zig +++ b/src/font/Collection.zig @@ -937,6 +937,9 @@ test init { } test "add full" { + // This test is way too slow to run under Valgrind, unfortunately. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + const testing = std.testing; const alloc = testing.allocator; const testFont = font.embedded.regular; diff --git a/src/terminal/Tabstops.zig b/src/terminal/Tabstops.zig index 4ab5133d9..c352cb351 100644 --- a/src/terminal/Tabstops.zig +++ b/src/terminal/Tabstops.zig @@ -129,6 +129,7 @@ pub fn resize(self: *Tabstops, alloc: Allocator, cols: usize) !void { // Note: we can probably try to realloc here but I'm not sure it matters. const new = try alloc.alloc(Unit, size); + @memset(new, 0); if (self.dynamic_stops.len > 0) { fastmem.copy(Unit, new, self.dynamic_stops); alloc.free(self.dynamic_stops); diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 5bc724c4c..d870bd160 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -3042,6 +3042,10 @@ test "Page moveCells graphemes" { } test "Page verifyIntegrity graphemes good" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, @@ -3063,6 +3067,10 @@ test "Page verifyIntegrity graphemes good" { } test "Page verifyIntegrity grapheme row not marked" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, @@ -3090,6 +3098,10 @@ test "Page verifyIntegrity grapheme row not marked" { } test "Page verifyIntegrity styles good" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, @@ -3122,6 +3134,10 @@ test "Page verifyIntegrity styles good" { } test "Page verifyIntegrity styles ref count mismatch" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, @@ -3160,6 +3176,10 @@ test "Page verifyIntegrity styles ref count mismatch" { } test "Page verifyIntegrity zero rows" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, @@ -3174,6 +3194,10 @@ test "Page verifyIntegrity zero rows" { } test "Page verifyIntegrity zero cols" { + // Too slow, and not really necessary because the integrity tests are + // only run in debug builds and unit tests verify they work well enough. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + var page = try Page.init(.{ .cols = 10, .rows = 10, From a57afd41ac2931214c7a2fc17f04cedc8e59e694 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2025 20:54:26 -0700 Subject: [PATCH 47/70] terminal: fix undefined memory access in unit test --- src/terminal/Terminal.zig | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index dd7207f6d..52e3d0fca 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -2824,14 +2824,21 @@ test "Terminal: input glitch text" { var t = try init(alloc, .{ .cols = 30, .rows = 30 }); defer t.deinit(alloc); - const page = t.screen.pages.pages.first.?; - const grapheme_cap = page.data.capacity.grapheme_bytes; + // Get our initial grapheme capacity. + const grapheme_cap = cap: { + const page = t.screen.pages.pages.first.?; + break :cap page.data.capacity.grapheme_bytes; + }; - while (page.data.capacity.grapheme_bytes == grapheme_cap) { + // Print glitch text until our capacity changes + while (true) { + const page = t.screen.pages.pages.first.?; + if (page.data.capacity.grapheme_bytes != grapheme_cap) break; try t.printString(glitch); } // We're testing to make sure that grapheme capacity gets increased. + const page = t.screen.pages.pages.first.?; try testing.expect(page.data.capacity.grapheme_bytes > grapheme_cap); } From 6427a216794cc368803b2fddb4bd2114102225b8 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 21 Aug 2025 00:42:18 -0500 Subject: [PATCH 48/70] flatpack: add back to list of required CI jobs --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27f63da34..f2b66ed38 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,6 +38,7 @@ jobs: - test-debian-13 - valgrind - zig-fmt + - flatpak steps: - id: status name: Determine status From fe5eafac0a0f50e947f02c3ef5f20845df4a30d4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 06:53:57 -0700 Subject: [PATCH 49/70] font: fix fontconfig leaks in unit tests --- src/font/DeferredFace.zig | 1 + src/font/discovery.zig | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/font/DeferredFace.zig b/src/font/DeferredFace.zig index f9ce0bff5..290a01d74 100644 --- a/src/font/DeferredFace.zig +++ b/src/font/DeferredFace.zig @@ -413,6 +413,7 @@ test "fontconfig" { // Get a deferred face from fontconfig var def = def: { var fc = discovery.Fontconfig.init(); + defer fc.deinit(); var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 }); defer it.deinit(); break :def (try it.next()).?; diff --git a/src/font/discovery.zig b/src/font/discovery.zig index 6f51379b4..390465916 100644 --- a/src/font/discovery.zig +++ b/src/font/discovery.zig @@ -897,6 +897,7 @@ test "fontconfig" { const alloc = testing.allocator; var fc = Fontconfig.init(); + defer fc.deinit(); var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 }); defer it.deinit(); } @@ -908,12 +909,14 @@ test "fontconfig codepoint" { const alloc = testing.allocator; var fc = Fontconfig.init(); + defer fc.deinit(); var it = try fc.discover(alloc, .{ .codepoint = 'A', .size = 12 }); defer it.deinit(); // The first result should have the codepoint. Later ones may not // because fontconfig returns all fonts sorted. - const face = (try it.next()).?; + var face = (try it.next()).?; + defer face.deinit(); try testing.expect(face.hasCodepoint('A', null)); // Should have other codepoints too From 96a0b9021c1c8115f427d546b7ceb5abba15e4c6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 06:57:23 -0700 Subject: [PATCH 50/70] font: disable sprite test in valgrind --- src/font/sprite/Face.zig | 3 +++ valgrind.supp | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/font/sprite/Face.zig b/src/font/sprite/Face.zig index cb335dff6..5442890bf 100644 --- a/src/font/sprite/Face.zig +++ b/src/font/sprite/Face.zig @@ -511,6 +511,9 @@ fn testDrawRanges( } test "sprite face render all sprites" { + // This test is way too slow to run under Valgrind, unfortunately. + if (std.valgrind.runningOnValgrind() > 0) return error.SkipZigTest; + // Renders all sprites to an atlas and compares // it to a ground truth for regression testing. diff --git a/valgrind.supp b/valgrind.supp index 162f3393a..bfc78bcff 100644 --- a/valgrind.supp +++ b/valgrind.supp @@ -574,6 +574,18 @@ ... } +{ + pango language + Memcheck:Leak + match-leak-kinds: possible + fun:calloc + fun:g_malloc0 + fun:pango_language_from_string + fun:pango_language_get_default + fun:gtk_get_locale_direction + fun:gtk_init_check + ... +} { Adwaita Stylesheet Load Memcheck:Leak From 793e817d741b528cb8a3a745ba165a21ed745cc6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 06:58:06 -0700 Subject: [PATCH 51/70] ci: run all valgrind tests --- .github/workflows/test.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ea4448bf4..c56182798 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1072,11 +1072,6 @@ jobs: sudo apt update -y sudo apt install -y valgrind libc6-dbg - # Currently, the entire Ghostty test suite does not pass Valgrind. - # As we incrementally add areas that pass, we'll add more filters here. - # Ultimately, we'll remove the filter and run the full suite. - name: valgrind run: | - nix develop -c zig build test-valgrind -Dtest-filter="OSC" - nix develop -c zig build test-valgrind -Dtest-filter="Parser" - nix develop -c zig build test-valgrind -Dtest-filter="PageList" + nix develop -c zig build test-valgrind From d66747407db75601cce279df11c3fe8fdc08eae6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 25 Jul 2025 07:07:00 -0700 Subject: [PATCH 52/70] ci: move to bleedging edge sequoia builds This will bring in Xcode 26 Beta 4 which I believe fixes all the known issue we were dealing with keeping us on beta 1. --- .github/workflows/release-pr.yml | 6 ++++-- .github/workflows/release-tag.yml | 7 +++++-- .github/workflows/release-tip.yml | 6 +++--- .github/workflows/test.yml | 15 ++++++++++++--- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index 6fa813c31..cb1e8c1ef 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -47,7 +47,7 @@ jobs: sentry-cli dif upload --project ghostty --wait dsym.zip build-macos: - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 steps: - name: Checkout code @@ -95,6 +95,7 @@ jobs: run: | cd macos sudo xcode-select -s /Applications/Xcode_26.0.app + xcodebuild -version xcodebuild -target Ghostty -configuration Release # We inject the "build number" as simply the number of commits since HEAD. @@ -201,7 +202,7 @@ jobs: destination-dir: ./ build-macos-debug: - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 steps: - name: Checkout code @@ -249,6 +250,7 @@ jobs: run: | cd macos sudo xcode-select -s /Applications/Xcode_26.0.app + xcodebuild -version xcodebuild -target Ghostty -configuration Release # We inject the "build number" as simply the number of commits since HEAD. diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 9c92d45a9..bbd013f4e 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -120,7 +120,7 @@ jobs: build-macos: needs: [setup] - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 env: GHOSTTY_VERSION: ${{ needs.setup.outputs.version }} @@ -141,6 +141,9 @@ jobs: - name: XCode Select run: sudo xcode-select -s /Applications/Xcode_16.4.app + - name: Xcode Version + run: xcodebuild -version + - name: Setup Sparkle env: SPARKLE_VERSION: 2.6.4 @@ -291,7 +294,7 @@ jobs: appcast: needs: [setup, build-macos] - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge env: GHOSTTY_VERSION: ${{ needs.setup.outputs.version }} GHOSTTY_BUILD: ${{ needs.setup.outputs.build }} diff --git a/.github/workflows/release-tip.yml b/.github/workflows/release-tip.yml index 58e114f1b..791482518 100644 --- a/.github/workflows/release-tip.yml +++ b/.github/workflows/release-tip.yml @@ -154,7 +154,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 steps: - name: Checkout code @@ -374,7 +374,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 steps: - name: Checkout code @@ -554,7 +554,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge timeout-minutes: 90 steps: - name: Checkout code diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b7c63b774..8a8ef2e6c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -273,7 +273,7 @@ jobs: ghostty-source.tar.gz build-macos: - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge needs: test steps: - name: Checkout code @@ -334,6 +334,9 @@ jobs: - name: Xcode Select run: sudo xcode-select -s /Applications/Xcode_26.0.app + - name: Xcode Version + run: xcodebuild -version + - name: get the Zig deps id: deps run: nix build -L .#deps && echo "deps=$(readlink ./result)" >> $GITHUB_OUTPUT @@ -356,7 +359,7 @@ jobs: xcodebuild -target Ghostty-iOS "CODE_SIGNING_ALLOWED=NO" build-macos-matrix: - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge needs: test steps: - name: Checkout code @@ -374,6 +377,9 @@ jobs: - name: Xcode Select run: sudo xcode-select -s /Applications/Xcode_26.0.app + - name: Xcode Version + run: xcodebuild -version + - name: get the Zig deps id: deps run: nix build -L .#deps && echo "deps=$(readlink ./result)" >> $GITHUB_OUTPUT @@ -672,7 +678,7 @@ jobs: nix develop -c zig build -Dsentry=${{ matrix.sentry }} test-macos: - runs-on: namespace-profile-ghostty-macos-sequoia + runs-on: namespace-profile-ghostty-macos-sequoia-edge needs: test steps: - name: Checkout code @@ -690,6 +696,9 @@ jobs: - name: Xcode Select run: sudo xcode-select -s /Applications/Xcode_26.0.app + - name: Xcode Version + run: xcodebuild -version + - name: get the Zig deps id: deps run: nix build -L .#deps && echo "deps=$(readlink ./result)" >> $GITHUB_OUTPUT From 7a42c82d1866aa4489c6488b79c13844e8603ef8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 18 Aug 2025 13:05:55 -0700 Subject: [PATCH 53/70] temp: try downloading metal explicitly --- .github/workflows/test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8a8ef2e6c..7c5228dbf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -294,6 +294,10 @@ jobs: - name: Xcode Version run: xcodebuild -version + # Workaround a bug where some Namespace images don't have the the metal + # toolchain installed. We should be able to remove this eventually. + - run: xcodebuild -downloadComponent MetalToolchain + - name: get the Zig deps id: deps run: nix build -L .#deps && echo "deps=$(readlink ./result)" >> $GITHUB_OUTPUT From 2d0f930e6aecc0562b90f35de5d6136bbe3f3e8e Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 21 Aug 2025 09:14:06 -0500 Subject: [PATCH 54/70] gtk-ng: properly skip Zig test --- src/apprt/gtk-ng/ext/actions.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apprt/gtk-ng/ext/actions.zig b/src/apprt/gtk-ng/ext/actions.zig index 9f724c850..7d13af961 100644 --- a/src/apprt/gtk-ng/ext/actions.zig +++ b/src/apprt/gtk-ng/ext/actions.zig @@ -103,7 +103,7 @@ pub fn addAsGroup(comptime T: type, self: *T, comptime name: [:0]const u8, actio test "adding actions to an object" { // This test requires a connection to an active display environment. - if (gtk.initCheck() == 0) return; + if (gtk.initCheck() == 0) return error.SkipZigTest; const callbacks = struct { fn callback(_: *gio.SimpleAction, variant_: ?*glib.Variant, self: *gtk.Box) callconv(.c) void { From 531924e7e70c84c1ead2f5b05210dda4f256dd9b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 07:27:40 -0700 Subject: [PATCH 55/70] terminal: explicitly initialize undefined fields at runtime This works around the Zig issue as noted in the comment. No new Valgrind issues found from this. --- src/terminal/Parser.zig | 34 ++++++++++++++++++------- src/terminal/kitty/graphics_command.zig | 24 ++++++++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/terminal/Parser.zig b/src/terminal/Parser.zig index ed099ee47..0223545e5 100644 --- a/src/terminal/Parser.zig +++ b/src/terminal/Parser.zig @@ -209,26 +209,42 @@ const MAX_INTERMEDIATE = 4; const MAX_PARAMS = 24; /// Current state of the state machine -state: State = .ground, +state: State, /// Intermediate tracking. -intermediates: [MAX_INTERMEDIATE]u8 = undefined, -intermediates_idx: u8 = 0, +intermediates: [MAX_INTERMEDIATE]u8, +intermediates_idx: u8, /// Param tracking, building -params: [MAX_PARAMS]u16 = undefined, -params_sep: Action.CSI.SepList = .initEmpty(), -params_idx: u8 = 0, -param_acc: u16 = 0, -param_acc_idx: u8 = 0, +params: [MAX_PARAMS]u16, +params_sep: Action.CSI.SepList, +params_idx: u8, +param_acc: u16, +param_acc_idx: u8, /// Parser for OSC sequences osc_parser: osc.Parser, pub fn init() Parser { - return .{ + var result: Parser = .{ + .state = .ground, + .intermediates_idx = 0, + .params_sep = .initEmpty(), + .params_idx = 0, + .param_acc = 0, + .param_acc_idx = 0, .osc_parser = .init(), + + .intermediates = undefined, + .params = undefined, }; + if (std.valgrind.runningOnValgrind() > 0) { + // Initialize our undefined fields so Valgrind can catch it. + // https://github.com/ziglang/zig/issues/19148 + result.intermediates = undefined; + result.params = undefined; + } + return result; } pub fn deinit(self: *Parser) void { diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index adc6edafe..dcb4850c9 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -21,14 +21,14 @@ pub const Parser = struct { arena: ArenaAllocator, /// This is the list of KV pairs that we're building up. - kv: KV = .{}, + kv: KV, /// This is used as a buffer to store the key/value of a KV pair. The value /// of a KV pair is at most a 32-bit integer which at most is 10 characters /// (4294967295), plus one character for the sign bit on signed ints. - kv_temp: [11]u8 = undefined, - kv_temp_len: u4 = 0, - kv_current: u8 = 0, // Current kv key + kv_temp: [11]u8, + kv_temp_len: u4, + kv_current: u8, // Current kv key /// This is the list we use to collect the bytes from the data payload. /// The Kitty Graphics protocol specification seems to imply that the @@ -38,7 +38,7 @@ pub const Parser = struct { data: std.ArrayList(u8), /// Internal state for parsing. - state: State = .control_key, + state: State, const State = enum { /// Parsing k/v pairs. The "ignore" variants are in that state @@ -57,10 +57,22 @@ pub const Parser = struct { pub fn init(alloc: Allocator) Parser { var arena = ArenaAllocator.init(alloc); errdefer arena.deinit(); - return .{ + var result: Parser = .{ .arena = arena, .data = std.ArrayList(u8).init(alloc), + .kv = .{}, + .kv_temp_len = 0, + .kv_current = 0, + .state = .control_key, + + .kv_temp = undefined, }; + if (std.valgrind.runningOnValgrind() > 0) { + // Initialize our undefined fields so Valgrind can catch it. + // https://github.com/ziglang/zig/issues/19148 + result.kv_temp = undefined; + } + return result; } pub fn deinit(self: *Parser) void { From 36f7e018ae9bb4d8a7b080dc2405895e08e731fe Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 21 Aug 2025 09:29:17 -0500 Subject: [PATCH 56/70] gtk-ng: more complete GTK startup/shutdown --- src/apprt/gtk-ng/ext/actions.zig | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/apprt/gtk-ng/ext/actions.zig b/src/apprt/gtk-ng/ext/actions.zig index 7d13af961..8499e7de8 100644 --- a/src/apprt/gtk-ng/ext/actions.zig +++ b/src/apprt/gtk-ng/ext/actions.zig @@ -105,6 +105,9 @@ test "adding actions to an object" { // This test requires a connection to an active display environment. if (gtk.initCheck() == 0) return error.SkipZigTest; + _ = glib.MainContext.acquire(null); + defer glib.MainContext.release(null); + const callbacks = struct { fn callback(_: *gio.SimpleAction, variant_: ?*glib.Variant, self: *gtk.Box) callconv(.c) void { const i32_variant_type = glib.ext.VariantType.newFor(i32); @@ -155,4 +158,6 @@ test "adding actions to an object" { const actual = value.getInt(); try testing.expectEqual(expected, actual); + + while (glib.MainContext.iteration(null, 0) != 0) {} } From 7bb493e6acceb62d83ae9a738288bb635319ace8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 07:36:54 -0700 Subject: [PATCH 57/70] ci: switch to Tahoe for builds --- .github/workflows/test.yml | 64 +++++--------------------------------- 1 file changed, 8 insertions(+), 56 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7c5228dbf..b61dd7eaf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,6 @@ jobs: - build-nix - build-snap - build-macos - - build-macos-tahoe - build-macos-matrix - build-windows - flatpak-check-zig-cache @@ -273,53 +272,6 @@ jobs: ghostty-source.tar.gz build-macos: - runs-on: namespace-profile-ghostty-macos-sequoia-edge - needs: test - steps: - - name: Checkout code - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 - with: - nix_path: nixpkgs=channel:nixos-unstable - - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 - with: - name: ghostty - authToken: "${{ secrets.CACHIX_AUTH_TOKEN }}" - - - name: Xcode Select - run: sudo xcode-select -s /Applications/Xcode_26.0.app - - - name: Xcode Version - run: xcodebuild -version - - # Workaround a bug where some Namespace images don't have the the metal - # toolchain installed. We should be able to remove this eventually. - - run: xcodebuild -downloadComponent MetalToolchain - - - name: get the Zig deps - id: deps - run: nix build -L .#deps && echo "deps=$(readlink ./result)" >> $GITHUB_OUTPUT - - # GhosttyKit is the framework that is built from Zig for our native - # Mac app to access. - - name: Build GhosttyKit - run: nix develop -c zig build --system ${{ steps.deps.outputs.deps }} -Demit-macos-app=false - - # The native app is built with native Xcode tooling. This also does - # codesigning. IMPORTANT: this must NOT run in a Nix environment. - # Nix breaks xcodebuild so this has to be run outside. - - name: Build Ghostty.app - run: cd macos && xcodebuild -target Ghostty - - # Build the iOS target without code signing just to verify it works. - - name: Build Ghostty iOS - run: | - cd macos - xcodebuild -target Ghostty-iOS "CODE_SIGNING_ALLOWED=NO" - - build-macos-tahoe: runs-on: namespace-profile-ghostty-macos-tahoe needs: test steps: @@ -363,16 +315,16 @@ jobs: xcodebuild -target Ghostty-iOS "CODE_SIGNING_ALLOWED=NO" build-macos-matrix: - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe needs: test steps: - name: Checkout code uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + # TODO(tahoe): https://github.com/NixOS/nix/issues/13342 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty @@ -682,16 +634,16 @@ jobs: nix develop -c zig build -Dsentry=${{ matrix.sentry }} test-macos: - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe needs: test steps: - name: Checkout code uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + # TODO(tahoe): https://github.com/NixOS/nix/issues/13342 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty From 82f7cd21339ac83e5e23feb38717b3efb66d98ee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 07:43:26 -0700 Subject: [PATCH 58/70] ci: switch release builds to tahoe builders --- .github/workflows/release-pr.yml | 12 ++++++------ .github/workflows/release-tag.yml | 8 ++++---- .github/workflows/release-tip.yml | 24 ++++++++++++------------ 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index cb1e8c1ef..46e18c450 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -47,7 +47,7 @@ jobs: sentry-cli dif upload --project ghostty --wait dsym.zip build-macos: - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 steps: - name: Checkout code @@ -57,9 +57,9 @@ jobs: fetch-depth: 0 # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty @@ -202,7 +202,7 @@ jobs: destination-dir: ./ build-macos-debug: - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 steps: - name: Checkout code @@ -212,9 +212,9 @@ jobs: fetch-depth: 0 # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index bbd013f4e..38eef0b92 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -120,7 +120,7 @@ jobs: build-macos: needs: [setup] - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 env: GHOSTTY_VERSION: ${{ needs.setup.outputs.version }} @@ -130,9 +130,9 @@ jobs: - name: Checkout code uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty @@ -294,7 +294,7 @@ jobs: appcast: needs: [setup, build-macos] - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe env: GHOSTTY_VERSION: ${{ needs.setup.outputs.version }} GHOSTTY_BUILD: ${{ needs.setup.outputs.build }} diff --git a/.github/workflows/release-tip.yml b/.github/workflows/release-tip.yml index 791482518..763acf6fa 100644 --- a/.github/workflows/release-tip.yml +++ b/.github/workflows/release-tip.yml @@ -154,7 +154,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 steps: - name: Checkout code @@ -163,10 +163,10 @@ jobs: # Important so that build number generation works fetch-depth: 0 - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + # TODO(tahoe): https://github.com/NixOS/nix/issues/13342 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty @@ -374,7 +374,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 steps: - name: Checkout code @@ -383,10 +383,10 @@ jobs: # Important so that build number generation works fetch-depth: 0 - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + # TODO(tahoe): https://github.com/NixOS/nix/issues/13342 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty @@ -554,7 +554,7 @@ jobs: ) }} - runs-on: namespace-profile-ghostty-macos-sequoia-edge + runs-on: namespace-profile-ghostty-macos-tahoe timeout-minutes: 90 steps: - name: Checkout code @@ -563,10 +563,10 @@ jobs: # Important so that build number generation works fetch-depth: 0 - # Install Nix and use that to run our tests so our environment matches exactly. - - uses: cachix/install-nix-action@fc6e360bedc9ee72d75e701397f0bb30dce77568 # v31.5.2 + # TODO(tahoe): https://github.com/NixOS/nix/issues/13342 + - uses: DeterminateSystems/nix-installer-action@main with: - nix_path: nixpkgs=channel:nixos-unstable + determinate: true - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 with: name: ghostty From e92fe9d9f814a8338b857e1f42dc5fd767b02ed3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 09:22:11 -0700 Subject: [PATCH 59/70] ci: add timeout to snap and windows jobs There have been times these runaway taking forever for unknown reasons. --- .github/workflows/test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b61dd7eaf..0f445dba3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -363,6 +363,7 @@ jobs: os: [namespace-profile-ghostty-snap, namespace-profile-ghostty-snap-arm64] runs-on: ${{ matrix.os }} + timeout-minutes: 45 needs: [test, build-dist] env: ZIG_LOCAL_CACHE_DIR: /zig/local-cache @@ -397,6 +398,7 @@ jobs: runs-on: windows-2022 # this will not stop other jobs from running continue-on-error: true + timeout-minutes: 45 needs: test steps: - name: Checkout code From 29419e7aacdf2033f95d170779a283fc51f15ae8 Mon Sep 17 00:00:00 2001 From: Nicholas Mata Date: Thu, 14 Aug 2025 03:14:35 -0700 Subject: [PATCH 60/70] Make macos icon persistent even when app is closed --- macos/Sources/App/macOS/AppDelegate.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 5940547b5..c6ad36e56 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -119,6 +119,9 @@ class AppDelegate: NSObject, @Published private(set) var appIcon: NSImage? = nil { didSet { NSApplication.shared.applicationIconImage = appIcon + let appPath = Bundle.main.bundlePath + NSWorkspace.shared.setIcon(appIcon, forFile: appPath, options: []) + NSWorkspace.shared.noteFileSystemChanged(appPath) } } From 5948bd3f02f3ee8f116a570177e49f847e870502 Mon Sep 17 00:00:00 2001 From: Nicholas Mata Date: Thu, 14 Aug 2025 03:17:33 -0700 Subject: [PATCH 61/70] Add support for 'custom' on 'macos_icon' to support a completely custom app icon --- macos/Sources/App/macOS/AppDelegate.swift | 7 +++++++ macos/Sources/Ghostty/Ghostty.Config.swift | 14 ++++++++++++++ macos/Sources/Ghostty/Package.swift | 1 + src/config/Config.zig | 12 ++++++++++++ 4 files changed, 34 insertions(+) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index c6ad36e56..3244f163c 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -837,6 +837,13 @@ class AppDelegate: NSObject, case .xray: self.appIcon = NSImage(named: "XrayImage")! + case .custom: + if let userIcon = NSImage(contentsOfFile: config.macosCustomIcon) { + self.appIcon = userIcon + } else { + self.appIcon = nil // Revert back to official icon if invalid location + } + case .customStyle: guard let ghostColor = config.macosIconGhostColor else { break } guard let screenColors = config.macosIconScreenColor else { break } diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index 241c10632..0f6276d01 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -301,6 +301,20 @@ extension Ghostty { return MacOSIcon(rawValue: str) ?? defaultValue } + var macosCustomIcon: String { + let homeDirURL = FileManager.default.homeDirectoryForCurrentUser + let ghosttyConfigIconPath = homeDirURL.appendingPathComponent( + ".config/ghostty/Ghostty.icns", + conformingTo: .fileURL).path() + let defaultValue = ghosttyConfigIconPath + guard let config = self.config else { return defaultValue } + var v: UnsafePointer? = nil + let key = "macos-custom-icon" + guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return defaultValue } + guard let ptr = v else { return defaultValue } + return String(cString: ptr) + } + var macosIconFrame: MacOSIconFrame { let defaultValue = MacOSIconFrame.aluminum guard let config = self.config else { return defaultValue } diff --git a/macos/Sources/Ghostty/Package.swift b/macos/Sources/Ghostty/Package.swift index 9b05934df..73487f1bd 100644 --- a/macos/Sources/Ghostty/Package.swift +++ b/macos/Sources/Ghostty/Package.swift @@ -280,6 +280,7 @@ extension Ghostty { case paper case retro case xray + case custom case customStyle = "custom-style" } diff --git a/src/config/Config.zig b/src/config/Config.zig index 5ac2d6617..178b9f851 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2735,6 +2735,8 @@ keybind: Keybinds = .{}, /// * `blueprint`, `chalkboard`, `microchip`, `glass`, `holographic`, /// `paper`, `retro`, `xray` - Official variants of the Ghostty icon /// hand-created by artists (no AI). +/// * `custom` - Use a completely custom icon. The location must be specified +/// using the additional `macos-custom-icon` configuration /// * `custom-style` - Use the official Ghostty icon but with custom /// styles applied to various layers. The custom styles must be /// specified using the additional `macos-icon`-prefixed configurations. @@ -2753,6 +2755,15 @@ keybind: Keybinds = .{}, /// effort. @"macos-icon": MacAppIcon = .official, +/// The absolute path to the custom icon file. +/// Supported formats include PNG, JPEG, and ICNS. +/// +/// Defaults to `~/.config/ghostty/Ghostty.icns` +/// +/// Note: This configuration is required when `macos-icon` is set to +/// `custom` +@"macos-custom-icon": ?[]const u8 = null, + /// The material to use for the frame of the macOS app icon. /// /// Valid values: @@ -6975,6 +6986,7 @@ pub const MacAppIcon = enum { paper, retro, xray, + custom, @"custom-style", }; From f1c68f698b588bdc55ac86e68c134eaed7514691 Mon Sep 17 00:00:00 2001 From: Nicholas Mata Date: Thu, 14 Aug 2025 03:18:46 -0700 Subject: [PATCH 62/70] Correct Swift formatting inconsistencies --- macos/Sources/App/macOS/AppDelegate.swift | 4 ++-- macos/Sources/Ghostty/Ghostty.Config.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 3244f163c..cbd3668c5 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -258,13 +258,13 @@ class AppDelegate: NSObject, // Setup signal handlers setupSignals() - + // If we launched via zig run then we need to force foreground. if Ghostty.launchSource == .zig_run { // This never gets called until we click the dock icon. This forces it // activate immediately. applicationDidBecomeActive(.init(name: NSApplication.didBecomeActiveNotification)) - + // We run in the background, this forces us to the front. DispatchQueue.main.async { NSApp.setActivationPolicy(.regular) diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index 0f6276d01..acc907edb 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -164,7 +164,7 @@ extension Ghostty { let key = "window-position-x" return ghostty_config_get(config, &v, key, UInt(key.count)) ? v : nil } - + var windowPositionY: Int16? { guard let config = self.config else { return nil } var v: Int16 = 0 From f178f4419e1815d514b267c7bc890a4f2c91e9ec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 10:03:23 -0700 Subject: [PATCH 63/70] macos: fix iOS builds --- macos/Sources/Ghostty/Ghostty.Config.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index acc907edb..a223b2a0a 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -302,6 +302,7 @@ extension Ghostty { } var macosCustomIcon: String { + #if os(macOS) let homeDirURL = FileManager.default.homeDirectoryForCurrentUser let ghosttyConfigIconPath = homeDirURL.appendingPathComponent( ".config/ghostty/Ghostty.icns", @@ -313,6 +314,9 @@ extension Ghostty { guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return defaultValue } guard let ptr = v else { return defaultValue } return String(cString: ptr) + #else + return "" + #endif } var macosIconFrame: MacOSIconFrame { From f9ad061ea8ed4da5cd954eb4e145ae1b857b085d Mon Sep 17 00:00:00 2001 From: David Keegan Date: Thu, 31 Jul 2025 13:16:52 -0600 Subject: [PATCH 64/70] Add macos-dock-drop-folder-behavior configuration option This adds a new configuration option that controls whether folders dropped onto the Ghostty dock icon open in a new tab (default) or a new window. The option accepts two values: - tab: Opens folders in a new tab in the main window (default) - window: Opens folders in a new window This is useful for users who prefer window-based workflows over tab-based workflows when opening folders via drag and drop. --- macos/Sources/App/macOS/AppDelegate.swift | 14 ++++++++++---- macos/Sources/Ghostty/Ghostty.Config.swift | 11 +++++++++++ macos/Sources/Ghostty/Package.swift | 6 ++++++ src/config/Config.zig | 20 ++++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index cbd3668c5..a4c6986a9 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -402,11 +402,17 @@ class AppDelegate: NSObject, var config = Ghostty.SurfaceConfiguration() if (isDirectory.boolValue) { - // When opening a directory, create a new tab in the main - // window with that as the working directory. - // If no windows exist, a new one will be created. + // When opening a directory, check the configuration to decide + // whether to open in a new tab or new window. config.workingDirectory = filename - _ = TerminalController.newTab(ghostty, withBaseConfig: config) + + let behavior = ghostty.config.macosDockDropFolderBehavior + if behavior == .window { + _ = TerminalController.newWindow(ghostty, withBaseConfig: config) + } else { + // Default to tab behavior + _ = TerminalController.newTab(ghostty, withBaseConfig: config) + } } else { // When opening a file, we want to execute the file. To do this, we // don't override the command directly, because it won't load the diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index a223b2a0a..fca272a3e 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -282,6 +282,17 @@ extension Ghostty { return MacOSTitlebarProxyIcon(rawValue: str) ?? defaultValue } + var macosDockDropFolderBehavior: MacOSDockDropFolderBehavior { + let defaultValue = MacOSDockDropFolderBehavior.tab + guard let config = self.config else { return defaultValue } + var v: UnsafePointer? = nil + let key = "macos-dock-drop-folder-behavior" + guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return defaultValue } + guard let ptr = v else { return defaultValue } + let str = String(cString: ptr) + return MacOSDockDropFolderBehavior(rawValue: str) ?? defaultValue + } + var macosWindowShadow: Bool { guard let config = self.config else { return false } var v = false; diff --git a/macos/Sources/Ghostty/Package.swift b/macos/Sources/Ghostty/Package.swift index 73487f1bd..6ba88045b 100644 --- a/macos/Sources/Ghostty/Package.swift +++ b/macos/Sources/Ghostty/Package.swift @@ -304,6 +304,12 @@ extension Ghostty { case hidden } + /// Enum for the macos-dock-drop-folder-behavior config option + enum MacOSDockDropFolderBehavior: String { + case tab + case window + } + /// Enum for auto-update-channel config option enum AutoUpdateChannel: String { case tip diff --git a/src/config/Config.zig b/src/config/Config.zig index 178b9f851..7ae767fd6 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2631,6 +2631,20 @@ keybind: Keybinds = .{}, /// editor, etc. @"macos-titlebar-proxy-icon": MacTitlebarProxyIcon = .visible, +/// Controls the behavior when dropping a folder onto the Ghostty dock icon +/// on macOS. +/// +/// Valid values are: +/// +/// * `tab` - Open the folder in a new tab in the main window (default). +/// * `window` - Open the folder in a new window. +/// +/// The default value is `tab`. +/// +/// This setting is only supported on macOS and has no effect on other +/// platforms. +@"macos-dock-drop-folder-behavior": MacOSDockDropFolderBehavior = .tab, + /// macOS doesn't have a distinct "alt" key and instead has the "option" /// key which behaves slightly differently. On macOS by default, the /// option key plus a character will sometimes produce a Unicode character. @@ -7082,6 +7096,12 @@ pub const WindowNewTabPosition = enum { end, }; +/// See macos-dock-drop-folder-behavior +pub const MacOSDockDropFolderBehavior = enum { + tab, + window, +}; + /// See window-show-tab-bar pub const WindowShowTabBar = enum { always, From e01ff4093a1e3b3106daef892c33fc9abb9ea249 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 10:45:12 -0700 Subject: [PATCH 65/70] macos: have macos-dock-drop-behavior apply to all drops --- macos/Sources/App/macOS/AppDelegate.swift | 15 +++++---------- macos/Sources/Ghostty/Ghostty.Config.swift | 13 +++++++++---- macos/Sources/Ghostty/Package.swift | 6 ------ src/config/Config.zig | 19 ++++++++++--------- src/config/formatter.zig | 2 ++ 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index a4c6986a9..d54b49642 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -405,14 +405,6 @@ class AppDelegate: NSObject, // When opening a directory, check the configuration to decide // whether to open in a new tab or new window. config.workingDirectory = filename - - let behavior = ghostty.config.macosDockDropFolderBehavior - if behavior == .window { - _ = TerminalController.newWindow(ghostty, withBaseConfig: config) - } else { - // Default to tab behavior - _ = TerminalController.newTab(ghostty, withBaseConfig: config) - } } else { // When opening a file, we want to execute the file. To do this, we // don't override the command directly, because it won't load the @@ -424,8 +416,11 @@ class AppDelegate: NSObject, // Set the parent directory to our working directory so that relative // paths in scripts work. config.workingDirectory = (filename as NSString).deletingLastPathComponent - - _ = TerminalController.newWindow(ghostty, withBaseConfig: config) + } + + switch ghostty.config.macosDockDropBehavior { + case .new_tab: _ = TerminalController.newTab(ghostty, withBaseConfig: config) + case .new_window: _ = TerminalController.newWindow(ghostty, withBaseConfig: config) } return true diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index fca272a3e..6992f59f6 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -282,15 +282,15 @@ extension Ghostty { return MacOSTitlebarProxyIcon(rawValue: str) ?? defaultValue } - var macosDockDropFolderBehavior: MacOSDockDropFolderBehavior { - let defaultValue = MacOSDockDropFolderBehavior.tab + var macosDockDropBehavior: MacDockDropBehavior { + let defaultValue = MacDockDropBehavior.new_tab guard let config = self.config else { return defaultValue } var v: UnsafePointer? = nil - let key = "macos-dock-drop-folder-behavior" + let key = "macos-dock-drop-behavior" guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return defaultValue } guard let ptr = v else { return defaultValue } let str = String(cString: ptr) - return MacOSDockDropFolderBehavior(rawValue: str) ?? defaultValue + return MacDockDropBehavior(rawValue: str) ?? defaultValue } var macosWindowShadow: Bool { @@ -618,6 +618,11 @@ extension Ghostty.Config { static let attention = BellFeatures(rawValue: 1 << 2) static let title = BellFeatures(rawValue: 1 << 3) } + + enum MacDockDropBehavior: String { + case new_tab = "new-tab" + case new_window = "new-window" + } enum MacHidden : String { case never diff --git a/macos/Sources/Ghostty/Package.swift b/macos/Sources/Ghostty/Package.swift index 6ba88045b..73487f1bd 100644 --- a/macos/Sources/Ghostty/Package.swift +++ b/macos/Sources/Ghostty/Package.swift @@ -304,12 +304,6 @@ extension Ghostty { case hidden } - /// Enum for the macos-dock-drop-folder-behavior config option - enum MacOSDockDropFolderBehavior: String { - case tab - case window - } - /// Enum for auto-update-channel config option enum AutoUpdateChannel: String { case tip diff --git a/src/config/Config.zig b/src/config/Config.zig index 7ae767fd6..d8fcfa1d7 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2631,19 +2631,20 @@ keybind: Keybinds = .{}, /// editor, etc. @"macos-titlebar-proxy-icon": MacTitlebarProxyIcon = .visible, -/// Controls the behavior when dropping a folder onto the Ghostty dock icon -/// on macOS. +/// Controls the windowing behavior when dropping a file or folder +/// onto the Ghostty icon in the macOS dock. /// /// Valid values are: /// -/// * `tab` - Open the folder in a new tab in the main window (default). -/// * `window` - Open the folder in a new window. +/// * `new-tab` - Create a new tab in the current window, or open +/// a new window if none exist. +/// * `new-window` - Create a new window unconditionally. /// -/// The default value is `tab`. +/// The default value is `new-tab`. /// /// This setting is only supported on macOS and has no effect on other /// platforms. -@"macos-dock-drop-folder-behavior": MacOSDockDropFolderBehavior = .tab, +@"macos-dock-drop-behavior": MacOSDockDropBehavior = .@"new-tab", /// macOS doesn't have a distinct "alt" key and instead has the "option" /// key which behaves slightly differently. On macOS by default, the @@ -7096,9 +7097,9 @@ pub const WindowNewTabPosition = enum { end, }; -/// See macos-dock-drop-folder-behavior -pub const MacOSDockDropFolderBehavior = enum { - tab, +/// See macos-dock-drop-behavior +pub const MacOSDockDropBehavior = enum { + @"new-tab", window, }; diff --git a/src/config/formatter.zig b/src/config/formatter.zig index cabf80953..a42395c19 100644 --- a/src/config/formatter.zig +++ b/src/config/formatter.zig @@ -147,6 +147,8 @@ pub const FileFormatter = struct { opts: std.fmt.FormatOptions, writer: anytype, ) !void { + @setEvalBranchQuota(10_000); + _ = layout; _ = opts; From 534aa508d670af5de859401cec774a02e8159ca7 Mon Sep 17 00:00:00 2001 From: Leah Amelia Chen Date: Thu, 21 Aug 2025 15:28:48 +0800 Subject: [PATCH 66/70] gtk-ng: allow XKB remaps for non-writing-system keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compromise solution to #7356 XKB is naughty. It's really really naughty. I don't understand why we didn't just kill XKB with hammers during the Wayland migration and change it for something much better. I don't understand why we're content with what amounts to an OS-level software key remapper that completely jumbles information about original physical key codes in order to fake keyboard layouts, and not just let users who really want to remap keys use some sort of evdev or udev-based mapper program. In a sane system like macOS, the "c" key is always the "c" key, but it's understood to produce the Unicode character "ц" when using a Russian layout. XKB defies sanity, and just pretends that your "c" key is actually a "ц" key instead, and so when you ask for the keybind "Ctrl+C" it just shrugs in apathy (#7309). And so, we took matters into our own hands and interpreted hardware keycodes ourselves. But then, a *lot* of people have the ingrained muscle memory of swapping Escape with Caps Lock so that it is easier to hit. We respect that. In a sane system, they would use a remapper that actually makes the system think you've hit the Escape key when in reality you've hit the Caps Lock key, so in all intents and purposes to the OS and any app developer, these two just have their wires swapped. But not on Linux. Somehow this and the aforementioned case should be treated by the same key transform algorithm, which is completely diabolical. As a result, we have to settle for a compromise that truly satisfies neither party — by allowing XKB remaps for keys that don't really change depending on the layout. The Linux input stack besets all hopes and aspirations. --- src/apprt/gtk-ng/class/surface.zig | 23 +++++++-- src/input/key.zig | 78 ++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk-ng/class/surface.zig b/src/apprt/gtk-ng/class/surface.zig index b412d6b6b..2debff93b 100644 --- a/src/apprt/gtk-ng/class/surface.zig +++ b/src/apprt/gtk-ng/class/surface.zig @@ -838,7 +838,7 @@ pub const Surface = extern struct { // such as single quote on a US international keyboard layout. if (priv.im_composing) return true; - // If we were composing and now we're not it means that we committed + // If we were composing and now we're not, it means that we committed // the text. We also don't want to encode a key event for this. // Example: enable Japanese input method, press "konn" and then // press enter. The final enter should not be encoded and "konn" @@ -878,9 +878,24 @@ pub const Surface = extern struct { // We want to get the physical unmapped key to process physical keybinds. // (These are keybinds explicitly marked as requesting physical mapping). - const physical_key = keycode: for (input.keycodes.entries) |entry| { - if (entry.native == keycode) break :keycode entry.key; - } else .unidentified; + const physical_key = keycode: { + const w3c_key: input.Key = w3c: for (input.keycodes.entries) |entry| { + if (entry.native == keycode) break :w3c entry.key; + } else .unidentified; + + // If the key should be remappable, then consult the pre-remapped + // XKB keyval/keysym to get the (possibly) remapped key. + // + // See the docs for `shouldBeRemappable` for why we even have to + // do this in the first place. + if (w3c_key.shouldBeRemappable()) { + if (gtk_key.keyFromKeyval(keyval)) |remapped| + break :keycode remapped; + } + + // Return the original physical key + break :keycode w3c_key; + }; // Get our modifier for the event const mods: input.Mods = gtk_key.eventMods( diff --git a/src/input/key.zig b/src/input/key.zig index 28aa3ccf4..a3814fb55 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -589,6 +589,84 @@ pub const Key = enum(c_int) { }; } + /// Whether this key should be remappable by the operating system. + /// + /// On certain OSes (namely Linux and the BSDs) certain keys like the + /// functional keys are expected to be remappable by the user, such as + /// in the very common use case of swapping the Caps Lock key with the + /// Escape key with the XKB option `caps:swapescape`. + /// + /// However, the way XKB implements this is by essentially acting as a + /// software key remapper that destroys all information about the original + /// physical key, leading to very annoying bugs like #7309 where the + /// physical key `XKB_KEY_c` gets remapped into `XKB_KEY_Cyrillic_tse`, + /// which causes all of our physical key handling to completely break down. + /// _Very naughty._ + /// + /// As a compromise, given that writing system keys (§3.1.1) comprise the + /// majority of keys that "change meaning [...] based on the current locale + /// and keyboard layout", we allow all other keys to be remapped by default + /// since they should be fairly harmless. We might consider making this + /// configurable, but for now this should at least placate most people. + pub fn shouldBeRemappable(self: Key) bool { + return switch (self) { + // "Writing System Keys" § 3.1.1 + .backquote, + .backslash, + .bracket_left, + .bracket_right, + .comma, + .digit_0, + .digit_1, + .digit_2, + .digit_3, + .digit_4, + .digit_5, + .digit_6, + .digit_7, + .digit_8, + .digit_9, + .equal, + .intl_backslash, + .intl_ro, + .intl_yen, + .key_a, + .key_b, + .key_c, + .key_d, + .key_e, + .key_f, + .key_g, + .key_h, + .key_i, + .key_j, + .key_k, + .key_l, + .key_m, + .key_n, + .key_o, + .key_p, + .key_q, + .key_r, + .key_s, + .key_t, + .key_u, + .key_v, + .key_w, + .key_x, + .key_y, + .key_z, + .minus, + .period, + .quote, + .semicolon, + .slash, + => false, + + else => true, + }; + } + /// Returns true if this is a keypad key. pub fn keypad(self: Key) bool { return switch (self) { From 795c745491767c9e7830f9d62bf37b04c5ab4681 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 11:44:57 -0700 Subject: [PATCH 67/70] ci: add 30 minute timeout to valgrind It usually takes less than a few minutes right now. Something is wrong if it takes more than that. --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0f445dba3..8910d8c07 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1010,6 +1010,7 @@ jobs: valgrind: if: github.repository == 'ghostty-org/ghostty' runs-on: namespace-profile-ghostty-lg + timeout-minutes: 30 needs: test env: ZIG_LOCAL_CACHE_DIR: /zig/local-cache From f736ee88657420f09b0e7139de8bd2c6d721a8e0 Mon Sep 17 00:00:00 2001 From: Aljoscha Krettek Date: Fri, 18 Jul 2025 14:52:58 +0200 Subject: [PATCH 68/70] macos: in new_window action, activate App MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change makes sure that the new window is focused and visible. When commit 33d128bcff2ee529359a844bde50c3aa9dfed460 removed the TerminalManager class and moved its functionality into TerminalController, it accidentally removed app activation for windows triggered by global keybinds. How the bug works: 1. Menu actions (like File → New Window) call AppDelegate.newWindow() which: 2. Calls TerminalController.newWindow() 3. AND explicitly calls NSApp.activate(ignoringOtherApps: true) in the AppDelegate 4. Global keybind actions trigger ghosttyNewWindow() notification handler which: 5. Only calls TerminalController.newWindow() 6. Does NOT call NSApp.activate(ignoringOtherApps: true) 7. While TerminalController.newWindow() does call NSApp.activate(ignoringOtherApps: true) internally, this call happens before the async dispatch that shows the window, so the activation occurs but the window isn't focused when it's actually shown. 8. In the old TerminalManager.newWindow(), the activation happened immediately before the async dispatch, ensuring proper timing for window focus. The fix would be to either move the NSApp.activate() call back into TerminalController.newWindow(), as it was for TerminalManager, or add the activation call to the notification handlers in AppDelegate. --- macos/Sources/Features/Terminal/TerminalController.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index c5e1c413f..7a11ac3ae 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -214,6 +214,10 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr } } + // All new_window actions force our app to be active, so that the new + // window is focused and visible. + NSApp.activate(ignoringOtherApps: true) + // We're dispatching this async because otherwise the lastCascadePoint doesn't // take effect. Our best theory is there is some next-event-loop-tick logic // that Cocoa is doing that we need to be after. From 1ce56a12fa3db6554d062008a95c008564ad5de4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Aug 2025 14:07:04 -0700 Subject: [PATCH 69/70] update sparkle There aren't any noteworthy changes here we're just using a very old version. Additionally, our CI was using... different versions! --- .github/workflows/release-pr.yml | 4 ++-- .github/workflows/release-tag.yml | 4 ++-- .github/workflows/release-tip.yml | 6 +++--- .../xcshareddata/swiftpm/Package.resolved | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index 46e18c450..8c42a2a55 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -68,7 +68,7 @@ jobs: # Setup Sparkle - name: Setup Sparkle env: - SPARKLE_VERSION: 2.6.4 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle @@ -223,7 +223,7 @@ jobs: # Setup Sparkle - name: Setup Sparkle env: - SPARKLE_VERSION: 2.5.1 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 38eef0b92..f7fb72f65 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -146,7 +146,7 @@ jobs: - name: Setup Sparkle env: - SPARKLE_VERSION: 2.6.4 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle @@ -311,7 +311,7 @@ jobs: - name: Setup Sparkle env: - SPARKLE_VERSION: 2.6.4 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle diff --git a/.github/workflows/release-tip.yml b/.github/workflows/release-tip.yml index 763acf6fa..bae096054 100644 --- a/.github/workflows/release-tip.yml +++ b/.github/workflows/release-tip.yml @@ -181,7 +181,7 @@ jobs: # Setup Sparkle - name: Setup Sparkle env: - SPARKLE_VERSION: 2.6.4 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle @@ -401,7 +401,7 @@ jobs: # Setup Sparkle - name: Setup Sparkle env: - SPARKLE_VERSION: 2.5.1 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle @@ -581,7 +581,7 @@ jobs: # Setup Sparkle - name: Setup Sparkle env: - SPARKLE_VERSION: 2.5.1 + SPARKLE_VERSION: 2.7.1 run: | mkdir -p .action/sparkle cd .action/sparkle diff --git a/macos/Ghostty.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/macos/Ghostty.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 5ace476e0..7ecedbc14 100644 --- a/macos/Ghostty.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/macos/Ghostty.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -6,8 +6,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/sparkle-project/Sparkle", "state" : { - "revision" : "0ef1ee0220239b3776f433314515fd849025673f", - "version" : "2.6.4" + "revision" : "df074165274afaa39539c05d57b0832620775b11", + "version" : "2.7.1" } } ], From 7d60c7c75bec6b78b51f2b1053b1ed0ad668ec68 Mon Sep 17 00:00:00 2001 From: Daniel Wennberg Date: Sat, 26 Jul 2025 14:40:50 -0700 Subject: [PATCH 70/70] macos: move activation to after new window/tab is created --- macos/Sources/App/macOS/AppDelegate.swift | 8 -------- .../Features/Terminal/TerminalController.swift | 12 ++++++++---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index d54b49642..c00025bf5 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -957,18 +957,10 @@ class AppDelegate: NSObject, @IBAction func newWindow(_ sender: Any?) { _ = TerminalController.newWindow(ghostty) - - // We also activate our app so that it becomes front. This may be - // necessary for the dock menu. - NSApp.activate(ignoringOtherApps: true) } @IBAction func newTab(_ sender: Any?) { _ = TerminalController.newTab(ghostty) - - // We also activate our app so that it becomes front. This may be - // necessary for the dock menu. - NSApp.activate(ignoringOtherApps: true) } @IBAction func closeAllWindows(_ sender: Any?) { diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index 7a11ac3ae..ec56fb934 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -214,10 +214,6 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr } } - // All new_window actions force our app to be active, so that the new - // window is focused and visible. - NSApp.activate(ignoringOtherApps: true) - // We're dispatching this async because otherwise the lastCascadePoint doesn't // take effect. Our best theory is there is some next-event-loop-tick logic // that Cocoa is doing that we need to be after. @@ -230,6 +226,10 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr } c.showWindow(self) + + // All new_window actions force our app to be active, so that the new + // window is focused and visible. + NSApp.activate(ignoringOtherApps: true) } // Setup our undo @@ -336,6 +336,10 @@ class TerminalController: BaseTerminalController, TabGroupCloseCoordinator.Contr controller.showWindow(self) window.makeKeyAndOrderFront(self) + + // We also activate our app so that it becomes front. This may be + // necessary for the dock menu. + NSApp.activate(ignoringOtherApps: true) } // It takes an event loop cycle until the macOS tabGroup state becomes