From 1270e04480c7925063ce2f037ff085566d2a0b45 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 2 Jul 2025 17:43:05 -0600 Subject: [PATCH 01/12] renderer/opengl: maybe fix issue with cell bg alignment By using integers for the fragcoords I may have stepped on an edge case which causes cell background positions to be shifted by 1 px under some circumstances. I couldn't reproduce that issue in a VM, so I'm making this commit for the user who was having the problem to test it. --- src/renderer/shaders/glsl/cell_bg.f.glsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/shaders/glsl/cell_bg.f.glsl b/src/renderer/shaders/glsl/cell_bg.f.glsl index 7ba6caaa6..fa48c6736 100644 --- a/src/renderer/shaders/glsl/cell_bg.f.glsl +++ b/src/renderer/shaders/glsl/cell_bg.f.glsl @@ -1,7 +1,7 @@ #include "common.glsl" // Position the origin to the upper left -layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord; +layout(origin_upper_left) in vec4 gl_FragCoord; // Must declare this output for some versions of OpenGL. layout(location = 0) out vec4 out_FragColor; From 182f8ddd1a00d9abcdcee5d7179ecabcdd126a0e Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Wed, 2 Jul 2025 17:37:30 -0700 Subject: [PATCH 02/12] Do not resolve the symbolic link for the initial working directory --- src/termio/Exec.zig | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index b8f838cf9..598617a12 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -847,6 +847,15 @@ const Subprocess = struct { else null; + // Propagate the current working directory (CWD) to the shell, enabling + // the shell to display the current directory name rather than the + // resolved path for symbolic links. This is important and based + // on the same behavior in Konsole and Kitty (see the linked issues): + // https://bugs.kde.org/show_bug.cgi?id=242114 + // https://github.com/kovidgoyal/kitty/issues/1595 + // https://github.com/ghostty-org/ghostty/discussions/7769 + if (cwd) |pwd| try env.put("PWD", pwd); + // If we have a cgroup, then we copy that into our arena so the // memory remains valid when we start. const linux_cgroup: Command.LinuxCgroup = cgroup: { From 9e341a3d60212b361c45527a82e8c8f774e6cf47 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 18 Jan 2025 20:47:23 -0600 Subject: [PATCH 03/12] Created tagged union for selection colors, enabled parsing Implemented cell color for Metal Removed use of selection-invert-fg-bg Mirrored feature to OpenGL Added tests for SelectionColor Fixed selection on inverted cell behavior Implemented cell colors for cursor-text Implemented cell colors for cursor-color, removed uses of cursor-invert-fg-bg during rendering Updated docs for dynamically colored options Updated docstrings, cleaned up awkward formatting, and moved style computation to avoid unnecssary invocations Bump version in docstrings --- src/config/Config.zig | 73 +++++++++++++++++++++++++++++++++-- src/termio/Termio.zig | 11 +++--- src/termio/stream_handler.zig | 7 +++- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index f36132ea9..76f91f6a8 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -591,8 +591,11 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// the selection color is just the inverted window background and foreground /// (note: not to be confused with the cell bg/fg). /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -@"selection-foreground": ?Color = null, -@"selection-background": ?Color = null, +/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// the cell foreground color, or `cell-background` to match the cell +/// background color. +@"selection-foreground": ?DynamicColor = null, +@"selection-background": ?DynamicColor = null, /// Swap the foreground and background colors of cells for selection. This /// option overrides the `selection-foreground` and `selection-background` @@ -600,6 +603,10 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// /// If you select across cells with differing foregrounds and backgrounds, the /// selection color will vary across the selection. +/// +/// Warning: This option has been deprecated as of version 1.1.1. Instead, +/// users should set `selection-foreground` and `selection-background` to +/// `cell-background` and `cell-foreground`, respectively. @"selection-invert-fg-bg": bool = false, /// Whether to clear selected text when typing. This defaults to `true`. @@ -645,10 +652,17 @@ palette: Palette = .{}, /// The color of the cursor. If this is not set, a default will be chosen. /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -@"cursor-color": ?Color = null, +/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// the cell foreground color, or `cell-background` to match the cell +/// background color. +@"cursor-color": ?DynamicColor = null, /// Swap the foreground and background colors of the cell under the cursor. This /// option overrides the `cursor-color` and `cursor-text` options. +/// +/// Warning: This option has been deprecated as of version 1.1.1. Instead, +/// users should set `cursor-color` and `cursor-text` to `cell-foreground` and +/// `cell-background`, respectively. @"cursor-invert-fg-bg": bool = false, /// The opacity level (opposite of transparency) of the cursor. A value of 1 @@ -699,7 +713,10 @@ palette: Palette = .{}, /// The color of the text under the cursor. If this is not set, a default will /// be chosen. /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -@"cursor-text": ?Color = null, +/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// the cell foreground color, or `cell-background` to match the cell +/// background color. +@"cursor-text": ?DynamicColor = null, /// Enables the ability to move the cursor at prompts by using `alt+click` on /// Linux and `option+click` on macOS. @@ -4409,6 +4426,54 @@ pub const Color = struct { } }; +/// Represents the color values that can be set to a non-static value. +/// +/// Can either be a Color or one of the special values +/// "cell-foreground" or "cell-background". +pub const DynamicColor = union(enum) { + color: Color, + @"cell-foreground", + @"cell-background", + + pub fn parseCLI(input_: ?[]const u8) !DynamicColor { + const input = input_ orelse return error.ValueRequired; + + if (std.mem.eql(u8, input, "cell-foreground")) return .@"cell-foreground"; + if (std.mem.eql(u8, input, "cell-background")) return .@"cell-background"; + + return DynamicColor{ .color = try Color.parseCLI(input) }; + } + + /// Used by Formatter + pub fn formatEntry(self: DynamicColor, formatter: anytype) !void { + switch (self) { + .color => try self.color.formatEntry(formatter), + .@"cell-foreground", .@"cell-background" => try formatter.formatEntry([:0]const u8, @tagName(self)), + } + } + + test "parseCLI" { + const testing = std.testing; + + try testing.expectEqual(DynamicColor{ .color = Color{ .r = 78, .g = 42, .b = 132 } }, try DynamicColor.parseCLI("#4e2a84")); + try testing.expectEqual(DynamicColor{ .color = Color{ .r = 0, .g = 0, .b = 0 } }, try DynamicColor.parseCLI("black")); + try testing.expectEqual(DynamicColor{.@"cell-foreground"}, try DynamicColor.parseCLI("cell-foreground")); + try testing.expectEqual(DynamicColor{.@"cell-background"}, try DynamicColor.parseCLI("cell-background")); + + try testing.expectError(error.InvalidValue, DynamicColor.parseCLI("a")); + } + + test "formatConfig" { + const testing = std.testing; + var buf = std.ArrayList(u8).init(testing.allocator); + defer buf.deinit(); + + var sc: DynamicColor = .{.@"cell-foreground"}; + try sc.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); + try testing.expectEqualSlices(u8, "a = cell-foreground\n", buf.items); + } +}; + pub const ColorList = struct { const Self = @This(); diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 8aaa87011..fda52c375 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -163,8 +163,7 @@ pub const DerivedConfig = struct { image_storage_limit: usize, cursor_style: terminalpkg.CursorStyle, cursor_blink: ?bool, - cursor_color: ?configpkg.Config.Color, - cursor_invert: bool, + cursor_color: ?configpkg.Config.DynamicColor, foreground: configpkg.Config.Color, background: configpkg.Config.Color, osc_color_report_format: configpkg.Config.OSCColorReportFormat, @@ -185,7 +184,6 @@ pub const DerivedConfig = struct { .cursor_style = config.@"cursor-style", .cursor_blink = config.@"cursor-style-blink", .cursor_color = config.@"cursor-color", - .cursor_invert = config.@"cursor-invert-fg-bg", .foreground = config.foreground, .background = config.background, .osc_color_report_format = config.@"osc-color-report-format", @@ -265,8 +263,11 @@ pub fn init(self: *Termio, alloc: Allocator, opts: termio.Options) !void { // Create our stream handler. This points to memory in self so it // isn't safe to use until self.* is set. const handler: StreamHandler = handler: { - const default_cursor_color = if (!opts.config.cursor_invert and opts.config.cursor_color != null) - opts.config.cursor_color.?.toTerminalRGB() + const default_cursor_color = if (opts.config.cursor_color) |color| + switch (color) { + .color => color.color.toTerminalRGB(), + else => null, + } else null; diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 1b4fdd3aa..9946b0b8a 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -121,8 +121,11 @@ pub const StreamHandler = struct { self.default_background_color = config.background.toTerminalRGB(); self.default_cursor_style = config.cursor_style; self.default_cursor_blink = config.cursor_blink; - self.default_cursor_color = if (!config.cursor_invert and config.cursor_color != null) - config.cursor_color.?.toTerminalRGB() + self.default_cursor_color = if (config.cursor_color) |color| + switch (color) { + .color => color.color.toTerminalRGB(), + else => null, + } else null; From 95de1987615bd40f3c8afa0180bc1c0f1c184d7d Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 1 Jul 2025 23:04:58 -0400 Subject: [PATCH 04/12] Squash and rebase, updated refactored version with selection and cursor cell color changes --- src/config/Config.zig | 10 +-- src/renderer/generic.zig | 159 +++++++++++++++++---------------------- 2 files changed, 76 insertions(+), 93 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 76f91f6a8..1fc2f0f71 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -591,7 +591,7 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// the selection color is just the inverted window background and foreground /// (note: not to be confused with the cell bg/fg). /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// Since version 1.2.0, this can also be set to `cell-foreground` to match /// the cell foreground color, or `cell-background` to match the cell /// background color. @"selection-foreground": ?DynamicColor = null, @@ -604,7 +604,7 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// If you select across cells with differing foregrounds and backgrounds, the /// selection color will vary across the selection. /// -/// Warning: This option has been deprecated as of version 1.1.1. Instead, +/// Warning: This option has been deprecated as of version 1.2.0. Instead, /// users should set `selection-foreground` and `selection-background` to /// `cell-background` and `cell-foreground`, respectively. @"selection-invert-fg-bg": bool = false, @@ -652,7 +652,7 @@ palette: Palette = .{}, /// The color of the cursor. If this is not set, a default will be chosen. /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// Since version 1.2.0, this can also be set to `cell-foreground` to match /// the cell foreground color, or `cell-background` to match the cell /// background color. @"cursor-color": ?DynamicColor = null, @@ -660,7 +660,7 @@ palette: Palette = .{}, /// Swap the foreground and background colors of the cell under the cursor. This /// option overrides the `cursor-color` and `cursor-text` options. /// -/// Warning: This option has been deprecated as of version 1.1.1. Instead, +/// Warning: This option has been deprecated as of version 1.2.0. Instead, /// users should set `cursor-color` and `cursor-text` to `cell-foreground` and /// `cell-background`, respectively. @"cursor-invert-fg-bg": bool = false, @@ -713,7 +713,7 @@ palette: Palette = .{}, /// The color of the text under the cursor. If this is not set, a default will /// be chosen. /// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -/// Since version 1.1.1, this can also be set to `cell-foreground` to match +/// Since version 1.2.0, this can also be set to `cell-foreground` to match /// the cell foreground color, or `cell-background` to match the cell /// background color. @"cursor-text": ?DynamicColor = null, diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index 810e17686..617862e1c 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -133,12 +133,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { /// This is cursor color as set in the user's config, if any. If no cursor color /// is set in the user's config, then the cursor color is determined by the /// current foreground color. - default_cursor_color: ?terminal.color.RGB, - - /// When `cursor_color` is null, swap the foreground and background colors of - /// the cell under the cursor for the cursor color. Otherwise, use the default - /// foreground color as the cursor color. - cursor_invert: bool, + default_cursor_color: ?configpkg.Config.DynamicColor, /// The current set of cells to render. This is rebuilt on every frame /// but we keep this around so that we don't reallocate. Each set of @@ -514,16 +509,14 @@ pub fn Renderer(comptime GraphicsAPI: type) type { font_features: std.ArrayListUnmanaged([:0]const u8), font_styles: font.CodepointResolver.StyleStatus, font_shaping_break: configpkg.FontShapingBreak, - cursor_color: ?terminal.color.RGB, - cursor_invert: bool, + cursor_color: ?configpkg.Config.DynamicColor, cursor_opacity: f64, - cursor_text: ?terminal.color.RGB, + cursor_text: ?configpkg.Config.DynamicColor, background: terminal.color.RGB, background_opacity: f64, foreground: terminal.color.RGB, - selection_background: ?terminal.color.RGB, - selection_foreground: ?terminal.color.RGB, - invert_selection_fg_bg: bool, + selection_background: ?configpkg.Config.DynamicColor, + selection_foreground: ?configpkg.Config.DynamicColor, bold_is_bright: bool, min_contrast: f32, padding_color: configpkg.WindowPaddingColor, @@ -571,8 +564,6 @@ pub fn Renderer(comptime GraphicsAPI: type) type { config.link.links.items, ); - const cursor_invert = config.@"cursor-invert-fg-bg"; - return .{ .background_opacity = @max(0, @min(1, config.@"background-opacity")), .font_thicken = config.@"font-thicken", @@ -581,36 +572,18 @@ pub fn Renderer(comptime GraphicsAPI: type) type { .font_styles = font_styles, .font_shaping_break = config.@"font-shaping-break", - .cursor_color = if (!cursor_invert and config.@"cursor-color" != null) - config.@"cursor-color".?.toTerminalRGB() - else - null, - - .cursor_invert = cursor_invert, - - .cursor_text = if (config.@"cursor-text") |txt| - txt.toTerminalRGB() - else - null, - + .cursor_color = config.@"cursor-color", + .cursor_text = config.@"cursor-text", .cursor_opacity = @max(0, @min(1, config.@"cursor-opacity")), .background = config.background.toTerminalRGB(), .foreground = config.foreground.toTerminalRGB(), - .invert_selection_fg_bg = config.@"selection-invert-fg-bg", .bold_is_bright = config.@"bold-is-bright", .min_contrast = @floatCast(config.@"minimum-contrast"), .padding_color = config.@"window-padding-color", - .selection_background = if (config.@"selection-background") |bg| - bg.toTerminalRGB() - else - null, - - .selection_foreground = if (config.@"selection-foreground") |bg| - bg.toTerminalRGB() - else - null, + .selection_background = config.@"selection-background", + .selection_foreground = config.@"selection-foreground", .custom_shaders = custom_shaders, .bg_image = bg_image, @@ -703,7 +676,6 @@ pub fn Renderer(comptime GraphicsAPI: type) type { .default_background_color = options.config.background, .cursor_color = null, .default_cursor_color = options.config.cursor_color, - .cursor_invert = options.config.cursor_invert, // Render state .cells = .{}, @@ -2079,8 +2051,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Set our new colors self.default_background_color = config.background; self.default_foreground_color = config.foreground; - self.default_cursor_color = if (!config.cursor_invert) config.cursor_color else null; - self.cursor_invert = config.cursor_invert; + self.default_cursor_color = config.cursor_color; const bg_image_config_changed = self.config.bg_image_fit != config.bg_image_fit or @@ -2583,22 +2554,15 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // The final background color for the cell. const bg = bg: { if (selected) { - break :bg if (self.config.invert_selection_fg_bg) - if (style.flags.inverse) - // Cell is selected with invert selection fg/bg - // enabled, and the cell has the inverse style - // flag, so they cancel out and we get the normal - // bg color. - bg_style - else - // If it doesn't have the inverse style - // flag then we use the fg color instead. - fg_style + break :bg if (self.config.selection_background) |selection_color| + // Use the selection background if set, otherwise the default fg color. + switch (selection_color) { + .color => selection_color.color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, + } else - // If we don't have invert selection fg/bg set then we - // just use the selection background if set, otherwise - // the default fg color. - break :bg self.config.selection_background orelse self.foreground_color orelse self.default_foreground_color; + self.foreground_color orelse self.default_foreground_color; } // Not selected @@ -2618,20 +2582,26 @@ pub fn Renderer(comptime GraphicsAPI: type) type { }; const fg = fg: { - if (selected and !self.config.invert_selection_fg_bg) { - // If we don't have invert selection fg/bg set - // then we just use the selection foreground if - // set, otherwise the default bg color. - break :fg self.config.selection_foreground orelse self.background_color orelse self.default_background_color; - } + const final_bg = bg_style orelse self.background_color orelse self.default_background_color; // Whether we need to use the bg color as our fg color: + // - Cell is selected, inverted, and set to cell-foreground + // - Cell is selected, not inverted, and set to cell-background // - Cell is inverted and not selected - // - Cell is selected and not inverted - // Note: if selected then invert sel fg / bg must be - // false since we separately handle it if true above. - break :fg if (style.flags.inverse != selected) - bg_style orelse self.background_color orelse self.default_background_color + if (selected) { + // Use the selection foreground if set, otherwise the default bg color. + break :fg if (self.config.selection_foreground) |selection_color| + switch (selection_color) { + .color => selection_color.color.toTerminalRGB(), + .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, + .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, + } + else + self.background_color orelse self.default_background_color; + } + + break :fg if (style.flags.inverse) + final_bg else fg_style; }; @@ -2817,19 +2787,25 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Prepare the cursor cell contents. const style = cursor_style_ orelse break :cursor; - const cursor_color = self.cursor_color orelse self.default_cursor_color orelse color: { - if (self.cursor_invert) { - // Use the foreground color from the cell under the cursor, if any. - const sty = screen.cursor.page_pin.style(screen.cursor.page_cell); - break :color if (sty.flags.inverse) - // If the cell is reversed, use background color instead. - (sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color) - else - (sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color); - } else { - break :color self.foreground_color orelse self.default_foreground_color; + const cursor_color = self.cursor_color orelse if (self.default_cursor_color) |color| color: { + // If cursor-color is set, then compute the correct color. + // Otherwise, use the foreground color + if (color == .color) { + // Use the color set by cursor-color, if any. + break :color color.color.toTerminalRGB(); } - }; + + const sty = screen.cursor.page_pin.style(screen.cursor.page_cell); + const fg_style = sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color; + const bg_style = sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color; + + break :color switch (color) { + // If the cell is reversed, use the opposite cell color instead. + .@"cell-foreground" => if (sty.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (sty.flags.inverse) fg_style else bg_style, + else => unreachable, + }; + } else self.foreground_color orelse self.default_foreground_color; self.addCursor(screen, style, cursor_color); @@ -2853,18 +2829,25 @@ pub fn Renderer(comptime GraphicsAPI: type) type { .wide, .spacer_tail => true, }; - const uniform_color = if (self.cursor_invert) blk: { - // Use the background color from the cell under the cursor, if any. + const uniform_color = if (self.config.cursor_text) |txt| blk: { + // If cursor-text is set, then compute the correct color. + // Otherwise, use the background color. + if (txt == .color) { + // Use the color set by cursor-text, if any. + break :blk txt.color.toTerminalRGB(); + } + const sty = screen.cursor.page_pin.style(screen.cursor.page_cell); - break :blk if (sty.flags.inverse) - // If the cell is reversed, use foreground color instead. - (sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color) - else - (sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color); - } else if (self.config.cursor_text) |txt| - txt - else - self.background_color orelse self.default_background_color; + const fg_style = sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color; + const bg_style = sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color; + + break :blk switch (txt) { + // If the cell is reversed, use the opposite cell color instead. + .@"cell-foreground" => if (sty.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (sty.flags.inverse) fg_style else bg_style, + else => unreachable, + }; + } else self.background_color orelse self.default_background_color; self.uniforms.cursor_color = .{ uniform_color.r, From e87e5e73614aa7ef68bcaaf5814ce0d26b2bb1ea Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2025 07:30:50 -0700 Subject: [PATCH 05/12] backwards compatibility handlers for removed fields --- src/config/Config.zig | 170 +++++++++++++++++++++++++++++++++--------- 1 file changed, 134 insertions(+), 36 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 1fc2f0f71..3cb808179 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -62,6 +62,13 @@ pub const compatibility = std.StaticStringMap( // Ghostty 1.2 removed the `hidden` value from `gtk-tabs-location` and // moved it to `window-show-tab-bar`. .{ "gtk-tabs-location", compatGtkTabsLocation }, + + // Ghostty 1.2 lets you set `cell-foreground` and `cell-background` + // to match the cell foreground and background colors, respectively. + // This can be used with `cursor-color` and `cursor-text` to recreate + // this behavior. This applies to selection too. + .{ "cursor-invert-fg-bg", compatCursorInvertFgBg }, + .{ "selection-invert-fg-bg", compatSelectionInvertFgBg }, }); /// The font families to use. @@ -597,18 +604,6 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, @"selection-foreground": ?DynamicColor = null, @"selection-background": ?DynamicColor = null, -/// Swap the foreground and background colors of cells for selection. This -/// option overrides the `selection-foreground` and `selection-background` -/// options. -/// -/// If you select across cells with differing foregrounds and backgrounds, the -/// selection color will vary across the selection. -/// -/// Warning: This option has been deprecated as of version 1.2.0. Instead, -/// users should set `selection-foreground` and `selection-background` to -/// `cell-background` and `cell-foreground`, respectively. -@"selection-invert-fg-bg": bool = false, - /// Whether to clear selected text when typing. This defaults to `true`. /// This is typical behavior for most terminal emulators as well as /// text input fields. If you set this to `false`, then the selected text @@ -651,19 +646,20 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, palette: Palette = .{}, /// The color of the cursor. If this is not set, a default will be chosen. -/// Specified as either hex (`#RRGGBB` or `RRGGBB`) or a named X11 color. -/// Since version 1.2.0, this can also be set to `cell-foreground` to match -/// the cell foreground color, or `cell-background` to match the cell -/// background color. -@"cursor-color": ?DynamicColor = null, - -/// Swap the foreground and background colors of the cell under the cursor. This -/// option overrides the `cursor-color` and `cursor-text` options. /// -/// Warning: This option has been deprecated as of version 1.2.0. Instead, -/// users should set `cursor-color` and `cursor-text` to `cell-foreground` and -/// `cell-background`, respectively. -@"cursor-invert-fg-bg": bool = false, +/// Direct colors can be specified as either hex (`#RRGGBB` or `RRGGBB`) +/// or a named X11 color. +/// +/// Additionally, special values can be used to set the color to match +/// other colors at runtime: +/// +/// * `cell-foreground` - Match the cell foreground color. +/// (Available since version 1.2.0) +/// +/// * `cell-background` - Match the cell background color. +/// (Available since version 1.2.0) +/// +@"cursor-color": ?DynamicColor = null, /// The opacity level (opposite of transparency) of the cursor. A value of 1 /// is fully opaque and a value of 0 is fully transparent. A value less than 0 @@ -3843,10 +3839,6 @@ pub fn parseManuallyHook( return true; } -/// parseFieldManuallyFallback is a fallback called only when -/// parsing the field directly failed. It can be used to implement -/// backward compatibility. Since this is only called when parsing -/// fails, it doesn't impact happy-path performance. fn compatGtkTabsLocation( self: *Config, alloc: Allocator, @@ -3864,6 +3856,51 @@ fn compatGtkTabsLocation( return false; } +fn compatCursorInvertFgBg( + self: *Config, + alloc: Allocator, + key: []const u8, + value_: ?[]const u8, +) bool { + _ = alloc; + assert(std.mem.eql(u8, key, "cursor-invert-fg-bg")); + + // We don't do anything if the value is unset, which is technically + // not EXACTLY the same as prior behavior since it would fallback + // to doing whatever cursor-color/cursor-text were set to, but + // I don't want to store what that is separately so this is close + // enough. + // + // Realistically, these fields were mutually exclusive so anyone + // relying on that behavior should just upgrade to the new + // cursor-color/cursor-text fields. + const set = cli.args.parseBool(value_ orelse "t") catch return false; + if (set) { + self.@"cursor-color" = .@"cell-foreground"; + self.@"cursor-text" = .@"cell-background"; + } + + return true; +} + +fn compatSelectionInvertFgBg( + self: *Config, + alloc: Allocator, + key: []const u8, + value_: ?[]const u8, +) bool { + _ = alloc; + assert(std.mem.eql(u8, key, "selection-invert-fg-bg")); + + const set = cli.args.parseBool(value_ orelse "t") catch return false; + if (set) { + self.@"selection-foreground" = .@"cell-background"; + self.@"selection-background" = .@"cell-foreground"; + } + + return true; +} + /// Create a shallow copy of this config. This will share all the memory /// allocated with the previous config but will have a new arena for /// any changes or new allocations. The config should have `deinit` @@ -4437,28 +4474,41 @@ pub const DynamicColor = union(enum) { pub fn parseCLI(input_: ?[]const u8) !DynamicColor { const input = input_ orelse return error.ValueRequired; - if (std.mem.eql(u8, input, "cell-foreground")) return .@"cell-foreground"; if (std.mem.eql(u8, input, "cell-background")) return .@"cell-background"; - - return DynamicColor{ .color = try Color.parseCLI(input) }; + return .{ .color = try Color.parseCLI(input) }; } /// Used by Formatter pub fn formatEntry(self: DynamicColor, formatter: anytype) !void { switch (self) { .color => try self.color.formatEntry(formatter), - .@"cell-foreground", .@"cell-background" => try formatter.formatEntry([:0]const u8, @tagName(self)), + + .@"cell-foreground", + .@"cell-background", + => try formatter.formatEntry([:0]const u8, @tagName(self)), } } test "parseCLI" { const testing = std.testing; - try testing.expectEqual(DynamicColor{ .color = Color{ .r = 78, .g = 42, .b = 132 } }, try DynamicColor.parseCLI("#4e2a84")); - try testing.expectEqual(DynamicColor{ .color = Color{ .r = 0, .g = 0, .b = 0 } }, try DynamicColor.parseCLI("black")); - try testing.expectEqual(DynamicColor{.@"cell-foreground"}, try DynamicColor.parseCLI("cell-foreground")); - try testing.expectEqual(DynamicColor{.@"cell-background"}, try DynamicColor.parseCLI("cell-background")); + try testing.expectEqual( + DynamicColor{ .color = Color{ .r = 78, .g = 42, .b = 132 } }, + try DynamicColor.parseCLI("#4e2a84"), + ); + try testing.expectEqual( + DynamicColor{ .color = Color{ .r = 0, .g = 0, .b = 0 } }, + try DynamicColor.parseCLI("black"), + ); + try testing.expectEqual( + DynamicColor{.@"cell-foreground"}, + try DynamicColor.parseCLI("cell-foreground"), + ); + try testing.expectEqual( + DynamicColor{.@"cell-background"}, + try DynamicColor.parseCLI("cell-background"), + ); try testing.expectError(error.InvalidValue, DynamicColor.parseCLI("a")); } @@ -8107,3 +8157,51 @@ test "theme specifying light/dark sets theme usage in conditional state" { try testing.expect(cfg._conditional_set.contains(.theme)); } } + +test "compatibility: removed cursor-invert-fg-bg" { + const testing = std.testing; + const alloc = testing.allocator; + + { + var cfg = try Config.default(alloc); + defer cfg.deinit(); + var it: TestIterator = .{ .data = &.{ + "--cursor-invert-fg-bg", + } }; + try cfg.loadIter(alloc, &it); + try cfg.finalize(); + + try testing.expectEqual( + DynamicColor.@"cell-foreground", + cfg.@"cursor-color", + ); + try testing.expectEqual( + DynamicColor.@"cell-background", + cfg.@"cursor-text", + ); + } +} + +test "compatibility: removed selection-invert-fg-bg" { + const testing = std.testing; + const alloc = testing.allocator; + + { + var cfg = try Config.default(alloc); + defer cfg.deinit(); + var it: TestIterator = .{ .data = &.{ + "--selection-invert-fg-bg", + } }; + try cfg.loadIter(alloc, &it); + try cfg.finalize(); + + try testing.expectEqual( + DynamicColor.@"cell-background", + cfg.@"selection-foreground", + ); + try testing.expectEqual( + DynamicColor.@"cell-foreground", + cfg.@"selection-background", + ); + } +} From 465ac5b1b7ad895951f459cfd4de578b14e0e741 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2025 09:25:55 -0700 Subject: [PATCH 06/12] clean up some of the color usage, use exhaustive switches --- src/config/Config.zig | 48 ++++++++-------- src/renderer/generic.zig | 100 +++++++++++++++++++++------------- src/termio/Termio.zig | 19 ++++--- src/termio/stream_handler.zig | 17 +++--- 4 files changed, 107 insertions(+), 77 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 3cb808179..5d9093bba 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -601,8 +601,8 @@ foreground: Color = .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }, /// Since version 1.2.0, this can also be set to `cell-foreground` to match /// the cell foreground color, or `cell-background` to match the cell /// background color. -@"selection-foreground": ?DynamicColor = null, -@"selection-background": ?DynamicColor = null, +@"selection-foreground": ?TerminalColor = null, +@"selection-background": ?TerminalColor = null, /// Whether to clear selected text when typing. This defaults to `true`. /// This is typical behavior for most terminal emulators as well as @@ -659,7 +659,7 @@ palette: Palette = .{}, /// * `cell-background` - Match the cell background color. /// (Available since version 1.2.0) /// -@"cursor-color": ?DynamicColor = null, +@"cursor-color": ?TerminalColor = null, /// The opacity level (opposite of transparency) of the cursor. A value of 1 /// is fully opaque and a value of 0 is fully transparent. A value less than 0 @@ -712,7 +712,7 @@ palette: Palette = .{}, /// Since version 1.2.0, this can also be set to `cell-foreground` to match /// the cell foreground color, or `cell-background` to match the cell /// background color. -@"cursor-text": ?DynamicColor = null, +@"cursor-text": ?TerminalColor = null, /// Enables the ability to move the cursor at prompts by using `alt+click` on /// Linux and `option+click` on macOS. @@ -4463,16 +4463,14 @@ pub const Color = struct { } }; -/// Represents the color values that can be set to a non-static value. -/// -/// Can either be a Color or one of the special values -/// "cell-foreground" or "cell-background". -pub const DynamicColor = union(enum) { +/// Represents color values that can also reference special color +/// values such as "cell-foreground" or "cell-background". +pub const TerminalColor = union(enum) { color: Color, @"cell-foreground", @"cell-background", - pub fn parseCLI(input_: ?[]const u8) !DynamicColor { + pub fn parseCLI(input_: ?[]const u8) !TerminalColor { const input = input_ orelse return error.ValueRequired; if (std.mem.eql(u8, input, "cell-foreground")) return .@"cell-foreground"; if (std.mem.eql(u8, input, "cell-background")) return .@"cell-background"; @@ -4480,7 +4478,7 @@ pub const DynamicColor = union(enum) { } /// Used by Formatter - pub fn formatEntry(self: DynamicColor, formatter: anytype) !void { + pub fn formatEntry(self: TerminalColor, formatter: anytype) !void { switch (self) { .color => try self.color.formatEntry(formatter), @@ -4494,23 +4492,23 @@ pub const DynamicColor = union(enum) { const testing = std.testing; try testing.expectEqual( - DynamicColor{ .color = Color{ .r = 78, .g = 42, .b = 132 } }, - try DynamicColor.parseCLI("#4e2a84"), + TerminalColor{ .color = Color{ .r = 78, .g = 42, .b = 132 } }, + try TerminalColor.parseCLI("#4e2a84"), ); try testing.expectEqual( - DynamicColor{ .color = Color{ .r = 0, .g = 0, .b = 0 } }, - try DynamicColor.parseCLI("black"), + TerminalColor{ .color = Color{ .r = 0, .g = 0, .b = 0 } }, + try TerminalColor.parseCLI("black"), ); try testing.expectEqual( - DynamicColor{.@"cell-foreground"}, - try DynamicColor.parseCLI("cell-foreground"), + TerminalColor{.@"cell-foreground"}, + try TerminalColor.parseCLI("cell-foreground"), ); try testing.expectEqual( - DynamicColor{.@"cell-background"}, - try DynamicColor.parseCLI("cell-background"), + TerminalColor{.@"cell-background"}, + try TerminalColor.parseCLI("cell-background"), ); - try testing.expectError(error.InvalidValue, DynamicColor.parseCLI("a")); + try testing.expectError(error.InvalidValue, TerminalColor.parseCLI("a")); } test "formatConfig" { @@ -4518,7 +4516,7 @@ pub const DynamicColor = union(enum) { var buf = std.ArrayList(u8).init(testing.allocator); defer buf.deinit(); - var sc: DynamicColor = .{.@"cell-foreground"}; + var sc: TerminalColor = .{.@"cell-foreground"}; try sc.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); try testing.expectEqualSlices(u8, "a = cell-foreground\n", buf.items); } @@ -8172,11 +8170,11 @@ test "compatibility: removed cursor-invert-fg-bg" { try cfg.finalize(); try testing.expectEqual( - DynamicColor.@"cell-foreground", + TerminalColor.@"cell-foreground", cfg.@"cursor-color", ); try testing.expectEqual( - DynamicColor.@"cell-background", + TerminalColor.@"cell-background", cfg.@"cursor-text", ); } @@ -8196,11 +8194,11 @@ test "compatibility: removed selection-invert-fg-bg" { try cfg.finalize(); try testing.expectEqual( - DynamicColor.@"cell-background", + TerminalColor.@"cell-background", cfg.@"selection-foreground", ); try testing.expectEqual( - DynamicColor.@"cell-foreground", + TerminalColor.@"cell-foreground", cfg.@"selection-background", ); } diff --git a/src/renderer/generic.zig b/src/renderer/generic.zig index 617862e1c..e7faf633f 100644 --- a/src/renderer/generic.zig +++ b/src/renderer/generic.zig @@ -133,7 +133,7 @@ pub fn Renderer(comptime GraphicsAPI: type) type { /// This is cursor color as set in the user's config, if any. If no cursor color /// is set in the user's config, then the cursor color is determined by the /// current foreground color. - default_cursor_color: ?configpkg.Config.DynamicColor, + default_cursor_color: ?configpkg.Config.TerminalColor, /// The current set of cells to render. This is rebuilt on every frame /// but we keep this around so that we don't reallocate. Each set of @@ -509,14 +509,14 @@ pub fn Renderer(comptime GraphicsAPI: type) type { font_features: std.ArrayListUnmanaged([:0]const u8), font_styles: font.CodepointResolver.StyleStatus, font_shaping_break: configpkg.FontShapingBreak, - cursor_color: ?configpkg.Config.DynamicColor, + cursor_color: ?configpkg.Config.TerminalColor, cursor_opacity: f64, - cursor_text: ?configpkg.Config.DynamicColor, + cursor_text: ?configpkg.Config.TerminalColor, background: terminal.color.RGB, background_opacity: f64, foreground: terminal.color.RGB, - selection_background: ?configpkg.Config.DynamicColor, - selection_foreground: ?configpkg.Config.DynamicColor, + selection_background: ?configpkg.Config.TerminalColor, + selection_foreground: ?configpkg.Config.TerminalColor, bold_is_bright: bool, min_contrast: f32, padding_color: configpkg.WindowPaddingColor, @@ -2548,21 +2548,31 @@ pub fn Renderer(comptime GraphicsAPI: type) type { else false; + // The `_style` suffixed values are the colors based on + // the cell style (SGR), before applying any additional + // configuration, inversions, selections, etc. const bg_style = style.bg(cell, color_palette); - const fg_style = style.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color; + const fg_style = style.fg( + color_palette, + self.config.bold_is_bright, + ) orelse self.foreground_color orelse self.default_foreground_color; // The final background color for the cell. const bg = bg: { if (selected) { - break :bg if (self.config.selection_background) |selection_color| - // Use the selection background if set, otherwise the default fg color. - switch (selection_color) { - .color => selection_color.color.toTerminalRGB(), + // If we have an explicit selection background color + // specified int he config, use that + if (self.config.selection_background) |v| { + break :bg switch (v) { + .color => |color| color.toTerminalRGB(), .@"cell-foreground" => if (style.flags.inverse) bg_style else fg_style, .@"cell-background" => if (style.flags.inverse) fg_style else bg_style, - } - else - self.foreground_color orelse self.default_foreground_color; + }; + } + + // If no configuration, then our selection background + // is our foreground color. + break :bg self.foreground_color orelse self.default_foreground_color; } // Not selected @@ -2582,22 +2592,27 @@ pub fn Renderer(comptime GraphicsAPI: type) type { }; const fg = fg: { - const final_bg = bg_style orelse self.background_color orelse self.default_background_color; + // Our happy-path non-selection background color + // is our style or our configured defaults. + const final_bg = bg_style orelse + self.background_color orelse + self.default_background_color; // Whether we need to use the bg color as our fg color: // - Cell is selected, inverted, and set to cell-foreground // - Cell is selected, not inverted, and set to cell-background // - Cell is inverted and not selected if (selected) { - // Use the selection foreground if set, otherwise the default bg color. - break :fg if (self.config.selection_foreground) |selection_color| - switch (selection_color) { - .color => selection_color.color.toTerminalRGB(), + // Use the selection foreground if set + if (self.config.selection_foreground) |v| { + break :fg switch (v) { + .color => |color| color.toTerminalRGB(), .@"cell-foreground" => if (style.flags.inverse) final_bg else fg_style, .@"cell-background" => if (style.flags.inverse) fg_style else final_bg, - } - else - self.background_color orelse self.default_background_color; + }; + } + + break :fg self.background_color orelse self.default_background_color; } break :fg if (style.flags.inverse) @@ -2787,25 +2802,36 @@ pub fn Renderer(comptime GraphicsAPI: type) type { // Prepare the cursor cell contents. const style = cursor_style_ orelse break :cursor; - const cursor_color = self.cursor_color orelse if (self.default_cursor_color) |color| color: { - // If cursor-color is set, then compute the correct color. - // Otherwise, use the foreground color - if (color == .color) { - // Use the color set by cursor-color, if any. - break :color color.color.toTerminalRGB(); - } + const cursor_color = cursor_color: { + // If an explicit cursor color was set by OSC 12, use that. + if (self.cursor_color) |v| break :cursor_color v; - const sty = screen.cursor.page_pin.style(screen.cursor.page_cell); - const fg_style = sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color; - const bg_style = sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color; + // Use our configured color if specified + if (self.default_cursor_color) |v| switch (v) { + .color => |color| break :cursor_color color.toTerminalRGB(), + inline .@"cell-foreground", + .@"cell-background", + => |_, tag| { + const sty = screen.cursor.page_pin.style(screen.cursor.page_cell); + const fg_style = sty.fg( + color_palette, + self.config.bold_is_bright, + ) orelse self.foreground_color orelse self.default_foreground_color; + const bg_style = sty.bg( + screen.cursor.page_cell, + color_palette, + ) orelse self.background_color orelse self.default_background_color; - break :color switch (color) { - // If the cell is reversed, use the opposite cell color instead. - .@"cell-foreground" => if (sty.flags.inverse) bg_style else fg_style, - .@"cell-background" => if (sty.flags.inverse) fg_style else bg_style, - else => unreachable, + break :cursor_color switch (tag) { + .color => unreachable, + .@"cell-foreground" => if (sty.flags.inverse) bg_style else fg_style, + .@"cell-background" => if (sty.flags.inverse) fg_style else bg_style, + }; + }, }; - } else self.foreground_color orelse self.default_foreground_color; + + break :cursor_color self.foreground_color orelse self.default_foreground_color; + }; self.addCursor(screen, style, cursor_color); diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index fda52c375..4b5b93641 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -163,7 +163,7 @@ pub const DerivedConfig = struct { image_storage_limit: usize, cursor_style: terminalpkg.CursorStyle, cursor_blink: ?bool, - cursor_color: ?configpkg.Config.DynamicColor, + cursor_color: ?configpkg.Config.TerminalColor, foreground: configpkg.Config.Color, background: configpkg.Config.Color, osc_color_report_format: configpkg.Config.OSCColorReportFormat, @@ -263,13 +263,16 @@ pub fn init(self: *Termio, alloc: Allocator, opts: termio.Options) !void { // Create our stream handler. This points to memory in self so it // isn't safe to use until self.* is set. const handler: StreamHandler = handler: { - const default_cursor_color = if (opts.config.cursor_color) |color| - switch (color) { - .color => color.color.toTerminalRGB(), - else => null, - } - else - null; + const default_cursor_color: ?terminalpkg.color.RGB = color: { + if (opts.config.cursor_color) |color| switch (color) { + .color => break :color color.color.toTerminalRGB(), + .@"cell-foreground", + .@"cell-background", + => {}, + }; + + break :color null; + }; break :handler .{ .alloc = alloc, diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 9946b0b8a..040132f03 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -121,13 +121,16 @@ pub const StreamHandler = struct { self.default_background_color = config.background.toTerminalRGB(); self.default_cursor_style = config.cursor_style; self.default_cursor_blink = config.cursor_blink; - self.default_cursor_color = if (config.cursor_color) |color| - switch (color) { - .color => color.color.toTerminalRGB(), - else => null, - } - else - null; + self.default_cursor_color = color: { + if (config.cursor_color) |color| switch (color) { + .color => break :color color.color.toTerminalRGB(), + .@"cell-foreground", + .@"cell-background", + => {}, + }; + + break :color null; + }; // If our cursor is the default, then we update it immediately. if (self.default_cursor) self.setCursorStyle(.default) catch |err| { From 32764f3a1d8aab3043c11170e2b4c691c3316ca4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2025 09:29:36 -0700 Subject: [PATCH 07/12] fix test syntax --- src/config/Config.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 5d9093bba..e140785bb 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -4500,11 +4500,11 @@ pub const TerminalColor = union(enum) { try TerminalColor.parseCLI("black"), ); try testing.expectEqual( - TerminalColor{.@"cell-foreground"}, + TerminalColor.@"cell-foreground", try TerminalColor.parseCLI("cell-foreground"), ); try testing.expectEqual( - TerminalColor{.@"cell-background"}, + TerminalColor.@"cell-background", try TerminalColor.parseCLI("cell-background"), ); @@ -4516,7 +4516,7 @@ pub const TerminalColor = union(enum) { var buf = std.ArrayList(u8).init(testing.allocator); defer buf.deinit(); - var sc: TerminalColor = .{.@"cell-foreground"}; + var sc: TerminalColor = .@"cell-foreground"; try sc.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); try testing.expectEqualSlices(u8, "a = cell-foreground\n", buf.items); } From e1be836283e0824f7f37dc9d94404ec4723c9050 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2025 13:45:38 -0700 Subject: [PATCH 08/12] config: for now, make goto_tab NOT performable on macOS Fixes #7786 Fixes regression from #7683 This is a band-aid fix. The issue is that performable keybinds don't show up in the reverse mapping that GUI toolkits use to find their key equivalents. The full explanation of why is already in Binding.zig. For macOS, we have a way to validate menu items before they're triggered so we ideally do want a way to get reverse mappings even with performable keybinds. But I think this wants to be optional and that's all a bigger change. For now, this is a simple fix that will work. --- src/config/Config.zig | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index e140785bb..fec5b41fc 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -5455,7 +5455,14 @@ pub const Keybinds = struct { .mods = mods, }, .{ .goto_tab = (i - start) + 1 }, - .{ .performable = true }, + .{ + // On macOS we keep this not performable so that the + // keyboard shortcuts in tabs work. In the future the + // correct fix is to fix the reverse mapping lookup + // to allow us to lookup performable keybinds + // conditionally. + .performable = !builtin.target.os.tag.isDarwin(), + }, ); } try self.set.putFlags( @@ -5465,7 +5472,10 @@ pub const Keybinds = struct { .mods = mods, }, .{ .last_tab = {} }, - .{ .performable = true }, + .{ + // See comment above with the numeric goto_tab + .performable = !builtin.target.os.tag.isDarwin(), + }, ); } From e494d94fb326c043e062ab5f60704e891f927371 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 3 Jul 2025 21:14:14 -0700 Subject: [PATCH 09/12] Handle `exec` failures more gracefully Fixes #7792 Our error handling for `exec` failing within the forked process never actually worked! It triggered all sorts of issues. We didn't catch this before because it used to be exceptionally hard to fail an exec because we used to wrap ALL commands in a `/bin/sh -c`. However, we now support direction execution, most notably when you do `ghostty -e ` but also via the `direct:` prefix on configured commands. This fixes up our exec failure handling by printing a useful error message and avoiding any errdefers in the child which was causing the double-close. --- src/Command.zig | 27 +++++++++++++++++--- src/termio/Exec.zig | 60 ++++++++++++++++++++++----------------------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/Command.zig b/src/Command.zig index 7ed026efe..1bddf8b82 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -188,10 +188,31 @@ fn startPosix(self: *Command, arena: Allocator) !void { // Finally, replace our process. // Note: we must use the "p"-variant of exec here because we // do not guarantee our command is looked up already in the path. - _ = posix.execvpeZ(self.path, argsZ, envp) catch null; + const err = posix.execvpeZ(self.path, argsZ, envp); - // If we are executing this code, the exec failed. In that scenario, - // we return a very specific error that can be detected to determine + // If we are executing this code, the exec failed. We're in the + // child process so there isn't much we can do. We try to output + // something reasonable. Its important to note we MUST NOT return + // any other error condition from here on out. + const stderr = std.io.getStdErr().writer(); + switch (err) { + error.FileNotFound => stderr.print( + \\Requested executable not found. Please verify the command is on + \\the PATH and try again. + \\ + , + .{}, + ) catch {}, + + else => stderr.print( + \\exec syscall failed with unexpected error: {} + \\ + , + .{err}, + ) catch {}, + } + + // We return a very specific error that can be detected to determine // we're in the child. return error.ExecFailedInChild; } diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 598617a12..15b6b8cd4 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -90,15 +90,13 @@ pub fn threadEnter( // Start our subprocess const pty_fds = self.subprocess.start(alloc) catch |err| { // If we specifically got this error then we are in the forked - // process and our child failed to execute. In that case - if (err != error.Termio) return err; + // process and our child failed to execute. If we DIDN'T + // get this specific error then we're in the parent and + // we need to bubble it up. + if (err != error.ExecFailedInChild) return err; - // Output an error message about the exec faililng and exit. - // This generally should NOT happen because we always wrap - // our command execution either in login (macOS) or /bin/sh - // (Linux) which are usually guaranteed to exist. Still, we - // want to handle this scenario. - execFailedInChild() catch {}; + // We're in the child. Nothing more we can do but abnormal exit. + // The Command will output some additional information. posix.exit(1); }; errdefer self.subprocess.stop(); @@ -272,25 +270,6 @@ pub fn resize( return try self.subprocess.resize(grid_size, screen_size); } -/// This outputs an error message when exec failed and we are the -/// child process. This returns so the caller should probably exit -/// after calling this. -/// -/// Note that this usually is only called under very very rare -/// circumstances because we wrap our command execution in login -/// (macOS) or /bin/sh (Linux). So this output can be pretty crude -/// because it should never happen. Notably, this is not the error -/// users see when `command` is invalid. -fn execFailedInChild() !void { - const stderr = std.io.getStdErr().writer(); - try stderr.writeAll("exec failed\n"); - try stderr.writeAll("press any key to exit\n"); - - var buf: [1]u8 = undefined; - var reader = std.io.getStdIn().reader(); - _ = try reader.read(&buf); -} - fn processExitCommon(td: *termio.Termio.ThreadData, exit_code: u32) void { assert(td.backend == .exec); const execdata = &td.backend.exec; @@ -895,6 +874,12 @@ const Subprocess = struct { } { assert(self.pty == null and self.command == null); + // This function is funny because on POSIX systems it can + // fail in the forked process. This is flipped to true if + // we're in an error state in the forked process (child + // process). + var in_child: bool = false; + // Create our pty var pty = try Pty.open(.{ .ws_row = @intCast(self.grid_size.rows), @@ -903,14 +888,14 @@ const Subprocess = struct { .ws_ypixel = @intCast(self.screen_size.height), }); self.pty = pty; - errdefer { + errdefer if (!in_child) { if (comptime builtin.os.tag != .windows) { _ = posix.close(pty.slave); } pty.deinit(); self.pty = null; - } + }; log.debug("starting command command={s}", .{self.args}); @@ -1013,7 +998,22 @@ const Subprocess = struct { .data = self, .linux_cgroup = self.linux_cgroup, }; - try cmd.start(alloc); + + cmd.start(alloc) catch |err| { + // We have to do this because start on Windows can't + // ever return ExecFailedInChild + const StartError = error{ExecFailedInChild} || @TypeOf(err); + switch (@as(StartError, err)) { + // If we fail in our child we need to flag it so our + // errdefers don't run. + error.ExecFailedInChild => { + in_child = true; + return err; + }, + + else => return err, + } + }; errdefer killCommand(&cmd) catch |err| { log.warn("error killing command during cleanup err={}", .{err}); }; From 95d9b1e627abed0543806cc6f1abad3e1d60384b Mon Sep 17 00:00:00 2001 From: Kat <65649991+00-kat@users.noreply.github.com> Date: Fri, 4 Jul 2025 07:24:55 +0000 Subject: [PATCH 10/12] Request translators to update the CODEOWNERS file. --- po/README_TRANSLATORS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/po/README_TRANSLATORS.md b/po/README_TRANSLATORS.md index ca1e45faa..c02a5bd48 100644 --- a/po/README_TRANSLATORS.md +++ b/po/README_TRANSLATORS.md @@ -148,6 +148,18 @@ const locales = [_][]const u8{ You should then be able to run `zig build` and see your translations in action. +Before opening a pull request with the new translation file, you should also add +your locale to the `CODEOWNERS` file. Find the `# Localization` section near the +bottom and add a line like so (where `xx_YY` is your locale): + +```diff + # Localization + /po/README_TRANSLATORS.md @ghostty-org/localization + /po/com.mitchellh.ghostty.pot @ghostty-org/localization + /po/zh_CN.UTF-8.po @ghostty-org/zh_CN ++/po/xx_YY.UTF-8.po @ghostty-org/xx_YY +``` + ## Style Guide These are general style guidelines for translations. Naturally, the specific From d8838cff0b1b1d9170568fa3fc99c69cc1f050a3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 4 Jul 2025 07:30:24 -0700 Subject: [PATCH 11/12] macOS: remove @DeferredProperty usage from TerminalEntity This fixes an Apple Shortcuts crash for macOS 15 and earlier. Unfortunately it looks like we can't guard these with `@available`. I'm going to report an Apple Feedback about this but for now this gets shortcuts working on macOS 15 and earlier. --- .../App Intents/Entities/TerminalEntity.swift | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/macos/Sources/Features/App Intents/Entities/TerminalEntity.swift b/macos/Sources/Features/App Intents/Entities/TerminalEntity.swift index e29fbba3f..cc3b9f63a 100644 --- a/macos/Sources/Features/App Intents/Entities/TerminalEntity.swift +++ b/macos/Sources/Features/App Intents/Entities/TerminalEntity.swift @@ -14,26 +14,6 @@ struct TerminalEntity: AppEntity { @Property(title: "Kind") var kind: Kind - @MainActor - @DeferredProperty(title: "Full Contents") - @available(macOS 26.0, *) - var screenContents: String? { - get async { - guard let surfaceView else { return nil } - return surfaceView.cachedScreenContents.get() - } - } - - @MainActor - @DeferredProperty(title: "Visible Contents") - @available(macOS 26.0, *) - var visibleContents: String? { - get async { - guard let surfaceView else { return nil } - return surfaceView.cachedVisibleContents.get() - } - } - var screenshot: Image? static var typeDisplayRepresentation: TypeDisplayRepresentation { From eea073c97bd9c4de15a7179c1ad3bb252652fbfa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 4 Jul 2025 07:40:59 -0700 Subject: [PATCH 12/12] On wait-after-command (or abnormal exit), only close on encoded key Fixes #7794 This commit also resets some terminal state to give us a better chance of getting an encoded key, such as ensuring keyboard input is enabled and disabling any Kitty protocols. This shouldn't ever be set but just in case! --- src/Surface.zig | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index dc7b0e3bf..6d0f1584b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1034,6 +1034,12 @@ fn childExited(self: *Surface, info: apprt.surface.Message.ChildExited) void { t.printString("Process exited. Press any key to close the terminal.") catch break :terminal; t.modes.set(.cursor_visible, false); + + // We also want to ensure that normal keyboard encoding is on + // so that we can close the terminal. We close the terminal on + // any key press that encodes a character. + t.modes.set(.disable_keyboard, false); + t.screen.kitty_keyboard.set(.set, .{}); } // Waiting after command we stop here. The terminal is updated, our @@ -2129,14 +2135,6 @@ pub fn keyCallback( if (self.io.terminal.modes.get(.disable_keyboard)) return .consumed; } - // If our process is exited and we press a key then we close the - // surface. We may want to eventually move this to the apprt rather - // than in core. - if (self.child_exited and event.action == .press) { - self.close(); - return .closed; - } - // If this input event has text, then we hide the mouse if configured. // We only do this on pressed events to avoid hiding the mouse when we // change focus due to a keybinding (i.e. switching tabs). @@ -2231,6 +2229,14 @@ pub fn keyCallback( event, if (insp_ev) |*ev| ev else null, )) |write_req| { + // If our process is exited and we press a key that results in + // an encoded value, we close the surface. We want to eventually + // move this behavior to the apprt probably. + if (self.child_exited) { + self.close(); + return .closed; + } + errdefer write_req.deinit(); self.io.queueMessage(switch (write_req) { .small => |v| .{ .write_small = v },