From 6a2597a6d63b7bc823e8edb9d1fbdc5634876c81 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 5 Dec 2024 16:41:15 -0500 Subject: [PATCH 01/48] =?UTF-8?q?macos:=20Make=20"Settings=E2=80=A6"=20men?= =?UTF-8?q?u=20item=20open=20config=20file=20in=20Application=20Support?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ...unless ~/.config/ghostty/config already exists, then that is opened. (Or whatever $XDG_CONFIG_HOME points to.) If both files exists, ghostty reads first the one in ~/.config/ghostty/config and then the one in Application Support, and merges the settings. In that case, the menu item opens the file at ~/.config. Fixes #2890. --- src/config/edit.zig | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/config/edit.zig b/src/config/edit.zig index 38d9f2b7f..e93d2a6f8 100644 --- a/src/config/edit.zig +++ b/src/config/edit.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const builtin = @import("builtin"); const Allocator = std.mem.Allocator; const internal_os = @import("../os/main.zig"); @@ -6,7 +7,21 @@ const internal_os = @import("../os/main.zig"); /// paths the main config file could be in. pub fn open(alloc_gpa: Allocator) !void { // default location - const config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" }); + var config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" }); + + if (comptime builtin.os.tag == .macos) { + const xdg_config_exists = if (std.fs.accessAbsolute(config_path, std.fs.File.OpenFlags{})) true else |err| switch (err) { + error.BadPathName => false, + error.FileNotFound => false, + else => true, + }; + + if (!xdg_config_exists) { + alloc_gpa.free(config_path); + config_path = try internal_os.macos.appSupportDir(alloc_gpa, "config"); + } + } + defer alloc_gpa.free(config_path); // Create config directory recursively. From 79d84af56ee32d3f342a10e183d690923a115f1a Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 22 Dec 2024 08:52:38 -0500 Subject: [PATCH 02/48] comment --- src/config/edit.zig | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/config/edit.zig b/src/config/edit.zig index e93d2a6f8..ede13ee9d 100644 --- a/src/config/edit.zig +++ b/src/config/edit.zig @@ -7,21 +7,23 @@ const internal_os = @import("../os/main.zig"); /// paths the main config file could be in. pub fn open(alloc_gpa: Allocator) !void { // default location - var config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" }); + const config_path = config_path: { + const xdg_config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" }); - if (comptime builtin.os.tag == .macos) { - const xdg_config_exists = if (std.fs.accessAbsolute(config_path, std.fs.File.OpenFlags{})) true else |err| switch (err) { - error.BadPathName => false, - error.FileNotFound => false, - else => true, - }; + if (comptime builtin.os.tag == .macos) macos: { + if (std.fs.accessAbsolute(xdg_config_path, .{})) { + break :macos; + } else |err| switch (err) { + error.BadPathName, error.FileNotFound => {}, + else => break :macos, + } - if (!xdg_config_exists) { - alloc_gpa.free(config_path); - config_path = try internal_os.macos.appSupportDir(alloc_gpa, "config"); + alloc_gpa.free(xdg_config_path); + break :config_path try internal_os.macos.appSupportDir(alloc_gpa, "config"); } - } + break :config_path xdg_config_path; + }; defer alloc_gpa.free(config_path); // Create config directory recursively. From 44e1df5df3ae712986335dac20fd62cb4123e09f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 22 Dec 2024 13:16:19 -0500 Subject: [PATCH 03/48] add a comment --- src/config/edit.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config/edit.zig b/src/config/edit.zig index ede13ee9d..692447594 100644 --- a/src/config/edit.zig +++ b/src/config/edit.zig @@ -11,6 +11,7 @@ pub fn open(alloc_gpa: Allocator) !void { const xdg_config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" }); if (comptime builtin.os.tag == .macos) macos: { + // On macOS, use the application support path if the XDG path doesn't exists. if (std.fs.accessAbsolute(xdg_config_path, .{})) { break :macos; } else |err| switch (err) { From 98651ab0e551519bd6fa3c3ad354e3dd7d7e0bb3 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 04:18:48 -0500 Subject: [PATCH 04/48] fmt --- src/font/sprite/Powerline.zig | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/font/sprite/Powerline.zig b/src/font/sprite/Powerline.zig index 904eae957..eaa7554b1 100644 --- a/src/font/sprite/Powerline.zig +++ b/src/font/sprite/Powerline.zig @@ -115,7 +115,7 @@ fn draw(self: Powerline, alloc: Allocator, canvas: *font.sprite.Canvas, cp: u32) fn draw_chevron(self: Powerline, canvas: *font.sprite.Canvas, cp: u32) !void { const width = self.width; const height = self.height; - + var p1_x: u32 = 0; var p1_y: u32 = 0; var p2_x: u32 = 0; @@ -123,7 +123,6 @@ fn draw_chevron(self: Powerline, canvas: *font.sprite.Canvas, cp: u32) !void { var p3_x: u32 = 0; var p3_y: u32 = 0; - switch (cp) { 0xE0B1 => { p1_x = 0; @@ -141,19 +140,15 @@ fn draw_chevron(self: Powerline, canvas: *font.sprite.Canvas, cp: u32) !void { p3_x = width; p3_y = height; }, - - else => unreachable, + else => unreachable, } try canvas.triangle_outline(.{ .p0 = .{ .x = @floatFromInt(p1_x), .y = @floatFromInt(p1_y) }, .p1 = .{ .x = @floatFromInt(p2_x), .y = @floatFromInt(p2_y) }, .p2 = .{ .x = @floatFromInt(p3_x), .y = @floatFromInt(p3_y) }, - }, - @floatFromInt(Thickness.light.height(self.thickness)), - .on); - + }, @floatFromInt(Thickness.light.height(self.thickness)), .on); } fn draw_wedge_triangle(self: Powerline, canvas: *font.sprite.Canvas, cp: u32) !void { From 9f4d9dc36e4bebfe805d3e28c9e9622c7fe83b97 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 04:21:33 -0500 Subject: [PATCH 05/48] font/sprite: fix z2d StaticPath accounting + undefined use Annotate the node count of all uses of z2d `StaticPath` to verify correctness, adjusted the size of a couple which were oversized, and changed all painter calls that take node slices from `StaticPath`s to use the slice from the wrapped `ArrayList` so that we don't include any potentially `undefined` nodes at the end of the list, which I think was causing a crash before. --- src/font/sprite/Box.zig | 38 +++++++++++++++--------------- src/font/sprite/canvas.zig | 48 +++++++++++++++++++------------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/font/sprite/Box.zig b/src/font/sprite/Box.zig index f64665622..ba7caa26a 100644 --- a/src/font/sprite/Box.zig +++ b/src/font/sprite/Box.zig @@ -2515,19 +2515,19 @@ fn draw_smooth_mosaic( const right: f64 = @floatFromInt(self.metrics.cell_width); var path: z2d.StaticPath(12) = .{}; - path.init(); + path.init(); // nodes.len = 0 - if (mosaic.tl) path.lineTo(left, top); - if (mosaic.ul) path.lineTo(left, upper); - if (mosaic.ll) path.lineTo(left, lower); - if (mosaic.bl) path.lineTo(left, bottom); - if (mosaic.bc) path.lineTo(center, bottom); - if (mosaic.br) path.lineTo(right, bottom); - if (mosaic.lr) path.lineTo(right, lower); - if (mosaic.ur) path.lineTo(right, upper); - if (mosaic.tr) path.lineTo(right, top); - if (mosaic.tc) path.lineTo(center, top); - path.close(); + if (mosaic.tl) path.lineTo(left, top); // +1, nodes.len = 1 + if (mosaic.ul) path.lineTo(left, upper); // +1, nodes.len = 2 + if (mosaic.ll) path.lineTo(left, lower); // +1, nodes.len = 3 + if (mosaic.bl) path.lineTo(left, bottom); // +1, nodes.len = 4 + if (mosaic.bc) path.lineTo(center, bottom); // +1, nodes.len = 5 + if (mosaic.br) path.lineTo(right, bottom); // +1, nodes.len = 6 + if (mosaic.lr) path.lineTo(right, lower); // +1, nodes.len = 7 + if (mosaic.ur) path.lineTo(right, upper); // +1, nodes.len = 8 + if (mosaic.tr) path.lineTo(right, top); // +1, nodes.len = 9 + if (mosaic.tc) path.lineTo(center, top); // +1, nodes.len = 10 + path.close(); // +2, nodes.len = 12 try z2d.painter.fill( canvas.alloc, @@ -2535,7 +2535,7 @@ fn draw_smooth_mosaic( &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(Shade.on) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{}, ); } @@ -2560,12 +2560,12 @@ fn draw_edge_triangle( }; var path: z2d.StaticPath(5) = .{}; - path.init(); + path.init(); // nodes.len = 0 - path.moveTo(center, middle); - path.lineTo(x0, y0); - path.lineTo(x1, y1); - path.close(); + path.moveTo(center, middle); // +1, nodes.len = 1 + path.lineTo(x0, y0); // +1, nodes.len = 2 + path.lineTo(x1, y1); // +1, nodes.len = 3 + path.close(); // +2, nodes.len = 5 try z2d.painter.fill( canvas.alloc, @@ -2573,7 +2573,7 @@ fn draw_edge_triangle( &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(Shade.on) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{}, ); } diff --git a/src/font/sprite/canvas.zig b/src/font/sprite/canvas.zig index be7bdf8cc..072e5bd46 100644 --- a/src/font/sprite/canvas.zig +++ b/src/font/sprite/canvas.zig @@ -184,13 +184,13 @@ pub const Canvas = struct { /// Draw and fill a quad. pub fn quad(self: *Canvas, q: Quad(f64), color: Color) !void { var path: z2d.StaticPath(6) = .{}; - path.init(); + path.init(); // nodes.len = 0 - path.moveTo(q.p0.x, q.p0.y); - path.lineTo(q.p1.x, q.p1.y); - path.lineTo(q.p2.x, q.p2.y); - path.lineTo(q.p3.x, q.p3.y); - path.close(); + path.moveTo(q.p0.x, q.p0.y); // +1, nodes.len = 1 + path.lineTo(q.p1.x, q.p1.y); // +1, nodes.len = 2 + path.lineTo(q.p2.x, q.p2.y); // +1, nodes.len = 3 + path.lineTo(q.p3.x, q.p3.y); // +1, nodes.len = 4 + path.close(); // +2, nodes.len = 6 try z2d.painter.fill( self.alloc, @@ -198,7 +198,7 @@ pub const Canvas = struct { &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(color) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{}, ); } @@ -206,12 +206,12 @@ pub const Canvas = struct { /// Draw and fill a triangle. pub fn triangle(self: *Canvas, t: Triangle(f64), color: Color) !void { var path: z2d.StaticPath(5) = .{}; - path.init(); + path.init(); // nodes.len = 0 - path.moveTo(t.p0.x, t.p0.y); - path.lineTo(t.p1.x, t.p1.y); - path.lineTo(t.p2.x, t.p2.y); - path.close(); + path.moveTo(t.p0.x, t.p0.y); // +1, nodes.len = 1 + path.lineTo(t.p1.x, t.p1.y); // +1, nodes.len = 2 + path.lineTo(t.p2.x, t.p2.y); // +1, nodes.len = 3 + path.close(); // +2, nodes.len = 5 try z2d.painter.fill( self.alloc, @@ -219,18 +219,18 @@ pub const Canvas = struct { &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(color) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{}, ); } pub fn triangle_outline(self: *Canvas, t: Triangle(f64), thickness: f64, color: Color) !void { - var path: z2d.StaticPath(5) = .{}; - path.init(); + var path: z2d.StaticPath(3) = .{}; + path.init(); // nodes.len = 0 - path.moveTo(t.p0.x, t.p0.y); - path.lineTo(t.p1.x, t.p1.y); - path.lineTo(t.p2.x, t.p2.y); + path.moveTo(t.p0.x, t.p0.y); // +1, nodes.len = 1 + path.lineTo(t.p1.x, t.p1.y); // +1, nodes.len = 2 + path.lineTo(t.p2.x, t.p2.y); // +1, nodes.len = 3 try z2d.painter.stroke( self.alloc, @@ -238,7 +238,7 @@ pub const Canvas = struct { &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(color) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{ .line_cap_mode = .round, .line_width = thickness, @@ -248,11 +248,11 @@ pub const Canvas = struct { /// Stroke a line. pub fn line(self: *Canvas, l: Line(f64), thickness: f64, color: Color) !void { - var path: z2d.StaticPath(3) = .{}; - path.init(); + var path: z2d.StaticPath(2) = .{}; + path.init(); // nodes.len = 0 - path.moveTo(l.p0.x, l.p0.y); - path.lineTo(l.p1.x, l.p1.y); + path.moveTo(l.p0.x, l.p0.y); // +1, nodes.len = 1 + path.lineTo(l.p1.x, l.p1.y); // +1, nodes.len = 2 try z2d.painter.stroke( self.alloc, @@ -260,7 +260,7 @@ pub const Canvas = struct { &.{ .opaque_pattern = .{ .pixel = .{ .alpha8 = .{ .a = @intFromEnum(color) } }, } }, - &path.nodes, + path.wrapped_path.nodes.items, .{ .line_cap_mode = .round, .line_width = thickness, From 19e865124781fb25406a37f43a21b2028c294c67 Mon Sep 17 00:00:00 2001 From: Iain H Date: Mon, 23 Dec 2024 11:48:22 -0500 Subject: [PATCH 06/48] apprt/gtk: move new tab button to start of header bar According to the GNOME human interface guidelines, buttons for the main user actions, such as new, add, open, and back should be placed at the start of the header bar. (https://developer.gnome.org/hig/patterns/containers/header-bars.html#header-bar-buttons) Moving the new tab button to the start of the header bar brings Ghostty in line with other GNOME applications such as gedit and gnome-terminal. --- src/apprt/gtk/Window.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apprt/gtk/Window.zig b/src/apprt/gtk/Window.zig index 0f6a14c8c..48e88e491 100644 --- a/src/apprt/gtk/Window.zig +++ b/src/apprt/gtk/Window.zig @@ -200,7 +200,7 @@ pub fn init(self: *Window, app: *App) !void { const btn = c.gtk_button_new_from_icon_name("tab-new-symbolic"); c.gtk_widget_set_tooltip_text(btn, "New Tab"); _ = c.g_signal_connect_data(btn, "clicked", c.G_CALLBACK(>kTabNewClick), self, null, c.G_CONNECT_DEFAULT); - header.packEnd(btn); + header.packStart(btn); } self.header = header; From a51871a3f776c49b52dd0076e905a8fac9608149 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 15:43:57 -0500 Subject: [PATCH 07/48] RefCountedSet: simplify `insert` logic, cleanup, improve comments Previous logic had multiple issues that were hiding in edge cases of edge cases with the ressurected item handling among other things; the added assertIntegrity method finds these issues, the primary one being an edge case where an ID is present in two different buckets. Added more comments to explain logic in more detail and fixed a couple little things like always using `+%` when incrementing the probe pos, and replacing a silent return on an integrity issue that should be impossible (`table[item.meta.bucket] != id`) with an assert. --- src/terminal/ref_counted_set.zig | 100 +++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 26 deletions(-) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index 09ab3a1e6..eecfae4b5 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -419,7 +419,7 @@ pub fn RefCountedSet( if (item.meta.bucket > self.layout.table_cap) return; - if (table[item.meta.bucket] != id) return; + assert(table[item.meta.bucket] == id); if (comptime @hasDecl(Context, "deleted")) { // Inform the context struct that we're @@ -449,6 +449,8 @@ pub fn RefCountedSet( } table[p] = 0; + + self.assertIntegrity(base, ctx); } /// Find an item in the table and return its ID. @@ -463,7 +465,7 @@ pub fn RefCountedSet( const hash: u64 = ctx.hash(value); for (0..self.max_psl + 1) |i| { - const p: usize = @intCast((hash + i) & self.layout.table_mask); + const p: usize = @intCast((hash +% i) & self.layout.table_mask); const id = table[p]; // Empty bucket, our item cannot have probed to @@ -538,11 +540,10 @@ pub fn RefCountedSet( var held_id: Id = new_id; var held_item: *Item = &new_item; - var chosen_p: ?Id = null; var chosen_id: Id = new_id; for (0..self.layout.table_cap - 1) |i| { - const p: Id = @intCast((hash + i) & self.layout.table_mask); + const p: Id = @intCast((hash +% i) & self.layout.table_mask); const id = table[p]; // Empty bucket, put our held item in to it and break. @@ -557,7 +558,9 @@ pub fn RefCountedSet( const item = &items[id]; // If there's a dead item then we resurrect it - // for our value so that we can re-use its ID. + // for our value so that we can re-use its ID, + // unless its ID is greater than the one we're + // given (i.e. prefer smaller IDs). if (item.meta.ref == 0) { if (comptime @hasDecl(Context, "deleted")) { // Inform the context struct that we're @@ -565,40 +568,33 @@ pub fn RefCountedSet( ctx.deleted(item.value); } - chosen_id = id; - - held_item.meta.bucket = p; + // Reap the dead item. self.psl_stats[item.meta.psl] -= 1; + item.* = .{}; + + // Only resurrect this item if it has a + // smaller id than the one we were given. + if (id < new_id) chosen_id = id; + + // Put the currently held item in to the + // bucket of the item that we just reaped. + table[p] = held_id; + held_item.meta.bucket = p; self.psl_stats[held_item.meta.psl] += 1; self.max_psl = @max(self.max_psl, held_item.meta.psl); - // If we're not still holding our new item then we - // need to make sure that we put the re-used ID in - // the right place, where we previously put new_id. - if (chosen_p) |c| { - table[c] = id; - table[p] = held_id; - } else { - // If we're still holding our new item then we - // don't actually have to do anything, because - // the table already has the correct ID here. - } - break; } // This item has a lower PSL, swap it out with our held item. if (item.meta.psl < held_item.meta.psl) { - if (held_id == new_id) { - chosen_p = p; - new_item.meta.bucket = p; - } - + // Put our held item in the bucket. table[p] = held_id; - items[held_id].meta.bucket = p; + held_item.meta.bucket = p; self.psl_stats[held_item.meta.psl] += 1; self.max_psl = @max(self.max_psl, held_item.meta.psl); + // Pick up the item that has a lower PSL. held_id = id; held_item = item; self.psl_stats[item.meta.psl] -= 1; @@ -608,8 +604,60 @@ pub fn RefCountedSet( held_item.meta.psl += 1; } + // Our chosen ID may have changed if we decided + // to re-use a dead item's ID, so we make sure + // the chosen bucket contains the correct ID. + table[new_item.meta.bucket] = chosen_id; + + // Finally place our new item in to our array. items[chosen_id] = new_item; + + self.assertIntegrity(base, ctx); + return chosen_id; } + + fn assertIntegrity( + self: *const Self, + base: anytype, + ctx: Context, + ) void { + // Disabled because this is excessively slow, only enable + // if debugging a RefCountedSet issue or modifying its logic. + if (false and std.debug.runtime_safety) { + const table = self.table.ptr(base); + const items = self.items.ptr(base); + + var psl_stats: [32]Id = [_]Id{0} ** 32; + + for (items[0..self.layout.cap], 0..) |item, id| { + if (item.meta.bucket < std.math.maxInt(Id)) { + assert(table[item.meta.bucket] == id); + psl_stats[item.meta.psl] += 1; + } + } + + std.testing.expectEqualSlices(Id, &psl_stats, &self.psl_stats) catch assert(false); + + assert(std.mem.eql(Id, &psl_stats, &self.psl_stats)); + + psl_stats = [_]Id{0} ** 32; + + for (table[0..self.layout.table_cap], 0..) |id, bucket| { + const item = items[id]; + if (item.meta.bucket < std.math.maxInt(Id)) { + assert(item.meta.bucket == bucket); + + const hash: u64 = ctx.hash(item.value); + const p: usize = @intCast((hash +% item.meta.psl) & self.layout.table_mask); + assert(p == bucket); + + psl_stats[item.meta.psl] += 1; + } + } + + std.testing.expectEqualSlices(Id, &psl_stats, &self.psl_stats) catch assert(false); + } + } }; } From b44ebed7988f3adeeb25c6bae746618c0cf10f9d Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 15:45:30 -0500 Subject: [PATCH 08/48] Fix a scenario that could cause issues under some conditions I don't know if this actually occurs in a way that can cause serious problems, but it's better to nip it in the bud. --- src/terminal/Screen.zig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 42bcd54c0..0e42dbb9b 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1763,10 +1763,15 @@ pub fn manualStyleUpdate(self: *Screen) !void { // If our new style is the default, just reset to that if (self.cursor.style.default()) { - self.cursor.style_id = 0; + self.cursor.style_id = style.default_id; return; } + // Clear the cursor style ID to prevent weird things from happening + // if the page capacity has to be adjusted which would end up calling + // manualStyleUpdate again. + self.cursor.style_id = style.default_id; + // After setting the style, we need to update our style map. // Note that we COULD lazily do this in print. We should look into // if that makes a meaningful difference. Our priority is to keep print From a908aca56331c180dbdd31f48a3849ffcc8672cf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Dec 2024 12:45:02 -0800 Subject: [PATCH 09/48] kittygfx: z-index handling fixes Fixes #2921 Our z-index handling was pretty much completely broken, hence I can't think of a better initial commit message. We were splitting the placements at the wrong points and just generally putting images in the wrong z-index. I'm shocked this didn't come up earlier. --- src/renderer/Metal.zig | 21 +++++++++++---------- src/renderer/OpenGL.zig | 21 +++++++++++---------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index fbd6c401b..acb4b4df5 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -1721,21 +1721,22 @@ fn prepKittyGraphics( }.lessThan, ); - // Find our indices - self.image_bg_end = 0; - self.image_text_end = 0; + // Find our indices. The values are sorted by z so we can find the + // first placement out of bounds to find the limits. + var bg_end: ?u32 = null; + var text_end: ?u32 = null; const bg_limit = std.math.minInt(i32) / 2; for (self.image_placements.items, 0..) |p, i| { - if (self.image_bg_end == 0 and p.z >= bg_limit) { - self.image_bg_end = @intCast(i); + if (bg_end == null and p.z >= bg_limit) { + bg_end = @intCast(i); } - if (self.image_text_end == 0 and p.z >= 0) { - self.image_text_end = @intCast(i); + if (text_end == null and p.z >= 0) { + text_end = @intCast(i); } } - if (self.image_text_end == 0) { - self.image_text_end = @intCast(self.image_placements.items.len); - } + + self.image_bg_end = bg_end orelse 0; + self.image_text_end = text_end orelse self.image_bg_end; } fn prepKittyVirtualPlacement( diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 481f5b0db..0a5179b56 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -952,21 +952,22 @@ fn prepKittyGraphics( }.lessThan, ); - // Find our indices - self.image_bg_end = 0; - self.image_text_end = 0; + // Find our indices. The values are sorted by z so we can find the + // first placement out of bounds to find the limits. + var bg_end: ?u32 = null; + var text_end: ?u32 = null; const bg_limit = std.math.minInt(i32) / 2; for (self.image_placements.items, 0..) |p, i| { - if (self.image_bg_end == 0 and p.z >= bg_limit) { - self.image_bg_end = @intCast(i); + if (bg_end == null and p.z >= bg_limit) { + bg_end = @intCast(i); } - if (self.image_text_end == 0 and p.z >= 0) { - self.image_text_end = @intCast(i); + if (text_end == null and p.z >= 0) { + text_end = @intCast(i); } } - if (self.image_text_end == 0) { - self.image_text_end = @intCast(self.image_placements.items.len); - } + + self.image_bg_end = bg_end orelse 0; + self.image_text_end = text_end orelse self.image_bg_end; } fn prepKittyVirtualPlacement( From 53ac0aa975f24831c16f5bdff7fe44ada5da31c1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Dec 2024 13:01:37 -0800 Subject: [PATCH 10/48] macos: update content scale whenever the screen changes Related to #2731 I'm not fully sure if this will fix this issue since I can't reproduce it but I don't see a downside to doing this and it might fix it. --- macos/Sources/Ghostty/SurfaceView_AppKit.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 14f2a43c8..5576515e3 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -409,6 +409,11 @@ extension Ghostty { // ID. If vsync is enabled, this will be used with the CVDisplayLink to ensure // the proper refresh rate is going. ghostty_surface_set_display_id(surface, screen.displayID ?? 0) + + // We also just trigger a backing property change. Just in case the screen has + // a different scaling factor, this ensures that we update our content scale. + // Issue: https://github.com/ghostty-org/ghostty/issues/2731 + viewDidChangeBackingProperties() } // MARK: - NSView From cb60f9d1da16813879bc18c6906c0a478bb2eec5 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 15:57:01 -0500 Subject: [PATCH 11/48] fix(RefCountedSet): Gracefully handle pathological cases Poor hash uniformity and/or a crafted or unlucky input could cause the bounds of the PSL stats array to be exceeded, which caused memory corruption (not good!) -- we avoid such cases now by returning an OutOfMemory error if we're about to insert and there's an item with a PSL in the last slot. --- src/terminal/ref_counted_set.zig | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index eecfae4b5..ddca32482 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -103,6 +103,12 @@ pub fn RefCountedSet( /// unlikely. Roughly a (1/table_cap)^32 -- with any normal /// table capacity that is so unlikely that it's not worth /// handling. + /// + /// However, that assumes a uniform hash function, which + /// is not guaranteed and can be subverted with a crafted + /// input. We handle this gracefully by returning an error + /// anywhere where we're about to insert if there's any + /// item with a PSL in the last slot of the stats array. psl_stats: [32]Id = [_]Id{0} ** 32, /// The backing store of items @@ -237,6 +243,16 @@ pub fn RefCountedSet( return id; } + // While it should be statistically impossible to exceed the + // bounds of `psl_stats`, the hash function is not perfect and + // in such a case we want to remain stable. If we're about to + // insert an item and there's something with a PSL of `len - 1`, + // we may end up with a PSL of `len` which would exceed the bounds. + // In such a case, we claim to be out of memory. + if (self.psl_stats[self.psl_stats.len - 1] > 0) { + return AddError.OutOfMemory; + } + // If the item doesn't exist, we need an available ID. if (self.next_id >= self.layout.cap) { // Arbitrarily chosen, threshold for rehashing. @@ -284,6 +300,11 @@ pub fn RefCountedSet( if (id < self.next_id) { if (items[id].meta.ref == 0) { + // See comment in `addContext` for details. + if (self.psl_stats[self.psl_stats.len - 1] > 0) { + return AddError.OutOfMemory; + } + self.deleteItem(base, id, ctx); const added_id = self.upsert(base, value, id, ctx); From 809593473b3f3c7516a29ceae78c8d6eb074a9ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Dec 2024 13:26:29 -0800 Subject: [PATCH 12/48] terminal: PageList.pin doesn't allow out of bounds x values Fixes #2958 The y was alround bounded but we allowed any x value and assumed the caller would handle it. This is not the case so we now check the x and return null if it's out of bounds (same as y, which was already doing this). --- src/terminal/PageList.zig | 12 +++++++++++- src/terminal/Selection.zig | 34 +++++++++++++++++----------------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index c976cf720..ca928fda6 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2406,9 +2406,19 @@ fn erasePage(self: *PageList, node: *List.Node) void { /// Returns the pin for the given point. The pin is NOT tracked so it /// is only valid as long as the pagelist isn't modified. +/// +/// This will return null if the point is out of bounds. The caller +/// should clamp the point to the bounds of the coordinate space if +/// necessary. pub fn pin(self: *const PageList, pt: point.Point) ?Pin { + // getTopLeft is much more expensive than checking the cols bounds + // so we do this first. + const x = pt.coord().x; + if (x >= self.cols) return null; + + // Grab the top left and move to the point. var p = self.getTopLeft(pt).down(pt.coord().y) orelse return null; - p.x = pt.coord().x; + p.x = x; return p; } diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index 6fdb921e7..a90595d20 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -451,7 +451,7 @@ pub fn adjust( test "Selection: adjust right" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A1234\nB5678\nC1234\nD5678"); @@ -518,7 +518,7 @@ test "Selection: adjust right" { test "Selection: adjust left" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A1234\nB5678\nC1234\nD5678"); @@ -567,7 +567,7 @@ test "Selection: adjust left" { test "Selection: adjust left skips blanks" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A1234\nB5678\nC12\nD56"); @@ -616,7 +616,7 @@ test "Selection: adjust left skips blanks" { test "Selection: adjust up" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A\nB\nC\nD\nE"); @@ -663,7 +663,7 @@ test "Selection: adjust up" { test "Selection: adjust down" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A\nB\nC\nD\nE"); @@ -702,7 +702,7 @@ test "Selection: adjust down" { .y = 1, } }, s.pages.pointFromPin(.screen, sel.start()).?); try testing.expectEqual(point.Point{ .screen = .{ - .x = 4, + .x = 9, .y = 4, } }, s.pages.pointFromPin(.screen, sel.end()).?); } @@ -710,7 +710,7 @@ test "Selection: adjust down" { test "Selection: adjust down with not full screen" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A\nB\nC"); @@ -730,7 +730,7 @@ test "Selection: adjust down with not full screen" { .y = 1, } }, s.pages.pointFromPin(.screen, sel.start()).?); try testing.expectEqual(point.Point{ .screen = .{ - .x = 4, + .x = 9, .y = 2, } }, s.pages.pointFromPin(.screen, sel.end()).?); } @@ -738,7 +738,7 @@ test "Selection: adjust down with not full screen" { test "Selection: adjust home" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A\nB\nC"); @@ -766,7 +766,7 @@ test "Selection: adjust home" { test "Selection: adjust end with not full screen" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); try s.testWriteString("A\nB\nC"); @@ -786,7 +786,7 @@ test "Selection: adjust end with not full screen" { .y = 0, } }, s.pages.pointFromPin(.screen, sel.start()).?); try testing.expectEqual(point.Point{ .screen = .{ - .x = 4, + .x = 9, .y = 2, } }, s.pages.pointFromPin(.screen, sel.end()).?); } @@ -1110,7 +1110,7 @@ test "Selection: order, rectangle" { test "topLeft" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); { // forward @@ -1173,7 +1173,7 @@ test "topLeft" { test "bottomRight" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); { // forward @@ -1236,7 +1236,7 @@ test "bottomRight" { test "ordered" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); { // forward @@ -1317,7 +1317,7 @@ test "ordered" { test "Selection: contains" { const testing = std.testing; - var s = try Screen.init(testing.allocator, 5, 10, 0); + var s = try Screen.init(testing.allocator, 10, 10, 0); defer s.deinit(); { const sel = Selection.init( @@ -1350,13 +1350,13 @@ test "Selection: contains" { { const sel = Selection.init( s.pages.pin(.{ .screen = .{ .x = 5, .y = 1 } }).?, - s.pages.pin(.{ .screen = .{ .x = 10, .y = 1 } }).?, + s.pages.pin(.{ .screen = .{ .x = 8, .y = 1 } }).?, false, ); try testing.expect(sel.contains(&s, s.pages.pin(.{ .screen = .{ .x = 6, .y = 1 } }).?)); try testing.expect(!sel.contains(&s, s.pages.pin(.{ .screen = .{ .x = 2, .y = 1 } }).?)); - try testing.expect(!sel.contains(&s, s.pages.pin(.{ .screen = .{ .x = 12, .y = 1 } }).?)); + try testing.expect(!sel.contains(&s, s.pages.pin(.{ .screen = .{ .x = 9, .y = 1 } }).?)); } } From bd90a6dd3b9af4f6d89ac8e34dd254d17d4e0ab2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Dec 2024 14:09:33 -0800 Subject: [PATCH 13/48] kittygfx: placement with rows (r) param scrolls properly out of viewport Fixes #2332 Two bugs fixed to fix this behavior: 1. Our destination height didn't account for the top-left being offscreen. 2. We were using the wrong height for the source rectangle. When a rows param (r=) is specified, the image height and destination height are at different scales. We were using the viewport scale for the offset but it should be the image scale. --- src/renderer/Metal.zig | 23 +++++++++++++++++++++-- src/renderer/OpenGL.zig | 23 +++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index acb4b4df5..3f24c3d2d 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -1821,6 +1821,21 @@ fn prepKittyPlacement( break :offset_y @intCast(offset_pixels); } else 0; + // If we specify `rows` then our offset above is in viewport space + // and not in the coordinate space of the source image. Without `rows` + // that's one and the same. + const source_offset_y: u32 = if (p.rows > 0) source_offset_y: { + // Determine the scale factor to apply for this row height. + const image_height: f64 = @floatFromInt(image.height); + const viewport_height: f64 = @floatFromInt(p.rows * self.grid_metrics.cell_height); + const scale: f64 = image_height / viewport_height; + + // Apply the scale to the offset + const offset_y_f64: f64 = @floatFromInt(offset_y); + const source_offset_y_f64: f64 = offset_y_f64 * scale; + break :source_offset_y @intFromFloat(@round(source_offset_y_f64)); + } else offset_y; + // We need to prep this image for upload if it isn't in the cache OR // it is in the cache but the transmit time doesn't match meaning this // image is different. @@ -1834,7 +1849,7 @@ fn prepKittyPlacement( // Calculate the source rectangle const source_x = @min(image.width, p.source_x); - const source_y = @min(image.height, p.source_y + offset_y); + const source_y = @min(image.height, p.source_y + source_offset_y); const source_width = if (p.source_width > 0) @min(image.width - source_x, p.source_width) else @@ -1846,7 +1861,11 @@ fn prepKittyPlacement( // Calculate the width/height of our image. const dest_width = if (p.columns > 0) p.columns * self.grid_metrics.cell_width else source_width; - const dest_height = if (p.rows > 0) p.rows * self.grid_metrics.cell_height else source_height; + const dest_height = if (p.rows > 0) rows: { + // Clip to the viewport to handle scrolling. offset_y is already in + // viewport scale so we can subtract it directly. + break :rows (p.rows * self.grid_metrics.cell_height) - offset_y; + } else source_height; // Accumulate the placement if (image.width > 0 and image.height > 0) { diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 0a5179b56..19843062c 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1052,6 +1052,21 @@ fn prepKittyPlacement( break :offset_y @intCast(offset_pixels); } else 0; + // If we specify `rows` then our offset above is in viewport space + // and not in the coordinate space of the source image. Without `rows` + // that's one and the same. + const source_offset_y: u32 = if (p.rows > 0) source_offset_y: { + // Determine the scale factor to apply for this row height. + const image_height: f64 = @floatFromInt(image.height); + const viewport_height: f64 = @floatFromInt(p.rows * self.grid_metrics.cell_height); + const scale: f64 = image_height / viewport_height; + + // Apply the scale to the offset + const offset_y_f64: f64 = @floatFromInt(offset_y); + const source_offset_y_f64: f64 = offset_y_f64 * scale; + break :source_offset_y @intFromFloat(@round(source_offset_y_f64)); + } else offset_y; + // We need to prep this image for upload if it isn't in the cache OR // it is in the cache but the transmit time doesn't match meaning this // image is different. @@ -1065,7 +1080,7 @@ fn prepKittyPlacement( // Calculate the source rectangle const source_x = @min(image.width, p.source_x); - const source_y = @min(image.height, p.source_y + offset_y); + const source_y = @min(image.height, p.source_y + source_offset_y); const source_width = if (p.source_width > 0) @min(image.width - source_x, p.source_width) else @@ -1077,7 +1092,11 @@ fn prepKittyPlacement( // Calculate the width/height of our image. const dest_width = if (p.columns > 0) p.columns * self.grid_metrics.cell_width else source_width; - const dest_height = if (p.rows > 0) p.rows * self.grid_metrics.cell_height else source_height; + const dest_height = if (p.rows > 0) rows: { + // Clip to the viewport to handle scrolling. offset_y is already in + // viewport scale so we can subtract it directly. + break :rows (p.rows * self.grid_metrics.cell_height) - offset_y; + } else source_height; // Accumulate the placement if (image.width > 0 and image.height > 0) { From 53c41255eb68b4ca13950260dd2c5c4684f4b2d4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Dec 2024 14:30:59 -0800 Subject: [PATCH 14/48] terminal: selectionString only applies x offset on first/last page Fixes #2841 We were incorrectly applying the start/end x offset for the first/last row of every single page. If a selection spanned multiple pages this would trim data incorrectly. Unit test updated to cover this case. --- src/terminal/Screen.zig | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 0e42dbb9b..97a1f56dd 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -2073,17 +2073,18 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! }; var page_it = sel_start.pageIterator(.right_down, sel_end); - var row_count: usize = 0; while (page_it.next()) |chunk| { const rows = chunk.rows(); - for (rows, chunk.start..) |row, y| { + for (rows, chunk.start.., 0..) |row, y, row_i| { const cells_ptr = row.cells.ptr(chunk.node.data.memory); - const start_x = if (row_count == 0 or sel_ordered.rectangle) + const start_x = if ((row_i == 0 or sel_ordered.rectangle) and + sel_start.node == chunk.node) sel_start.x else 0; - const end_x = if (row_count == rows.len - 1 or sel_ordered.rectangle) + const end_x = if ((row_i == rows.len - 1 or sel_ordered.rectangle) and + sel_end.node == chunk.node) sel_end.x + 1 else self.pages.cols; @@ -2138,8 +2139,6 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! .x = chunk.node.data.size.cols - 1, }); } - - row_count += 1; } } @@ -8306,7 +8305,7 @@ test "Screen: selectionString multi-page" { const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, 1, 3, 2048); + var s = try init(alloc, 10, 3, 2048); defer s.deinit(); const first_page_size = s.pages.pages.first.?.data.capacity.rows; @@ -8318,20 +8317,20 @@ test "Screen: selectionString multi-page" { } s.pages.pages.first.?.data.pauseIntegrityChecks(false); - try s.testWriteString("y\ny\ny"); + try s.testWriteString("123456789\n!@#$%^&*(\n123456789"); { const sel = Selection.init( s.pages.pin(.{ .active = .{ .x = 0, .y = 0 } }).?, - s.pages.pin(.{ .active = .{ .x = 0, .y = 2 } }).?, + s.pages.pin(.{ .active = .{ .x = 2, .y = 2 } }).?, false, ); const contents = try s.selectionString(alloc, .{ .sel = sel, - .trim = false, + .trim = true, }); defer alloc.free(contents); - const expected = "y\ny\ny"; + const expected = "123456789\n!@#$%^&*(\n123"; try testing.expectEqualStrings(expected, contents); } } From 415902fe832a525fe99c38b6afc925655c97ae5f Mon Sep 17 00:00:00 2001 From: Iain H Date: Mon, 23 Dec 2024 19:50:57 -0500 Subject: [PATCH 15/48] apprt/gtk: support dark window decorations with GtkWindow Add support for the dark GtkWindow variant when `window-theme` is `dark` or `auto`. --- src/apprt/gtk/App.zig | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 8c42ddf37..042079dd6 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -221,6 +221,9 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { switch (config.@"window-theme") { .system, .light => {}, .dark => { + const settings = c.gtk_settings_get_default(); + c.g_object_set(@ptrCast(@alignCast(settings)), "gtk-application-prefer-dark-theme", true, @as([*c]const u8, null)); + c.gtk_css_provider_load_from_resource( provider, "/com/mitchellh/ghostty/style-dark.css", @@ -234,6 +237,9 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { .auto, .ghostty => { const lum = config.background.toTerminalRGB().perceivedLuminance(); if (lum <= 0.5) { + const settings = c.gtk_settings_get_default(); + c.g_object_set(@ptrCast(@alignCast(settings)), "gtk-application-prefer-dark-theme", true, @as([*c]const u8, null)); + c.gtk_css_provider_load_from_resource( provider, "/com/mitchellh/ghostty/style-dark.css", From 56cbbd940bfb8f49870a264c8439ed4d5b90b7c3 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 21:41:51 -0500 Subject: [PATCH 16/48] perf(RefCountedSet): make swap metric prioritize high refcount items This experimentally yields a ~20% performance improvement as measured by running DOOM-fire-zig, which is honestly a lot more than I expected. --- src/terminal/ref_counted_set.zig | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index ddca32482..1a58a4e5b 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -607,8 +607,16 @@ pub fn RefCountedSet( break; } - // This item has a lower PSL, swap it out with our held item. - if (item.meta.psl < held_item.meta.psl) { + // If this item has a lower PSL, or has equal PSL and lower ref + // count, then we swap it out with our held item. By doing this, + // items with high reference counts are prioritized for earlier + // placement. The assumption is that an item which has a higher + // reference count will be accessed more frequently, so we want + // to minimize the time it takes to find it. + if (item.meta.psl < held_item.meta.psl or + item.meta.psl == held_item.meta.psl and + item.meta.ref < held_item.meta.ref) + { // Put our held item in the bucket. table[p] = held_id; held_item.meta.bucket = p; From 3bfe4cd25ca7a5ae4d4084818b86ada9236b3bb5 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 23 Dec 2024 22:04:37 -0500 Subject: [PATCH 17/48] perf(styles): greatly improve style.hash performance By switching to one-shot hashing of the raw bytes of the struct with XxHash3 instead of using `autoHash` with Wyhash, a performance gain of around 20% can be observed in DOOM-fire-zig. --- src/terminal/style.zig | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/terminal/style.zig b/src/terminal/style.zig index 0340047e9..fe2589a46 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -8,8 +8,7 @@ const Offset = size.Offset; const OffsetBuf = size.OffsetBuf; const RefCountedSet = @import("ref_counted_set.zig").RefCountedSet; -const Wyhash = std.hash.Wyhash; -const autoHash = std.hash.autoHash; +const XxHash3 = std.hash.XxHash3; /// The unique identifier for a style. This is at most the number of cells /// that can fit into a terminal page. @@ -230,10 +229,13 @@ pub const Style = struct { _ = try writer.write(" }"); } + /// Hash the raw bytes of the struct with XxHash3 + /// + /// NOTE: Because the struct does not have a guaranteed in-memory layout + /// this hash is NOT suitable for serialization. If used for a hash + /// table that is then serialized, it MUST be re-hashed when read. pub fn hash(self: *const Style) u64 { - var hasher = Wyhash.init(0); - autoHash(&hasher, self.*); - return hasher.final(); + return XxHash3.hash(0, @as(*const [@sizeOf(Style)]u8, @ptrCast(self))); } test { From 1497e90f9535f9b850a9cf39b8db76cda64a9415 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Tue, 24 Dec 2024 14:44:21 +0100 Subject: [PATCH 18/48] feat: add support for middle position in quick terminal This update introduces the ability to launch the quick terminal in the middle position. Resolves #2494 --- .../QuickTerminal/QuickTerminalPosition.swift | 19 +++++++++++++++++++ src/config/Config.zig | 1 + 2 files changed, 20 insertions(+) diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift index 51b450700..8669af3de 100644 --- a/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift +++ b/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift @@ -5,6 +5,7 @@ enum QuickTerminalPosition : String { case bottom case left case right + case middle /// Set the loaded state for a window. func setLoaded(_ window: NSWindow) { @@ -25,6 +26,14 @@ enum QuickTerminalPosition : String { width: screen.frame.width / 4, height: screen.frame.height) ), display: false) + + case .middle: + window.setFrame(.init( + origin: window.frame.origin, + size: .init( + width: screen.frame.width / 2, + height: screen.frame.height / 3) + ), display: false) } } @@ -61,6 +70,10 @@ enum QuickTerminalPosition : String { case .left, .right: finalSize.height = screen.frame.height + + case .middle: + finalSize.width = screen.frame.width / 2 + finalSize.height = screen.frame.height / 3 } return finalSize @@ -80,6 +93,9 @@ enum QuickTerminalPosition : String { case .right: return .init(x: screen.frame.maxX, y: 0) + + case .middle: + return .init(x: (screen.visibleFrame.maxX - window.frame.width) / 2, y: screen.visibleFrame.maxY - window.frame.width) } } @@ -97,6 +113,9 @@ enum QuickTerminalPosition : String { case .right: return .init(x: screen.visibleFrame.maxX - window.frame.width, y: window.frame.origin.y) + + case .middle: + return .init(x: (screen.visibleFrame.maxX - window.frame.width) / 2, y: (screen.visibleFrame.maxY - window.frame.height) / 2) } } } diff --git a/src/config/Config.zig b/src/config/Config.zig index 4727da832..4c146bc1c 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -5257,6 +5257,7 @@ pub const QuickTerminalPosition = enum { bottom, left, right, + middle, }; /// See quick-terminal-screen From 67fb7d0bee29366da2f1a351c3b80c7279b1aeda Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 24 Dec 2024 07:20:55 -0800 Subject: [PATCH 19/48] Fix UB in style hashing by using autoHash, but keep XxHash3 Back out "perf(styles): greatly improve style.hash performance" This backs out commit 3bfe4cd25ca7a5ae4d4084818b86ada9236b3bb5, but keeps the hash algorithm as XxHash3 which showed improvements in performance. --- src/terminal/style.zig | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/terminal/style.zig b/src/terminal/style.zig index fe2589a46..7dd4edae7 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -9,6 +9,7 @@ const OffsetBuf = size.OffsetBuf; const RefCountedSet = @import("ref_counted_set.zig").RefCountedSet; const XxHash3 = std.hash.XxHash3; +const autoHash = std.hash.autoHash; /// The unique identifier for a style. This is at most the number of cells /// that can fit into a terminal page. @@ -229,13 +230,10 @@ pub const Style = struct { _ = try writer.write(" }"); } - /// Hash the raw bytes of the struct with XxHash3 - /// - /// NOTE: Because the struct does not have a guaranteed in-memory layout - /// this hash is NOT suitable for serialization. If used for a hash - /// table that is then serialized, it MUST be re-hashed when read. pub fn hash(self: *const Style) u64 { - return XxHash3.hash(0, @as(*const [@sizeOf(Style)]u8, @ptrCast(self))); + var hasher = XxHash3.init(0); + autoHash(&hasher, self.*); + return hasher.final(); } test { From 120a2b9597915bb218271cfff52abe305e1f0b15 Mon Sep 17 00:00:00 2001 From: David Rubin Date: Tue, 24 Dec 2024 08:20:54 -0800 Subject: [PATCH 20/48] optimize `Style` hashing to be single-shot --- src/terminal/color.zig | 8 ++--- src/terminal/style.zig | 81 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/terminal/color.zig b/src/terminal/color.zig index df94baf0e..08f725d5c 100644 --- a/src/terminal/color.zig +++ b/src/terminal/color.zig @@ -95,7 +95,7 @@ pub const Name = enum(u8) { }; /// RGB -pub const RGB = struct { +pub const RGB = packed struct(u24) { r: u8 = 0, g: u8 = 0, b: u8 = 0, @@ -155,9 +155,9 @@ pub const RGB = struct { return 0.299 * (r_f64 / 255) + 0.587 * (g_f64 / 255) + 0.114 * (b_f64 / 255); } - test "size" { - try std.testing.expectEqual(@as(usize, 24), @bitSizeOf(RGB)); - try std.testing.expectEqual(@as(usize, 3), @sizeOf(RGB)); + comptime { + assert(@bitSizeOf(RGB) == 24); + assert(@sizeOf(RGB) == 4); } /// Parse a color from a floating point intensity value. diff --git a/src/terminal/style.zig b/src/terminal/style.zig index 7dd4edae7..84001d292 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -27,7 +27,9 @@ pub const Style = struct { /// On/off attributes that don't require much bit width so we use /// a packed struct to make this take up significantly less space. - flags: packed struct { + flags: Flags = .{}, + + const Flags = packed struct(u16) { bold: bool = false, italic: bool = false, faint: bool = false, @@ -37,16 +39,23 @@ pub const Style = struct { strikethrough: bool = false, overline: bool = false, underline: sgr.Attribute.Underline = .none, - } = .{}, + _padding: u5 = 0, + }; /// The color for an SGR attribute. A color can come from multiple /// sources so we use this to track the source plus color value so that /// we can properly react to things like palette changes. - pub const Color = union(enum) { + pub const Color = union(Tag) { none: void, palette: u8, rgb: color.RGB, + const Tag = enum(u8) { + none, + palette, + rgb, + }; + /// Formatting to make debug logs easier to read /// by only including non-default attributes. pub fn format( @@ -230,16 +239,68 @@ pub const Style = struct { _ = try writer.write(" }"); } + const PackedStyle = packed struct(u128) { + tags: packed struct { + fg: Color.Tag, + bg: Color.Tag, + underline: Color.Tag, + }, + data: packed struct { + fg: Data, + bg: Data, + underline: Data, + }, + flags: Flags, + _padding: u16 = 0, + + const Data = packed union { + none: u24, + palette: packed struct(u24) { + idx: u8, + _padding: u16 = 0, + }, + rgb: color.RGB, + + fn fromColor(c: Color) Data { + return switch (c) { + inline else => |v, t| @unionInit( + Data, + @tagName(t), + switch (t) { + .none => 0, + .palette => .{ .idx = v }, + .rgb => v, + }, + ), + }; + } + }; + + fn fromStyle(style: Style) PackedStyle { + return .{ + .tags = .{ + .fg = std.meta.activeTag(style.fg_color), + .bg = std.meta.activeTag(style.bg_color), + .underline = std.meta.activeTag(style.underline_color), + }, + .data = .{ + .fg = Data.fromColor(style.fg_color), + .bg = Data.fromColor(style.bg_color), + .underline = Data.fromColor(style.underline_color), + }, + .flags = style.flags, + }; + } + }; + pub fn hash(self: *const Style) u64 { - var hasher = XxHash3.init(0); - autoHash(&hasher, self.*); - return hasher.final(); + const packed_style = PackedStyle.fromStyle(self.*); + return XxHash3.hash(0, std.mem.asBytes(&packed_style)); } - test { - // The size of the struct so we can be aware of changes. - const testing = std.testing; - try testing.expectEqual(@as(usize, 14), @sizeOf(Style)); + comptime { + assert(@sizeOf(PackedStyle) == 16); + assert(std.meta.hasUniqueRepresentation(PackedStyle)); } }; From 5052be3efe8dc1672f518cb6ca37cf53b74a49ca Mon Sep 17 00:00:00 2001 From: David Rubin Date: Tue, 24 Dec 2024 10:07:58 -0800 Subject: [PATCH 21/48] add some comments --- src/terminal/style.zig | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/terminal/style.zig b/src/terminal/style.zig index 84001d292..24b3a19dd 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -239,6 +239,14 @@ pub const Style = struct { _ = try writer.write(" }"); } + /// `PackedStyle` represents the same data as `Style` but without padding, + /// which is necassary for hashing via re-interpretation of the underlying bytes. + /// + /// `Style` is still prefered for everything else as it has type-safety when using + /// the `Color` tagged union. + /// + /// Empirical testing shows that storing all of the tags first and then the datas + /// provides a better layout for serializing into and is faster on benchmarks. const PackedStyle = packed struct(u128) { tags: packed struct { fg: Color.Tag, @@ -253,6 +261,10 @@ pub const Style = struct { flags: Flags, _padding: u16 = 0, + /// After https://github.com/ziglang/zig/issues/19754 is implemented, it + /// will be an compiler-error to have packed union fields of differing size. + /// + /// For now we just need to be careful not to accidentally introduce padding. const Data = packed union { none: u24, palette: packed struct(u24) { From d7542ec504e63d1d52727504d2307dbafb2502ee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 24 Dec 2024 12:07:58 -0800 Subject: [PATCH 22/48] terminal: address typos in style struct --- src/terminal/style.zig | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/terminal/style.zig b/src/terminal/style.zig index 24b3a19dd..b58a33196 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -240,13 +240,15 @@ pub const Style = struct { } /// `PackedStyle` represents the same data as `Style` but without padding, - /// which is necassary for hashing via re-interpretation of the underlying bytes. + /// which is necessary for hashing via re-interpretation of the underlying + /// bytes. /// - /// `Style` is still prefered for everything else as it has type-safety when using - /// the `Color` tagged union. + /// `Style` is still preferred for everything else as it has type-safety + /// when using the `Color` tagged union. /// - /// Empirical testing shows that storing all of the tags first and then the datas - /// provides a better layout for serializing into and is faster on benchmarks. + /// Empirical testing shows that storing all of the tags first and then the + /// data provides a better layout for serializing into and is faster on + /// benchmarks. const PackedStyle = packed struct(u128) { tags: packed struct { fg: Color.Tag, @@ -261,10 +263,12 @@ pub const Style = struct { flags: Flags, _padding: u16 = 0, - /// After https://github.com/ziglang/zig/issues/19754 is implemented, it - /// will be an compiler-error to have packed union fields of differing size. + /// After https://github.com/ziglang/zig/issues/19754 is implemented, + /// it will be an compiler-error to have packed union fields of + /// differing size. /// - /// For now we just need to be careful not to accidentally introduce padding. + /// For now we just need to be careful not to accidentally introduce + /// padding. const Data = packed union { none: u24, palette: packed struct(u24) { From 5bb2c62fbab8aeeb1c0648ead1da8123adcffbdd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 24 Dec 2024 14:46:43 -0800 Subject: [PATCH 23/48] config: revert cmd/alt+left/right to legacy encoding on macOS by default Fixes #3106 Related to #2838 Discussion in #2363 This breaks fixterms encodings for these specific keys so that shells and other programs that rely on the legacy encoding for these keys will work by default. This only does this for macOS because for whatever reason during the large beta period, only macOS users found this as lacking. If users want to restore fixterms behaviors, they can rebind these keys as `unbind`. --- src/config/Config.zig | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index 4727da832..806c16b1c 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2532,6 +2532,32 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { .{ .key = .{ .translated = .f }, .mods = .{ .super = true, .ctrl = true } }, .{ .toggle_fullscreen = {} }, ); + + // "Natural text editing" keybinds. This forces these keys to go back + // to legacy encoding (not fixterms). It seems macOS users more than + // others are used to these keys so we set them as defaults. If + // people want to get back to the fixterm encoding they can set + // the keybinds to `unbind`. + try result.keybind.set.put( + alloc, + .{ .key = .{ .translated = .right }, .mods = .{ .super = true } }, + .{ .text = "\x05" }, + ); + try result.keybind.set.put( + alloc, + .{ .key = .{ .translated = .left }, .mods = .{ .super = true } }, + .{ .text = "\x01" }, + ); + try result.keybind.set.put( + alloc, + .{ .key = .{ .translated = .left }, .mods = .{ .alt = true } }, + .{ .esc = "b" }, + ); + try result.keybind.set.put( + alloc, + .{ .key = .{ .translated = .right }, .mods = .{ .alt = true } }, + .{ .esc = "f" }, + ); } // Add our default link for URL detection From 44459e93d1ed6081071428fdbc2e0122bcfb86b3 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Wed, 25 Dec 2024 00:07:12 +0100 Subject: [PATCH 24/48] code review --- .../Features/QuickTerminal/QuickTerminalPosition.swift | 10 +++++----- src/config/Config.zig | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift index 8669af3de..3d2a2a045 100644 --- a/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift +++ b/macos/Sources/Features/QuickTerminal/QuickTerminalPosition.swift @@ -5,7 +5,7 @@ enum QuickTerminalPosition : String { case bottom case left case right - case middle + case center /// Set the loaded state for a window. func setLoaded(_ window: NSWindow) { @@ -27,7 +27,7 @@ enum QuickTerminalPosition : String { height: screen.frame.height) ), display: false) - case .middle: + case .center: window.setFrame(.init( origin: window.frame.origin, size: .init( @@ -71,7 +71,7 @@ enum QuickTerminalPosition : String { case .left, .right: finalSize.height = screen.frame.height - case .middle: + case .center: finalSize.width = screen.frame.width / 2 finalSize.height = screen.frame.height / 3 } @@ -94,7 +94,7 @@ enum QuickTerminalPosition : String { case .right: return .init(x: screen.frame.maxX, y: 0) - case .middle: + case .center: return .init(x: (screen.visibleFrame.maxX - window.frame.width) / 2, y: screen.visibleFrame.maxY - window.frame.width) } } @@ -114,7 +114,7 @@ enum QuickTerminalPosition : String { case .right: return .init(x: screen.visibleFrame.maxX - window.frame.width, y: window.frame.origin.y) - case .middle: + case .center: return .init(x: (screen.visibleFrame.maxX - window.frame.width) / 2, y: (screen.visibleFrame.maxY - window.frame.height) / 2) } } diff --git a/src/config/Config.zig b/src/config/Config.zig index 4c146bc1c..e82b4f97a 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1377,6 +1377,7 @@ keybind: Keybinds = .{}, /// * `bottom` - Terminal appears at the bottom of the screen. /// * `left` - Terminal appears at the left of the screen. /// * `right` - Terminal appears at the right of the screen. +/// * `center` - Terminal appears at the center of the screen. /// /// Changing this configuration requires restarting Ghostty completely. @"quick-terminal-position": QuickTerminalPosition = .top, @@ -5257,7 +5258,7 @@ pub const QuickTerminalPosition = enum { bottom, left, right, - middle, + center, }; /// See quick-terminal-screen From 8efa638110a05c2b9fcc72608d97d34993fba3d7 Mon Sep 17 00:00:00 2001 From: David Rubin Date: Tue, 24 Dec 2024 22:53:39 -0800 Subject: [PATCH 25/48] optimize `Style.eql` using `PackedStyle` --- src/terminal/style.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/terminal/style.zig b/src/terminal/style.zig index b58a33196..6c1f8b608 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -87,7 +87,10 @@ pub const Style = struct { /// True if the style is equal to another style. pub fn eql(self: Style, other: Style) bool { - return std.meta.eql(self, other); + const packed_self = PackedStyle.fromStyle(self); + const packed_other = PackedStyle.fromStyle(other); + // TODO: in Zig 0.14, equating packed structs is allowed. Remove this work around. + return @as(u128, @bitCast(packed_self)) == @as(u128, @bitCast(packed_other)); } /// Returns the bg color for a cell with this style given the cell From c3e493f247f7a913885a1b188271fc2149278159 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 25 Dec 2024 07:04:49 -0800 Subject: [PATCH 26/48] config: use escaped encoding for macOS cmd+left/right Fixes #3114 I forgot that the format gets parsed as a Zig string so putting it in already parsed made `+list-keybinds` incorrect. It worked either way but this fixes the `+list-keybinds` CLI action. --- src/config/Config.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index b89aa566d..941e794f8 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2542,12 +2542,12 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { try result.keybind.set.put( alloc, .{ .key = .{ .translated = .right }, .mods = .{ .super = true } }, - .{ .text = "\x05" }, + .{ .text = "\\x05" }, ); try result.keybind.set.put( alloc, .{ .key = .{ .translated = .left }, .mods = .{ .super = true } }, - .{ .text = "\x01" }, + .{ .text = "\\x01" }, ); try result.keybind.set.put( alloc, From 48368471a76a3298ca8b569c2376010f68e7b1dd Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Wed, 25 Dec 2024 22:45:18 +0300 Subject: [PATCH 27/48] fix: correct handling of CTC and DECST8C Cursor Tab Control (CTC) `CSI W` has a default value of 0, which is the same as setting a tab stop at the current cursor position. Set Tab at every 8th Column (DECST8C) `CSI ? 5 W` is a DEC private sequence that resets the tab stops to every 8th column. Reference: https://vt100.net/docs/vt510-rm/DECST8C.html Reference: https://wezfurlong.org/ecma48/07-control.html?highlight=ctc#7210-ctc---cursor-tabulation-control Reference: https://gitlab.gnome.org/GNOME/vte/-/blob/master/src/parser-seq.py#L596-L601 Fixes: https://github.com/ghostty-org/ghostty/issues/3120 --- src/terminal/stream.zig | 102 ++++++++++++++++++++++++++++++++-------- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index b8d60a13f..71d2bf9c2 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -601,30 +601,37 @@ pub fn Stream(comptime Handler: type) type { // Cursor Tabulation Control 'W' => { switch (input.params.len) { - 0 => if (input.intermediates.len == 1 and input.intermediates[0] == '?') { - if (@hasDecl(T, "tabReset")) - try self.handler.tabReset() - else - log.warn("unimplemented tab reset callback: {}", .{input}); - }, + 0 => if (@hasDecl(T, "tabSet")) + try self.handler.tabSet() + else + log.warn("unimplemented tab set callback: {}", .{input}), - 1 => switch (input.params[0]) { - 0 => if (@hasDecl(T, "tabSet")) - try self.handler.tabSet() - else - log.warn("unimplemented tab set callback: {}", .{input}), + 1 => if (input.intermediates.len == 1 and input.intermediates[0] == '?') { + if (input.params[0] == 5) { + if (@hasDecl(T, "tabReset")) + try self.handler.tabReset() + else + log.warn("unimplemented tab reset callback: {}", .{input}); + } else log.warn("invalid cursor tabulation control: {}", .{input}); + } else { + switch (input.params[0]) { + 0 => if (@hasDecl(T, "tabSet")) + try self.handler.tabSet() + else + log.warn("unimplemented tab set callback: {}", .{input}), - 2 => if (@hasDecl(T, "tabClear")) - try self.handler.tabClear(.current) - else - log.warn("unimplemented tab clear callback: {}", .{input}), + 2 => if (@hasDecl(T, "tabClear")) + try self.handler.tabClear(.current) + else + log.warn("unimplemented tab clear callback: {}", .{input}), - 5 => if (@hasDecl(T, "tabClear")) - try self.handler.tabClear(.all) - else - log.warn("unimplemented tab clear callback: {}", .{input}), + 5 => if (@hasDecl(T, "tabClear")) + try self.handler.tabClear(.all) + else + log.warn("unimplemented tab clear callback: {}", .{input}), - else => {}, + else => {}, + } }, else => {}, @@ -2327,3 +2334,58 @@ test "stream: CSI t pop title with index" { .index = 5, }, s.handler.op.?); } + +test "stream CSI W clear tab stops" { + const H = struct { + op: ?csi.TabClear = null, + + pub fn tabClear(self: *@This(), op: csi.TabClear) !void { + self.op = op; + } + }; + + var s: Stream(H) = .{ .handler = .{} }; + + try s.nextSlice("\x1b[2W"); + try testing.expectEqual(csi.TabClear.current, s.handler.op.?); + + try s.nextSlice("\x1b[5W"); + try testing.expectEqual(csi.TabClear.all, s.handler.op.?); +} + +test "stream CSI W tab set" { + const H = struct { + called: bool = false, + + pub fn tabSet(self: *@This()) !void { + self.called = true; + } + }; + + var s: Stream(H) = .{ .handler = .{} }; + + try s.nextSlice("\x1b[W"); + try testing.expect(s.handler.called); + + s.handler.called = false; + try s.nextSlice("\x1b[0W"); + try testing.expect(s.handler.called); +} + +test "stream CSI ? W reset tab stops" { + const H = struct { + reset: bool = false, + + pub fn tabReset(self: *@This()) !void { + self.reset = true; + } + }; + + var s: Stream(H) = .{ .handler = .{} }; + + try s.nextSlice("\x1b[?2W"); + try testing.expect(!s.handler.reset); + + try s.nextSlice("\x1b[?5W"); + try testing.expect(s.handler.reset); +} From c3bf7246f64a60194e957805f25aa82c76a426e7 Mon Sep 17 00:00:00 2001 From: Jan200101 Date: Tue, 24 Dec 2024 21:50:57 +0100 Subject: [PATCH 28/48] terminal: parse ConEmu progress OSC 9 Fixes #3119 ConEmu and iTerm2 both use OSC 9 to implement different things. iTerm2 uses it to implement desktop notifications, while ConEmu uses it to implement various OS commands. Ghostty has supported iTerm2 OSC 9 for a while, but it didn't (and doesn't) support ConEmu OSC 9. This means that if a program tries to send a ConEmu OSC 9 to Ghostty, it will turn into a desktop notification. This commit adds parsing for ConEmu OSC 9 progress reports. This means that these specific syntaxes can never be desktop notifications, but they're quite strange to be desktop notifications anyway so this should be an okay tradeoff. This doesn't actually _do anything with the progress reports_, it just parses them so that they don't turn into desktop notifications. --- src/terminal/osc.zig | 252 ++++++++++++++++++++++++++++++++++++++-- src/terminal/stream.zig | 4 + 2 files changed, 249 insertions(+), 7 deletions(-) diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index 34bc46745..3f7236a2c 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -158,6 +158,12 @@ pub const Command = union(enum) { /// End a hyperlink (OSC 8) hyperlink_end: void, + /// Set progress state (OSC 9;4) + progress: struct { + state: ProgressState, + progress: ?u8 = null, + }, + pub const ColorKind = union(enum) { palette: u8, foreground, @@ -173,6 +179,14 @@ pub const Command = union(enum) { }; } }; + + pub const ProgressState = enum { + remove, + set, + @"error", + indeterminate, + pause, + }; }; /// The terminator used to end an OSC command. For OSC commands that demand @@ -322,6 +336,27 @@ pub const Parser = struct { // https://sw.kovidgoyal.net/kitty/color-stack/#id1 kitty_color_protocol_key, kitty_color_protocol_value, + + // OSC 9 is used by ConEmu and iTerm2 for different things. + // iTerm2 uses it to post a notification[1]. + // ConEmu uses it to implement many custom functions[2]. + // + // Some Linux applications (namely systemd and flatpak) have + // adopted the ConEmu implementation but this causes bogus + // notifications on iTerm2 compatible terminal emulators. + // + // Ghostty supports both by disallowing ConEmu-specific commands + // from being shown as desktop notifications. + // + // [1]: https://iterm2.com/documentation-escape-codes.html + // [2]: https://conemu.github.io/en/AnsiEscapeCodes.html#OSC_Operating_system_commands + osc_9, + + // ConEmu specific substates + conemu_progress_prestate, + conemu_progress_state, + conemu_progress_prevalue, + conemu_progress_value, }; /// This must be called to clean up any allocated memory. @@ -735,18 +770,99 @@ pub const Parser = struct { .@"9" => switch (c) { ';' => { - self.command = .{ .show_desktop_notification = .{ - .title = "", - .body = undefined, - } }; - - self.temp_state = .{ .str = &self.command.show_desktop_notification.body }; self.buf_start = self.buf_idx; - self.state = .string; + self.state = .osc_9; }, else => self.state = .invalid, }, + .osc_9 => switch (c) { + '4' => { + self.state = .conemu_progress_prestate; + }, + + // Todo: parse out other ConEmu operating system commands. + // Even if we don't support them we probably don't want + // them showing up as desktop notifications. + + else => self.showDesktopNotification(), + }, + + .conemu_progress_prestate => switch (c) { + ';' => { + self.command = .{ .progress = .{ + .state = undefined, + } }; + self.state = .conemu_progress_state; + }, + else => self.showDesktopNotification(), + }, + + .conemu_progress_state => switch (c) { + '0' => { + self.command.progress.state = .remove; + self.state = .conemu_progress_prevalue; + self.complete = true; + }, + '1' => { + self.command.progress.state = .set; + self.command.progress.progress = 0; + self.state = .conemu_progress_prevalue; + }, + '2' => { + self.command.progress.state = .@"error"; + self.complete = true; + self.state = .conemu_progress_prevalue; + }, + '3' => { + self.command.progress.state = .indeterminate; + self.complete = true; + self.state = .conemu_progress_prevalue; + }, + '4' => { + self.command.progress.state = .pause; + self.complete = true; + self.state = .conemu_progress_prevalue; + }, + else => self.showDesktopNotification(), + }, + + .conemu_progress_prevalue => switch (c) { + ';' => { + self.state = .conemu_progress_value; + }, + + else => self.showDesktopNotification(), + }, + + .conemu_progress_value => switch (c) { + '0'...'9' => value: { + // No matter what substate we're in, a number indicates + // a completed ConEmu progress command. + self.complete = true; + + // If we aren't a set substate, then we don't care + // about the value. + const p = &self.command.progress; + if (p.state != .set) break :value; + assert(p.progress != null); + + // If we're over 100% we're done. + if (p.progress.? >= 100) break :value; + + // If we're over 10 then any new digit forces us to + // be 100. + if (p.progress.? >= 10) + p.progress = 100 + else { + const d = std.fmt.charToDigit(c, 10) catch 0; + p.progress = @min(100, (p.progress.? * 10) + d); + } + }, + + else => self.showDesktopNotification(), + }, + .query_fg_color => switch (c) { '?' => { self.command = .{ .report_color = .{ .kind = .foreground } }; @@ -901,6 +1017,16 @@ pub const Parser = struct { } } + fn showDesktopNotification(self: *Parser) void { + self.command = .{ .show_desktop_notification = .{ + .title = "", + .body = undefined, + } }; + + self.temp_state = .{ .str = &self.command.show_desktop_notification.body }; + self.state = .string; + } + fn prepAllocableString(self: *Parser) void { assert(self.buf_dynamic == null); @@ -1532,6 +1658,118 @@ test "OSC: show desktop notification with title" { try testing.expectEqualStrings(cmd.show_desktop_notification.body, "Body"); } +test "OSC: OSC9 progress set" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;1;100"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .set); + try testing.expect(cmd.progress.progress == 100); +} + +test "OSC: OSC9 progress set overflow" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;1;900"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .set); + try testing.expect(cmd.progress.progress == 100); +} + +test "OSC: OSC9 progress set single digit" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;1;9"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .set); + try testing.expect(cmd.progress.progress == 9); +} + +test "OSC: OSC9 progress set double digit" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;1;94"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .set); + try testing.expect(cmd.progress.progress == 94); +} + +test "OSC: OSC9 progress set extra semicolon triggers desktop notification" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;1;100;"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .show_desktop_notification); + try testing.expectEqualStrings(cmd.show_desktop_notification.title, ""); + try testing.expectEqualStrings(cmd.show_desktop_notification.body, "4;1;100;"); +} + +test "OSC: OSC9 progress remove with no progress" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;0;"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .remove); + try testing.expect(cmd.progress.progress == null); +} + +test "OSC: OSC9 progress remove ignores progress" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;0;100"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .remove); + try testing.expect(cmd.progress.progress == null); +} + +test "OSC: OSC9 progress remove extra semicolon" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;0;100;"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .show_desktop_notification); + try testing.expectEqualStrings(cmd.show_desktop_notification.title, ""); + try testing.expectEqualStrings(cmd.show_desktop_notification.body, "4;0;100;"); +} + test "OSC: empty param" { const testing = std.testing; diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig index b8d60a13f..8e8be90b1 100644 --- a/src/terminal/stream.zig +++ b/src/terminal/stream.zig @@ -1447,6 +1447,10 @@ pub fn Stream(comptime Handler: type) type { return; } else log.warn("unimplemented OSC callback: {}", .{cmd}); }, + + .progress => { + log.warn("unimplemented OSC callback: {}", .{cmd}); + }, } // Fall through for when we don't have a handler. From 7027147c7695064bf86f084e316e19dc604fcaaf Mon Sep 17 00:00:00 2001 From: Jan200101 Date: Wed, 25 Dec 2024 23:28:03 +0100 Subject: [PATCH 29/48] terminal: support progress for ConEmu pause and error state --- src/terminal/osc.zig | 64 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/src/terminal/osc.zig b/src/terminal/osc.zig index 3f7236a2c..19d8212a0 100644 --- a/src/terminal/osc.zig +++ b/src/terminal/osc.zig @@ -844,8 +844,12 @@ pub const Parser = struct { // If we aren't a set substate, then we don't care // about the value. const p = &self.command.progress; - if (p.state != .set) break :value; - assert(p.progress != null); + if (p.state != .set and p.state != .@"error" and p.state != .pause) break :value; + + if (p.state == .set) + assert(p.progress != null) + else if (p.progress == null) + p.progress = 0; // If we're over 100% we're done. if (p.progress.? >= 100) break :value; @@ -1770,6 +1774,62 @@ test "OSC: OSC9 progress remove extra semicolon" { try testing.expectEqualStrings(cmd.show_desktop_notification.body, "4;0;100;"); } +test "OSC: OSC9 progress error" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;2"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .@"error"); + try testing.expect(cmd.progress.progress == null); +} + +test "OSC: OSC9 progress error with progress" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;2;100"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .@"error"); + try testing.expect(cmd.progress.progress == 100); +} + +test "OSC: OSC9 progress pause" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;4"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .pause); + try testing.expect(cmd.progress.progress == null); +} + +test "OSC: OSC9 progress pause with progress" { + const testing = std.testing; + + var p: Parser = .{}; + + const input = "9;4;4;100"; + for (input) |ch| p.next(ch); + + const cmd = p.end('\x1b').?; + try testing.expect(cmd == .progress); + try testing.expect(cmd.progress.state == .pause); + try testing.expect(cmd.progress.progress == 100); +} + test "OSC: empty param" { const testing = std.testing; From b89e87c80da7f70b764db8f1cdc485c11e142bb2 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Dec 2024 18:58:42 -0500 Subject: [PATCH 30/48] test: add failing test for `Screen.cursorAbsolute` memory corruption --- src/terminal/Screen.zig | 75 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 97a1f56dd..1a8e4b75d 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -3764,6 +3764,81 @@ test "Screen: cursorAbsolute across pages preserves style" { } } +test "Screen: cursorAbsolute to page with insufficient capacity" { + // This test checks for a very specific edge case + // which previously resulted in memory corruption. + // + // The conditions for this edge case are as such: + // - The cursor has an associated style or other managed memory. + // - The cursor moves to a different page. + // - The new page is at capacity and must have its capacity adjusted. + + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 3, 1); + defer s.deinit(); + + // Scroll down enough to go to another page + const start_page = &s.pages.pages.last.?.data; + const rem = start_page.capacity.rows; + start_page.pauseIntegrityChecks(true); + for (0..rem) |_| try s.cursorDownOrScroll(); + start_page.pauseIntegrityChecks(false); + + const new_page = &s.cursor.page_pin.node.data; + + // We need our page to change for this test to make sense. If this + // assertion fails then the bug is in the test: we should be scrolling + // above enough for a new page to show up. + try testing.expect(start_page != new_page); + + // Add styles to the start page until it reaches capacity. + { + // Pause integrity checks because they're slow and + // we're not testing this, this is just setup. + start_page.pauseIntegrityChecks(true); + defer start_page.pauseIntegrityChecks(false); + defer start_page.assertIntegrity(); + + var n: u24 = 1; + while (start_page.styles.add( + start_page.memory, + .{ .bg_color = .{ .rgb = @bitCast(n) } }, + )) |_| n += 1 else |_| {} + } + + // Set a style on the cursor. + try s.setAttribute(.{ .bold = {} }); + { + const styleval = new_page.styles.get( + new_page.memory, + s.cursor.style_id, + ); + try testing.expect(styleval.flags.bold); + } + + // Go back up into the start page and we should still have that style. + s.cursorAbsolute(1, 1); + { + const cur_page = &s.cursor.page_pin.node.data; + // The page we're on now should NOT equal start_page, since its + // capacity should have been adjusted, which invalidates our ptr. + try testing.expect(start_page != cur_page); + // To make sure we DID change pages we check we're not on new_page. + try testing.expect(new_page != cur_page); + + const styleval = cur_page.styles.get( + cur_page.memory, + s.cursor.style_id, + ); + try testing.expect(styleval.flags.bold); + } + + s.cursor.page_pin.node.data.assertIntegrity(); + new_page.assertIntegrity(); +} + test "Screen: scrolling" { const testing = std.testing; const alloc = testing.allocator; From 8f0dcb9d913df5ce62eb84969ff23ad876711bd1 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 24 Dec 2024 21:24:15 -0500 Subject: [PATCH 31/48] fix: memory corruption in `Screen.cursorAbsolute` We call `cursorChangePin` which may invalidate the provided pin if it needs to adjust the page capacity, and as such we should consider the pin we pass in to it invalid afterwards, and access it through cursor instead. --- src/terminal/Screen.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 1a8e4b75d..93958d427 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -620,7 +620,7 @@ pub fn cursorAbsolute(self: *Screen, x: size.CellCountInt, y: size.CellCountInt) self.cursor.x = x; // Must be set before cursorChangePin self.cursor.y = y; self.cursorChangePin(page_pin); - const page_rac = page_pin.rowAndCell(); + const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; } From a387e680ed440fc435b650386c503be4801fdf7d Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Dec 2024 20:06:48 -0500 Subject: [PATCH 32/48] test: add additional checks to scroll above cross-page test Reveals a memory corruption issue caused by the direct assignment of `cursor.page_pin` instead of using the `cursorChangePin` method. --- src/terminal/Screen.zig | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 97a1f56dd..52fba95ef 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -4477,6 +4477,15 @@ test "Screen: scroll above same page but cursor on previous page last row" { try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); + + // Attempt to clear the style from the cursor and + // then assert the integrity of both of our pages. + // + // This catches a case of memory corruption where the cursor + // is moved between pages without accounting for style refs. + try s.setAttribute(.{ .reset_bg = {} }); + s.pages.pages.first.?.data.assertIntegrity(); + s.pages.pages.last.?.data.assertIntegrity(); } test "Screen: scroll above creates new page" { From 6d034d04a0eedc6126f0c8cfa447ccf08ddd1949 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 24 Dec 2024 22:47:38 -0500 Subject: [PATCH 33/48] fix: memory corruption in `Screen.cursorScrollAboveRotate` Unless it's guaranteed that the new pin is in the same page as the old one, `cursor.page_pin` should only be updated through `cursorChangePin`, not directly. --- src/terminal/Screen.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 52fba95ef..57818b351 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -863,7 +863,7 @@ pub fn cursorScrollAbove(self: *Screen) !void { } fn cursorScrollAboveRotate(self: *Screen) !void { - self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; + self.cursorChangePin(self.cursor.page_pin.down(1).?); // Go through each of the pages following our pin, shift all rows // down by one, and copy the last row of the previous page. From b68c161d98887882104ca855416396a6b653615e Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Dec 2024 20:30:24 -0500 Subject: [PATCH 34/48] improve comments The new diagrams are different due to changes that have happened since they were last generated. --- src/terminal/Screen.zig | 122 +++++++++++++++++++++++++++++++++++----- 1 file changed, 109 insertions(+), 13 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 57818b351..d54bbbdad 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -779,8 +779,8 @@ pub fn cursorDownScroll(self: *Screen) !void { } } -/// This scrolls the active area at and above the cursor. The lines below -/// the cursor are not scrolled. +/// This scrolls the active area at and above the cursor. +/// The lines below the cursor are not scrolled. pub fn cursorScrollAbove(self: *Screen) !void { // If the cursor is on the bottom of the screen, its faster to use // our specialized function for that case. @@ -793,6 +793,14 @@ pub fn cursorScrollAbove(self: *Screen) !void { // Logic below assumes we always have at least one row that isn't moving assert(self.cursor.y < self.pages.rows - 1); + // Explanation: + // We don't actually move everything that's at or above the cursor row, + // since this would require us to shift up our ENTIRE scrollback, which + // would be ridiculously expensive. Instead, we insert a new row at the + // end of the pagelist (`grow()`), and move everything BELOW the cursor + // DOWN by one row. This has the same practical result but it's a whole + // lot cheaper in 99% of cases. + const old_pin = self.cursor.page_pin.*; if (try self.pages.grow()) |_| { try self.cursorScrollAboveRotate(); @@ -803,6 +811,9 @@ pub fn cursorScrollAbove(self: *Screen) !void { // If we're on the last page we can do a very fast path because // all the rows we need to move around are within a single page. + // Note: we don't need to call cursorChangePin here because + // the pin page is the same so there is no accounting to do + // for styles or any of that. assert(old_pin.node == self.cursor.page_pin.node); self.cursor.page_pin.* = self.cursor.page_pin.down(1).?; @@ -823,10 +834,6 @@ pub fn cursorScrollAbove(self: *Screen) !void { const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; - - // Note: we don't need to call cursorChangePin here because - // the pin page is the same so there is no accounting to do for - // styles or any of that. } else { // We didn't grow pages but our cursor isn't on the last page. // In this case we need to do more work because we need to copy @@ -4317,8 +4324,31 @@ test "Screen: scroll above same page" { try s.testWriteString("1ABCD\n2EFGH\n3IJKL"); s.cursorAbsolute(0, 1); s.pages.clearDirty(); + + // At this point: + // +-------------+ ACTIVE + // +----------+ : = PAGE 0 + // 0 |1ABCD00000| | 0 + // 1 |2EFGH00000| | 1 + // :^ : : = PIN 0 + // 2 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ + try s.cursorScrollAbove(); + // +----------+ = PAGE 0 + // 0 |1ABCD00000| + // +-------------+ ACTIVE + // 1 |2EFGH00000| | 0 + // 2 | | | 1 + // :^ : : = PIN 0 + // 3 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ + + // try s.pages.diagram(std.io.getStdErr().writer()); + { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); defer alloc.free(contents); @@ -4368,8 +4398,25 @@ test "Screen: scroll above same page but cursor on previous page" { // +----------+ = PAGE 0 // ... : : // +-------------+ ACTIVE - // 4303 |1A00000000| | 0 - // 4304 |2B00000000| | 1 + // 4305 |1A00000000| | 0 + // 4306 |2B00000000| | 1 + // :^ : : = PIN 0 + // 4307 |3C00000000| | 2 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |4D00000000| | 3 + // 1 |5E00000000| | 4 + // +----------+ : + // +-------------+ + + try s.cursorScrollAbove(); + + // +----------+ = PAGE 0 + // ... : : + // 4305 |1A00000000| + // +-------------+ ACTIVE + // 4306 |2B00000000| | 0 + // 4307 | | | 1 // :^ : : = PIN 0 // +----------+ : // +----------+ : = PAGE 1 @@ -4379,7 +4426,7 @@ test "Screen: scroll above same page but cursor on previous page" { // +----------+ : // +-------------+ - try s.cursorScrollAbove(); + // try s.pages.diagram(std.io.getStdErr().writer()); { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); @@ -4428,8 +4475,8 @@ test "Screen: scroll above same page but cursor on previous page last row" { // +----------+ = PAGE 0 // ... : : // +-------------+ ACTIVE - // 4303 |1A00000000| | 0 - // 4304 |2B00000000| | 1 + // 4306 |1A00000000| | 0 + // 4307 |2B00000000| | 1 // :^ : : = PIN 0 // +----------+ : // +----------+ : = PAGE 1 @@ -4443,9 +4490,9 @@ test "Screen: scroll above same page but cursor on previous page last row" { // +----------+ = PAGE 0 // ... : : - // 4303 |1A00000000| + // 4306 |1A00000000| // +-------------+ ACTIVE - // 4304 |2B00000000| | 0 + // 4307 |2B00000000| | 0 // +----------+ : // +----------+ : = PAGE 1 // 0 | | | 1 @@ -4508,8 +4555,34 @@ test "Screen: scroll above creates new page" { // Ensure we're still on the first page try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + + // At this point: + // +----------+ = PAGE 0 + // ... : : + // +-------------+ ACTIVE + // 4305 |1ABCD00000| | 0 + // 4306 |2EFGH00000| | 1 + // :^ : : = PIN 0 + // 4307 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ try s.cursorScrollAbove(); + // +----------+ = PAGE 0 + // ... : : + // 4305 |1ABCD00000| + // +-------------+ ACTIVE + // 4306 |2EFGH00000| | 0 + // 4307 | | | 1 + // :^ : : = PIN 0 + // +----------+ : + // +----------+ : = PAGE 1 + // 0 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ + + // try s.pages.diagram(std.io.getStdErr().writer()); + { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); defer alloc.free(contents); @@ -4548,8 +4621,31 @@ test "Screen: scroll above no scrollback bottom of page" { try s.testWriteString("1ABCD\n2EFGH\n3IJKL"); s.cursorAbsolute(0, 1); s.pages.clearDirty(); + + // At this point: + // +-------------+ ACTIVE + // +----------+ : = PAGE 0 + // 0 |1ABCD00000| | 0 + // 1 |2EFGH00000| | 1 + // :^ : : = PIN 0 + // 2 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ + try s.cursorScrollAbove(); + // +----------+ = PAGE 0 + // 0 |1ABCD00000| + // +-------------+ ACTIVE + // 1 |2EFGH00000| | 0 + // 2 | | | 1 + // :^ : : = PIN 0 + // 3 |3IJKL00000| | 2 + // +----------+ : + // +-------------+ + + //try s.pages.diagram(std.io.getStdErr().writer()); + { const contents = try s.dumpStringAlloc(alloc, .{ .viewport = .{} }); defer alloc.free(contents); From 67f51dcffdfb3500473c8a1eafe06bb38d756b15 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Dec 2024 21:06:59 -0500 Subject: [PATCH 35/48] test: correct scroll above dirty assertions Accounts for improved behavior due to prior memory corruption fix for `cursorScrollAboveRotate` and reveals a new problem in a different `cursorScrollAbove` sub-function. --- src/terminal/Screen.zig | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index d54bbbdad..16dec7230 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -4365,10 +4365,11 @@ test "Screen: scroll above same page" { }, cell.content.color_rgb); } - // Only y=1,2 are dirty because they are the ones that CHANGED contents - // (not just scroll). - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0 row 1 (active row 0) is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0 row 2 (active row 1) is dirty because it was cleared. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + // Page 0 row 3 (active row 2) is dirty because it's new. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } @@ -4444,9 +4445,13 @@ test "Screen: scroll above same page but cursor on previous page" { }, cell.content.color_rgb); } - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0's penultimate row is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // The rest of the rows are dirty because they've been modified or are new. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 3 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 4 } })); } test "Screen: scroll above same page but cursor on previous page last row" { @@ -4521,9 +4526,13 @@ test "Screen: scroll above same page but cursor on previous page last row" { }, cell.content.color_rgb); } - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0's final row is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 1's rows are all dirty because every row was moved. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 3 } })); + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 4 } })); // Attempt to clear the style from the cursor and // then assert the integrity of both of our pages. @@ -4599,9 +4608,11 @@ test "Screen: scroll above creates new page" { }, cell.content.color_rgb); } - // Only y=1 is dirty because they are the ones that CHANGED contents - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0's penultimate row is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0's final row is dirty because it was cleared. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + // Page 1's row is dirty because it's new. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } @@ -4662,10 +4673,11 @@ test "Screen: scroll above no scrollback bottom of page" { }, cell.content.color_rgb); } - // Only y=1,2 are dirty because they are the ones that CHANGED contents - // (not just scroll). - try testing.expect(!s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0 row 1 (active row 0) is dirty because the cursor moved off of it. + try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 0 } })); + // Page 0 row 2 (active row 1) is dirty because it was cleared. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 1 } })); + // Page 0 row 3 (active row 2) is dirty because it is new. try testing.expect(s.pages.isDirty(.{ .active = .{ .x = 0, .y = 2 } })); } From d030155c8144a45124781665e37fa69b62f11b9c Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Wed, 25 Dec 2024 21:15:58 -0500 Subject: [PATCH 36/48] fix: unconditionally mark cursor row dirty in `cursorScrollAbove` explained in comment --- src/terminal/Screen.zig | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 16dec7230..00e8a8d35 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -782,6 +782,12 @@ pub fn cursorDownScroll(self: *Screen) !void { /// This scrolls the active area at and above the cursor. /// The lines below the cursor are not scrolled. pub fn cursorScrollAbove(self: *Screen) !void { + // We unconditionally mark the cursor row as dirty here because + // the cursor always changes page rows inside this function, and + // when that happens it can mean the text in the old row needs to + // be re-shaped because the cursor splits runs to break ligatures. + self.cursor.page_pin.markDirty(); + // If the cursor is on the bottom of the screen, its faster to use // our specialized function for that case. if (self.cursor.y == self.pages.rows - 1) { From a38acbc11c0ce3ae86e129fc9021d6dd2e8bff7a Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Wed, 25 Dec 2024 23:00:20 -0600 Subject: [PATCH 37/48] apprt/gtk: handle nullable event from event controller gtk_event_controller_get_current_event() is documented to possibly return NULL. Fixes: https://github.com/ghostty-org/ghostty/issues/2022 Fixes: https://github.com/ghostty-org/ghostty/issues/3088 Link: https://docs.gtk.org/gtk4/method.EventController.get_current_event.html Signed-off-by: Tristan Partin --- src/apprt/gtk/ImguiWidget.zig | 3 ++- src/apprt/gtk/Surface.zig | 22 ++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/apprt/gtk/ImguiWidget.zig b/src/apprt/gtk/ImguiWidget.zig index d78ed28a7..1f42f0b49 100644 --- a/src/apprt/gtk/ImguiWidget.zig +++ b/src/apprt/gtk/ImguiWidget.zig @@ -386,7 +386,8 @@ fn keyEvent( // Try to process the event as text const event = c.gtk_event_controller_get_current_event(@ptrCast(ec_key)); - _ = c.gtk_im_context_filter_keypress(self.im_context, event); + if (event != null) + _ = c.gtk_im_context_filter_keypress(self.im_context, event); return true; } diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 3ad695909..3d80d9259 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -1341,8 +1341,9 @@ fn gtkMouseDown( y: c.gdouble, ud: ?*anyopaque, ) callconv(.C) void { + const event = c.gtk_event_controller_get_current_event(@ptrCast(gesture)) orelse return; + const self = userdataSelf(ud.?); - const event = c.gtk_event_controller_get_current_event(@ptrCast(gesture)); const gtk_mods = c.gdk_event_get_modifier_state(event); const button = translateMouseButton(c.gtk_gesture_single_get_current_button(@ptrCast(gesture))); @@ -1374,7 +1375,8 @@ fn gtkMouseUp( _: c.gdouble, ud: ?*anyopaque, ) callconv(.C) void { - const event = c.gtk_event_controller_get_current_event(@ptrCast(gesture)); + const event = c.gtk_event_controller_get_current_event(@ptrCast(gesture)) orelse return; + const gtk_mods = c.gdk_event_get_modifier_state(event); const button = translateMouseButton(c.gtk_gesture_single_get_current_button(@ptrCast(gesture))); @@ -1393,6 +1395,8 @@ fn gtkMouseMotion( y: c.gdouble, ud: ?*anyopaque, ) callconv(.C) void { + const event = c.gtk_event_controller_get_current_event(@ptrCast(ec)) orelse return; + const self = userdataSelf(ud.?); const scaled = self.scaledCoordinates(x, y); @@ -1401,13 +1405,6 @@ fn gtkMouseMotion( .y = @floatCast(scaled.y), }; - // GTK can send spurious mouse movement events. Ignore them - // because this can cause actual issues: - // https://github.com/ghostty-org/ghostty/issues/2022 - if (pos.x == self.cursor_pos.x and pos.y == self.cursor_pos.y) { - return; - } - // Our pos changed, update self.cursor_pos = pos; @@ -1418,7 +1415,6 @@ fn gtkMouseMotion( } // Get our modifiers - const event = c.gtk_event_controller_get_current_event(@ptrCast(ec)); const gtk_mods = c.gdk_event_get_modifier_state(event); const mods = gtk_key.translateMods(gtk_mods); @@ -1432,10 +1428,11 @@ fn gtkMouseLeave( ec: *c.GtkEventControllerMotion, ud: ?*anyopaque, ) callconv(.C) void { + const event = c.gtk_event_controller_get_current_event(@ptrCast(ec)) orelse return; + const self = userdataSelf(ud.?); // Get our modifiers - const event = c.gtk_event_controller_get_current_event(@ptrCast(ec)); const gtk_mods = c.gdk_event_get_modifier_state(event); const mods = gtk_key.translateMods(gtk_mods); self.core_surface.cursorPosCallback(.{ .x = -1, .y = -1 }, mods) catch |err| { @@ -1536,11 +1533,12 @@ pub fn keyEvent( keycode: c.guint, gtk_mods: c.GdkModifierType, ) bool { - const keyval_unicode = c.gdk_keyval_to_unicode(keyval); const event = c.gtk_event_controller_get_current_event( @ptrCast(ec_key), ) orelse return false; + const keyval_unicode = c.gdk_keyval_to_unicode(keyval); + // Get the unshifted unicode value of the keyval. This is used // by the Kitty keyboard protocol. const keyval_unicode_unshifted: u21 = gtk_key.keyvalUnicodeUnshifted( From 4e47b2ab6b8dd2161b30d6b71f55cd0b28590a7e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 25 Dec 2024 15:06:44 -0800 Subject: [PATCH 38/48] macos: send a cursor position event on mouseEnter Fixes #3117 On mouseExit we sent a cursor position event with (-1, -1). Negative values are meant to indicate that the cursor is not on the surface. The magnitude of the values are irrelevant. However, we never reset the cursor position on mouseEnter. This has the effect of the previous cursor position being stuck outside the viewport which makes certain things such as `button` mouse reporting not report until the mouse is moved. This commit sends the correct cursor position event on mouseEnter. --- macos/Sources/Ghostty/SurfaceView_AppKit.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/macos/Sources/Ghostty/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/SurfaceView_AppKit.swift index 5576515e3..48c4249be 100644 --- a/macos/Sources/Ghostty/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/SurfaceView_AppKit.swift @@ -575,6 +575,20 @@ extension Ghostty { super.rightMouseUp(with: event) } + override func mouseEntered(with event: NSEvent) { + super.mouseEntered(with: event) + + guard let surface = self.surface else { return } + + // On mouse enter we need to reset our cursor position. This is + // super important because we set it to -1/-1 on mouseExit and + // lots of mouse logic (i.e. whether to send mouse reports) depend + // on the position being in the viewport if it is. + let pos = self.convert(event.locationInWindow, from: nil) + let mods = Ghostty.ghosttyMods(event.modifierFlags) + ghostty_surface_mouse_pos(surface, pos.x, frame.height - pos.y, mods) + } + override func mouseExited(with event: NSEvent) { guard let surface = self.surface else { return } From e56a9e3721c4de704685b42dfaaacd93b22f0f38 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 26 Dec 2024 11:45:54 -0800 Subject: [PATCH 39/48] Create LICENSE --- LICENSE | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 LICENSE diff --git a/LICENSE b/LICENSE new file mode 100644 index 000000000..14e132f55 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2024 Mitchell Hashimoto + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. From 68f09eb60d7dfeb0dc4d44b81b95dd7a2b862fde Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 26 Dec 2024 11:49:11 -0800 Subject: [PATCH 40/48] set version metadata to 1.0.0 --- build.zig.zon | 2 +- nix/package.nix | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index 30a07ba2a..5c7a7afca 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -1,6 +1,6 @@ .{ .name = "ghostty", - .version = "0.1.0", + .version = "1.0.0", .paths = .{""}, .dependencies = .{ // Zig libs diff --git a/nix/package.nix b/nix/package.nix index 302e04a0f..ce20544c8 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -110,7 +110,7 @@ in stdenv.mkDerivation (finalAttrs: { pname = "ghostty"; - version = "0.1.0"; + version = "1.0.0"; inherit src; nativeBuildInputs = [ From 4b4d4062dfed7b37424c7210d1230242c709e990 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 26 Dec 2024 12:00:16 -0800 Subject: [PATCH 41/48] build.zig: v1.0.0 --- build.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.zig b/build.zig index d642f254a..c3f73026b 100644 --- a/build.zig +++ b/build.zig @@ -43,7 +43,7 @@ comptime { } /// The version of the next release. -const app_version = std.SemanticVersion{ .major = 0, .minor = 1, .patch = 0 }; +const app_version = std.SemanticVersion{ .major = 1, .minor = 0, .patch = 0 }; pub fn build(b: *std.Build) !void { const optimize = b.standardOptimizeOption(.{}); From 6217dbebcf0e1b85099a4a00e35552c463432125 Mon Sep 17 00:00:00 2001 From: Lucas Crownover Date: Thu, 26 Dec 2024 14:58:43 -0800 Subject: [PATCH 42/48] Update help comment for backslash --- src/input/key.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/input/key.zig b/src/input/key.zig index 8fc7c6f20..eb2526593 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -308,7 +308,7 @@ pub const Key = enum(c_int) { equal, left_bracket, // [ right_bracket, // ] - backslash, // / + backslash, // \ // control up, From 35b9ceee2116331b83c0c86269394e2545070b0f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 26 Dec 2024 15:21:35 -0800 Subject: [PATCH 43/48] up the version to 1.0.1 everywhere for dev --- build.zig | 2 +- build.zig.zon | 2 +- nix/package.nix | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build.zig b/build.zig index c3f73026b..b1b992fe7 100644 --- a/build.zig +++ b/build.zig @@ -43,7 +43,7 @@ comptime { } /// The version of the next release. -const app_version = std.SemanticVersion{ .major = 1, .minor = 0, .patch = 0 }; +const app_version = std.SemanticVersion{ .major = 1, .minor = 0, .patch = 1 }; pub fn build(b: *std.Build) !void { const optimize = b.standardOptimizeOption(.{}); diff --git a/build.zig.zon b/build.zig.zon index 5c7a7afca..f214a0557 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -1,6 +1,6 @@ .{ .name = "ghostty", - .version = "1.0.0", + .version = "1.0.1", .paths = .{""}, .dependencies = .{ // Zig libs diff --git a/nix/package.nix b/nix/package.nix index ce20544c8..bfc1e47de 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -110,7 +110,7 @@ in stdenv.mkDerivation (finalAttrs: { pname = "ghostty"; - version = "1.0.0"; + version = "1.0.1"; inherit src; nativeBuildInputs = [ From cb5fbc10413420a9b98072502731b7c76c9d6299 Mon Sep 17 00:00:00 2001 From: Chip Bilbrey Date: Thu, 26 Dec 2024 16:08:22 -0800 Subject: [PATCH 44/48] Re-add nix-compat flake input Its entry in flake.lock is required for shell.nix to operate as it's been written. Hash values are restored to where they last existed. --- flake.lock | 17 +++++++++++++++++ flake.nix | 6 ++++++ 2 files changed, 23 insertions(+) diff --git a/flake.lock b/flake.lock index f517f07e4..bf678156b 100644 --- a/flake.lock +++ b/flake.lock @@ -1,5 +1,21 @@ { "nodes": { + "flake-compat": { + "flake": false, + "locked": { + "lastModified": 1696426674, + "narHash": "sha256-kvjfFW7WAETZlt09AgDn1MrtKzP7t90Vf7vypd3OL1U=", + "owner": "edolstra", + "repo": "flake-compat", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", + "type": "github" + }, + "original": { + "owner": "edolstra", + "repo": "flake-compat", + "type": "github" + } + }, "flake-utils": { "inputs": { "systems": "systems" @@ -52,6 +68,7 @@ }, "root": { "inputs": { + "flake-compat": "flake-compat", "nixpkgs-stable": "nixpkgs-stable", "nixpkgs-unstable": "nixpkgs-unstable", "zig": "zig" diff --git a/flake.nix b/flake.nix index d52f96d72..e17fffed3 100644 --- a/flake.nix +++ b/flake.nix @@ -9,6 +9,12 @@ # system glibc that the user is building for. nixpkgs-stable.url = "github:nixos/nixpkgs/release-24.11"; + # Used for shell.nix + flake-compat = { + url = "github:edolstra/flake-compat"; + flake = false; + }; + zig = { url = "github:mitchellh/zig-overlay"; inputs = { From 7aced21a8efd1377ab5e4991de5fb2280914d1b2 Mon Sep 17 00:00:00 2001 From: Anmol Wadhwani <4815989+anmolw@users.noreply.github.com> Date: Fri, 27 Dec 2024 08:29:35 +0530 Subject: [PATCH 45/48] Use doc comments for focus-follows-mouse --- src/config/Config.zig | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 941e794f8..d427d43bb 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1189,12 +1189,12 @@ keybind: Keybinds = .{}, /// value larger than this will be clamped to the maximum value. @"resize-overlay-duration": Duration = .{ .duration = 750 * std.time.ns_per_ms }, -// If true, when there are multiple split panes, the mouse selects the pane -// that is focused. This only applies to the currently focused window; i.e. -// mousing over a split in an unfocused window will now focus that split -// and bring the window to front. -// -// Default is false. +/// If true, when there are multiple split panes, the mouse selects the pane +/// that is focused. This only applies to the currently focused window; i.e. +/// mousing over a split in an unfocused window will now focus that split +/// and bring the window to front. +/// +/// Default is false. @"focus-follows-mouse": bool = false, /// Whether to allow programs running in the terminal to read/write to the From ebfa606c677052f056c9d0007e990fb8b11687f7 Mon Sep 17 00:00:00 2001 From: Felix Salcher Date: Fri, 27 Dec 2024 04:14:17 +0100 Subject: [PATCH 46/48] updated logic for grouping actions --- src/build/webgen/main_actions.zig | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/build/webgen/main_actions.zig b/src/build/webgen/main_actions.zig index 587851003..c4c323154 100644 --- a/src/build/webgen/main_actions.zig +++ b/src/build/webgen/main_actions.zig @@ -22,9 +22,18 @@ pub fn genKeybindActions(writer: anytype) !void { @setEvalBranchQuota(5_000); const fields = @typeInfo(KeybindAction).Union.fields; + + var buffer = std.ArrayList(u8).init(std.heap.page_allocator); inline for (fields) |field| { if (field.name[0] == '_') continue; + if (@hasDecl(help_strings.KeybindAction, field.name)) { + try writer.writeAll(buffer.items); + try writer.writeAll("\n"); + + buffer.clearRetainingCapacity(); + } + // Write the field name. try writer.writeAll("## `"); try writer.writeAll(field.name); @@ -37,10 +46,9 @@ pub fn genKeybindActions(writer: anytype) !void { '\n', ); while (iter.next()) |s| { - try writer.writeAll(s); - try writer.writeAll("\n"); + try buffer.appendSlice(s); + try buffer.appendSlice("\n"); } - try writer.writeAll("\n\n"); } } } From 5411c001c8175e0cce8ac99857817822b6c57c4f Mon Sep 17 00:00:00 2001 From: Felix Salcher Date: Fri, 27 Dec 2024 04:23:17 +0100 Subject: [PATCH 47/48] added doc comment --- src/build/webgen/main_actions.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/build/webgen/main_actions.zig b/src/build/webgen/main_actions.zig index c4c323154..c9eb44d8c 100644 --- a/src/build/webgen/main_actions.zig +++ b/src/build/webgen/main_actions.zig @@ -27,6 +27,7 @@ pub fn genKeybindActions(writer: anytype) !void { inline for (fields) |field| { if (field.name[0] == '_') continue; + // Write previously stored doc comment below all related actions if (@hasDecl(help_strings.KeybindAction, field.name)) { try writer.writeAll(buffer.items); try writer.writeAll("\n"); From 73367a55f6b5d9144232ddcaca7046a1c907a455 Mon Sep 17 00:00:00 2001 From: Joris Guex Date: Fri, 27 Dec 2024 11:45:42 +0800 Subject: [PATCH 48/48] Fix docs formatting --- src/cli/list_themes.zig | 10 +++++----- src/config/Config.zig | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cli/list_themes.zig b/src/cli/list_themes.zig index f78752c24..a8cc7b21c 100644 --- a/src/cli/list_themes.zig +++ b/src/cli/list_themes.zig @@ -73,11 +73,11 @@ const ThemeListElement = struct { /// /// The second directory is the `themes` subdirectory of the Ghostty resources /// directory. Ghostty ships with a multitude of themes that will be installed -/// into this directory. On macOS, this directory is the `Ghostty.app/Contents/ -/// Resources/ghostty/themes`. On Linux, this directory is the `share/ghostty/ -/// themes` (wherever you installed the Ghostty "share" directory). If you're -/// running Ghostty from the source, this is the `zig-out/share/ghostty/themes` -/// directory. +/// into this directory. On macOS, this directory is the +/// `Ghostty.app/Contents/Resources/ghostty/themes`. On Linux, this directory +/// is the `share/ghostty/themes` (wherever you installed the Ghostty "share" +/// directory). If you're running Ghostty from the source, this is the +/// `zig-out/share/ghostty/themes` directory. /// /// You can also set the `GHOSTTY_RESOURCES_DIR` environment variable to point /// to the resources directory. diff --git a/src/config/Config.zig b/src/config/Config.zig index 941e794f8..1bbcd78f8 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -351,10 +351,10 @@ const c = @cImport({ /// /// The second directory is the `themes` subdirectory of the Ghostty resources /// directory. Ghostty ships with a multitude of themes that will be installed -/// into this directory. On macOS, this list is in the `Ghostty.app/Contents/ -/// Resources/ghostty/themes` directory. On Linux, this list is in the `share/ -/// ghostty/themes` directory (wherever you installed the Ghostty "share" -/// directory. +/// into this directory. On macOS, this list is in the +/// `Ghostty.app/Contents/Resources/ghostty/themes` directory. On Linux, this +/// list is in the `share/ghostty/themes` directory (wherever you installed the +/// Ghostty "share" directory. /// /// To see a list of available themes, run `ghostty +list-themes`. ///