diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a9adcfbc2..fbac22a47 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -978,8 +978,6 @@ jobs: --check-sourced \ --color=always \ --severity=warning \ - --shell=bash \ - --external-sources \ $(find . \( -name "*.sh" -o -name "*.bash" \) -type f ! -path "./zig-out/*" ! -path "./macos/build/*" ! -path "./.git/*" | sort) translations: diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 000000000..919cc175d --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,8 @@ +# ShellCheck +# https://github.com/koalaman/shellcheck/wiki/Directive#shellcheckrc-file + +# Allow opening any 'source'd file, even if not specified as input +external-sources=true + +# Assume bash by default +shell=bash diff --git a/HACKING.md b/HACKING.md index bde50ec99..0abb3a2d8 100644 --- a/HACKING.md +++ b/HACKING.md @@ -164,6 +164,28 @@ alejandra . Make sure your Alejandra version matches the version of Alejandra in [devShell.nix](https://github.com/ghostty-org/ghostty/blob/main/nix/devShell.nix). +### ShellCheck + +Bash scripts are checked with [ShellCheck](https://www.shellcheck.net/) in CI. + +Nix users can use the following command to run ShellCheck over all of our scripts: + +``` +nix develop -c shellcheck \ + --check-sourced \ + --severity=warning \ + $(find . \( -name "*.sh" -o -name "*.bash" \) -type f ! -path "./zig-out/*" ! -path "./macos/build/*" ! -path "./.git/*" | sort) +``` + +Non-Nix users can [install ShellCheck](https://github.com/koalaman/shellcheck#user-content-installing) and then run: + +``` +shellcheck \ + --check-sourced \ + --severity=warning \ + $(find . \( -name "*.sh" -o -name "*.bash" \) -type f ! -path "./zig-out/*" ! -path "./macos/build/*" ! -path "./.git/*" | sort) +``` + ### Updating the Zig Cache Fixed-Output Derivation Hash The Nix package depends on a [fixed-output diff --git a/build.zig.zon b/build.zig.zon index c3e2de9f8..d5c06259a 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -116,8 +116,8 @@ // Other .apple_sdk = .{ .path = "./pkg/apple-sdk" }, .iterm2_themes = .{ - .url = "https://deps.files.ghostty.org/ghostty-themes-release-20251229-150532-f279991.tgz", - .hash = "N-V-__8AAIdIAwAO4ro1DOaG7QTFq3ewrTQIViIKJ3lKY6lV", + .url = "https://deps.files.ghostty.org/ghostty-themes-release-20260112-150707-28c8f5b.tgz", + .hash = "N-V-__8AAIdIAwDt5PxH-cwCxEcTfw4jBV8sR6fZ_XLh-cR7", .lazy = true, }, }, diff --git a/build.zig.zon.json b/build.zig.zon.json index 022a7401e..b12216bd9 100644 --- a/build.zig.zon.json +++ b/build.zig.zon.json @@ -54,10 +54,10 @@ "url": "https://github.com/ocornut/imgui/archive/refs/tags/v1.92.5-docking.tar.gz", "hash": "sha256-yBbCDox18+Fa6Gc1DnmSVQLRpqhZOLsac7iSfl8x+cs=" }, - "N-V-__8AAIdIAwAO4ro1DOaG7QTFq3ewrTQIViIKJ3lKY6lV": { + "N-V-__8AAIdIAwDt5PxH-cwCxEcTfw4jBV8sR6fZ_XLh-cR7": { "name": "iterm2_themes", - "url": "https://deps.files.ghostty.org/ghostty-themes-release-20251229-150532-f279991.tgz", - "hash": "sha256-bWKQxRggz/ZLr6w0Zt/hTnnAAb13VQWV70ScCsNFIZk=" + "url": "https://deps.files.ghostty.org/ghostty-themes-release-20260112-150707-28c8f5b.tgz", + "hash": "sha256-NIqF12KqXhIrP+LyBtg6WtkHxNUdWOyziAdq8S45RrU=" }, "N-V-__8AAIC5lwAVPJJzxnCAahSvZTIlG-HhtOvnM1uh-66x": { "name": "jetbrains_mono", diff --git a/build.zig.zon.nix b/build.zig.zon.nix index d845ea10e..430619e74 100644 --- a/build.zig.zon.nix +++ b/build.zig.zon.nix @@ -171,11 +171,11 @@ in }; } { - name = "N-V-__8AAIdIAwAO4ro1DOaG7QTFq3ewrTQIViIKJ3lKY6lV"; + name = "N-V-__8AAIdIAwDt5PxH-cwCxEcTfw4jBV8sR6fZ_XLh-cR7"; path = fetchZigArtifact { name = "iterm2_themes"; - url = "https://deps.files.ghostty.org/ghostty-themes-release-20251229-150532-f279991.tgz"; - hash = "sha256-bWKQxRggz/ZLr6w0Zt/hTnnAAb13VQWV70ScCsNFIZk="; + url = "https://deps.files.ghostty.org/ghostty-themes-release-20260112-150707-28c8f5b.tgz"; + hash = "sha256-NIqF12KqXhIrP+LyBtg6WtkHxNUdWOyziAdq8S45RrU="; }; } { diff --git a/build.zig.zon.txt b/build.zig.zon.txt index 0eeb7c5f1..72597a650 100644 --- a/build.zig.zon.txt +++ b/build.zig.zon.txt @@ -6,7 +6,7 @@ https://deps.files.ghostty.org/breakpad-b99f444ba5f6b98cac261cbb391d8766b34a5918 https://deps.files.ghostty.org/fontconfig-2.14.2.tar.gz https://deps.files.ghostty.org/freetype-1220b81f6ecfb3fd222f76cf9106fecfa6554ab07ec7fdc4124b9bb063ae2adf969d.tar.gz https://deps.files.ghostty.org/gettext-0.24.tar.gz -https://deps.files.ghostty.org/ghostty-themes-release-20251229-150532-f279991.tgz +https://deps.files.ghostty.org/ghostty-themes-release-20260112-150707-28c8f5b.tgz https://deps.files.ghostty.org/glslang-12201278a1a05c0ce0b6eb6026c65cd3e9247aa041b1c260324bf29cee559dd23ba1.tar.gz https://deps.files.ghostty.org/gobject-2025-11-08-23-1.tar.zst https://deps.files.ghostty.org/gtk4-layer-shell-1.1.0.tar.gz diff --git a/flatpak/zig-packages.json b/flatpak/zig-packages.json index 7749eb67e..3e2b1e26d 100644 --- a/flatpak/zig-packages.json +++ b/flatpak/zig-packages.json @@ -67,9 +67,9 @@ }, { "type": "archive", - "url": "https://deps.files.ghostty.org/ghostty-themes-release-20251229-150532-f279991.tgz", - "dest": "vendor/p/N-V-__8AAIdIAwAO4ro1DOaG7QTFq3ewrTQIViIKJ3lKY6lV", - "sha256": "6d6290c51820cff64bafac3466dfe14e79c001bd77550595ef449c0ac3452199" + "url": "https://deps.files.ghostty.org/ghostty-themes-release-20260112-150707-28c8f5b.tgz", + "dest": "vendor/p/N-V-__8AAIdIAwDt5PxH-cwCxEcTfw4jBV8sR6fZ_XLh-cR7", + "sha256": "348a85d762aa5e122b3fe2f206d83a5ad907c4d51d58ecb388076af12e3946b5" }, { "type": "archive", diff --git a/include/ghostty.h b/include/ghostty.h index b884ebc08..0133fac73 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -66,6 +66,14 @@ typedef enum { GHOSTTY_MOUSE_LEFT, GHOSTTY_MOUSE_RIGHT, GHOSTTY_MOUSE_MIDDLE, + GHOSTTY_MOUSE_FOUR, + GHOSTTY_MOUSE_FIVE, + GHOSTTY_MOUSE_SIX, + GHOSTTY_MOUSE_SEVEN, + GHOSTTY_MOUSE_EIGHT, + GHOSTTY_MOUSE_NINE, + GHOSTTY_MOUSE_TEN, + GHOSTTY_MOUSE_ELEVEN, } ghostty_input_mouse_button_e; typedef enum { diff --git a/macos/Sources/Ghostty/Ghostty.Input.swift b/macos/Sources/Ghostty/Ghostty.Input.swift index 4b3fb9937..7b2905abb 100644 --- a/macos/Sources/Ghostty/Ghostty.Input.swift +++ b/macos/Sources/Ghostty/Ghostty.Input.swift @@ -370,6 +370,14 @@ extension Ghostty.Input { case left case right case middle + case four + case five + case six + case seven + case eight + case nine + case ten + case eleven var cMouseButton: ghostty_input_mouse_button_e { switch self { @@ -377,6 +385,33 @@ extension Ghostty.Input { case .left: GHOSTTY_MOUSE_LEFT case .right: GHOSTTY_MOUSE_RIGHT case .middle: GHOSTTY_MOUSE_MIDDLE + case .four: GHOSTTY_MOUSE_FOUR + case .five: GHOSTTY_MOUSE_FIVE + case .six: GHOSTTY_MOUSE_SIX + case .seven: GHOSTTY_MOUSE_SEVEN + case .eight: GHOSTTY_MOUSE_EIGHT + case .nine: GHOSTTY_MOUSE_NINE + case .ten: GHOSTTY_MOUSE_TEN + case .eleven: GHOSTTY_MOUSE_ELEVEN + } + } + + /// Initialize from NSEvent.buttonNumber + /// NSEvent buttonNumber: 0=left, 1=right, 2=middle, 3=back (button 8), 4=forward (button 9), etc. + init(fromNSEventButtonNumber buttonNumber: Int) { + switch buttonNumber { + case 0: self = .left + case 1: self = .right + case 2: self = .middle + case 3: self = .eight // Back button + case 4: self = .nine // Forward button + case 5: self = .six + case 6: self = .seven + case 7: self = .four + case 8: self = .five + case 9: self = .ten + case 10: self = .eleven + default: self = .unknown } } } diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift index 80de2e823..0ddfe57b8 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift @@ -860,16 +860,16 @@ extension Ghostty { override func otherMouseDown(with event: NSEvent) { guard let surface = self.surface else { return } - guard event.buttonNumber == 2 else { return } let mods = Ghostty.ghosttyMods(event.modifierFlags) - ghostty_surface_mouse_button(surface, GHOSTTY_MOUSE_PRESS, GHOSTTY_MOUSE_MIDDLE, mods) + let button = Ghostty.Input.MouseButton(fromNSEventButtonNumber: event.buttonNumber) + ghostty_surface_mouse_button(surface, GHOSTTY_MOUSE_PRESS, button.cMouseButton, mods) } override func otherMouseUp(with event: NSEvent) { guard let surface = self.surface else { return } - guard event.buttonNumber == 2 else { return } let mods = Ghostty.ghosttyMods(event.modifierFlags) - ghostty_surface_mouse_button(surface, GHOSTTY_MOUSE_RELEASE, GHOSTTY_MOUSE_MIDDLE, mods) + let button = Ghostty.Input.MouseButton(fromNSEventButtonNumber: event.buttonNumber) + ghostty_surface_mouse_button(surface, GHOSTTY_MOUSE_RELEASE, button.cMouseButton, mods) } diff --git a/src/cli/ssh-cache/DiskCache.zig b/src/cli/ssh-cache/DiskCache.zig index 62620ecb0..6214d0429 100644 --- a/src/cli/ssh-cache/DiskCache.zig +++ b/src/cli/ssh-cache/DiskCache.zig @@ -9,7 +9,6 @@ const assert = @import("../../quirks.zig").inlineAssert; const Allocator = std.mem.Allocator; const internal_os = @import("../../os/main.zig"); const xdg = internal_os.xdg; -const TempDir = internal_os.TempDir; const Entry = @import("Entry.zig"); // 512KB - sufficient for approximately 10k entries @@ -125,7 +124,7 @@ pub fn add( break :update .updated; }; - try self.writeCacheFile(alloc, entries, null); + try self.writeCacheFile(entries, null); return result; } @@ -166,7 +165,7 @@ pub fn remove( alloc.free(kv.value.terminfo_version); } - try self.writeCacheFile(alloc, entries, null); + try self.writeCacheFile(entries, null); } /// Check if a hostname exists in the cache. @@ -209,32 +208,30 @@ fn fixupPermissions(file: std.fs.File) (std.fs.File.StatError || std.fs.File.Chm fn writeCacheFile( self: DiskCache, - alloc: Allocator, entries: std.StringHashMap(Entry), expire_days: ?u32, ) !void { - var td: TempDir = try .init(); - defer td.deinit(); + const cache_dir = std.fs.path.dirname(self.path) orelse return error.InvalidCachePath; + const cache_basename = std.fs.path.basename(self.path); - const tmp_file = try td.dir.createFile("ssh-cache", .{ .mode = 0o600 }); - defer tmp_file.close(); - const tmp_path = try td.dir.realpathAlloc(alloc, "ssh-cache"); - defer alloc.free(tmp_path); + var dir = try std.fs.cwd().openDir(cache_dir, .{}); + defer dir.close(); var buf: [1024]u8 = undefined; - var writer = tmp_file.writer(&buf); + var atomic_file = try dir.atomicFile(cache_basename, .{ + .mode = 0o600, + .write_buffer = &buf, + }); + defer atomic_file.deinit(); + var iter = entries.iterator(); while (iter.next()) |kv| { // Only write non-expired entries if (kv.value_ptr.isExpired(expire_days)) continue; - try kv.value_ptr.format(&writer.interface); + try kv.value_ptr.format(&atomic_file.file_writer.interface); } - // Don't forget to flush!! - try writer.interface.flush(); - - // Atomic replace - try std.fs.renameAbsolute(tmp_path, self.path); + try atomic_file.finish(); } /// List all entries in the cache. @@ -382,16 +379,16 @@ test "disk cache clear" { const alloc = testing.allocator; // Create our path - var td: TempDir = try .init(); - defer td.deinit(); + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); var buf: [4096]u8 = undefined; { - var file = try td.dir.createFile("cache", .{}); + var file = try tmp.dir.createFile("cache", .{}); defer file.close(); var file_writer = file.writer(&buf); try file_writer.interface.writeAll("HELLO!"); } - const path = try td.dir.realpathAlloc(alloc, "cache"); + const path = try tmp.dir.realpathAlloc(alloc, "cache"); defer alloc.free(path); // Setup our cache @@ -401,7 +398,7 @@ test "disk cache clear" { // Verify the file is gone try testing.expectError( error.FileNotFound, - td.dir.openFile("cache", .{}), + tmp.dir.openFile("cache", .{}), ); } @@ -410,18 +407,18 @@ test "disk cache operations" { const alloc = testing.allocator; // Create our path - var td: TempDir = try .init(); - defer td.deinit(); + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); var buf: [4096]u8 = undefined; { - var file = try td.dir.createFile("cache", .{}); + var file = try tmp.dir.createFile("cache", .{}); defer file.close(); var file_writer = file.writer(&buf); const writer = &file_writer.interface; try writer.writeAll("HELLO!"); try writer.flush(); } - const path = try td.dir.realpathAlloc(alloc, "cache"); + const path = try tmp.dir.realpathAlloc(alloc, "cache"); defer alloc.free(path); // Setup our cache @@ -453,6 +450,32 @@ test "disk cache operations" { ); } +test "disk cache cleans up temp files" { + const testing = std.testing; + const alloc = testing.allocator; + + var tmp = testing.tmpDir(.{ .iterate = true }); + defer tmp.cleanup(); + + const tmp_path = try tmp.dir.realpathAlloc(alloc, "."); + defer alloc.free(tmp_path); + const cache_path = try std.fs.path.join(alloc, &.{ tmp_path, "cache" }); + defer alloc.free(cache_path); + + const cache: DiskCache = .{ .path = cache_path }; + try testing.expectEqual(AddResult.added, try cache.add(alloc, "example.com")); + try testing.expectEqual(AddResult.added, try cache.add(alloc, "example.org")); + + // Verify only the cache file exists and no temp files left behind + var count: usize = 0; + var iter = tmp.dir.iterate(); + while (try iter.next()) |entry| { + count += 1; + try testing.expectEqualStrings("cache", entry.name); + } + try testing.expectEqual(1, count); +} + test isValidHost { const testing = std.testing; diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index fc680b971..41f8d6533 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -292,7 +292,8 @@ fn initialCapacity(cols: size.CellCountInt) Capacity { comptime { var cap = std_capacity; cap.cols = std.math.maxInt(size.CellCountInt); - _ = Page.layout(cap); + const layout = Page.layout(cap); + assert(layout.total_size <= size.max_page_size); } if (std_capacity.adjust( @@ -413,6 +414,10 @@ fn initPages( const pooled = layout.total_size <= std_size; const page_alloc = pool.pages.arena.child_allocator; + // Guaranteed by comptime checks in initialCapacity but + // redundant here for safety. + assert(layout.total_size <= size.max_page_size); + var rem = rows; while (rem > 0) { const node = try pool.nodes.create(); @@ -480,10 +485,11 @@ pub inline fn pauseIntegrityChecks(self: *PageList, pause: bool) void { } const IntegrityError = error{ + PageSerialInvalid, TotalRowsMismatch, + TrackedPinInvalid, ViewportPinOffsetMismatch, ViewportPinInsufficientRows, - PageSerialInvalid, }; /// Verify the integrity of the PageList. This is expensive and should @@ -524,6 +530,11 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void { return IntegrityError.TotalRowsMismatch; } + // Verify that all our tracked pins point to valid pages. + for (self.tracked_pins.keys()) |p| { + if (!self.pinIsValid(p.*)) return error.TrackedPinInvalid; + } + if (self.viewport == .pin) { // Verify that our viewport pin row offset is correct. const actual_offset: usize = offset: { @@ -745,8 +756,8 @@ pub fn clone( ); errdefer pool.deinit(); - // Our viewport pin is always undefined since our viewport in a clones - // goes back to the top + // Create our viewport. In a clone, the viewport always goes + // to the top. const viewport_pin = try pool.pins.create(); var tracked_pins: PinSet = .{}; errdefer tracked_pins.deinit(pool.alloc); @@ -812,6 +823,10 @@ pub fn clone( } } + // Initialize our viewport pin to point to the first cloned page + // so it points to valid memory. + viewport_pin.* = .{ .node = page_list.first.? }; + var result: PageList = .{ .pool = pool, .pages = page_list, @@ -874,7 +889,7 @@ pub const Resize = struct { /// Resize /// TODO: docs -pub fn resize(self: *PageList, opts: Resize) !void { +pub fn resize(self: *PageList, opts: Resize) Allocator.Error!void { defer self.assertIntegrity(); if (comptime std.debug.runtime_safety) { @@ -944,7 +959,7 @@ fn resizeCols( self: *PageList, cols: size.CellCountInt, cursor: ?Resize.Cursor, -) !void { +) Allocator.Error!void { assert(cols != self.cols); // Update our cols. We have to do this early because grow() that we @@ -992,32 +1007,71 @@ fn resizeCols( } else null; defer if (preserved_cursor) |c| self.untrackPin(c.tracked_pin); - const first = self.pages.first.?; - var it = self.rowIterator(.right_down, .{ .screen = .{} }, null); + // Create the first node that contains our reflow. + const first_rewritten_node = node: { + const page = &self.pages.first.?.data; + const cap = page.capacity.adjust( + .{ .cols = cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); - const dst_node = try self.createPage(try first.data.capacity.adjust(.{ .cols = cols })); - dst_node.data.size.rows = 1; + // We verify all maxed out page layouts work. + var cap = page.capacity; + cap.cols = cols; + + // We're growing columns so we can only get less rows so use + // the lesser of our capacity and size so we minimize wasted + // rows. + cap.rows = @min(page.size.rows, cap.rows); + break :err cap; + }; + + const node = try self.createPage(cap); + node.data.size.rows = 1; + break :node node; + }; + + // We need to grab our rowIterator now before we rewrite our + // linked list below. + var it = self.rowIterator( + .right_down, + .{ .screen = .{} }, + null, + ); + errdefer { + // If an error occurs, we're in a pretty disastrous broken state, + // but we should still try to clean up our leaked memory. Free + // any of the remaining orphaned pages from before. If we reflowed + // successfully this will be null. + var node_: ?*Node = if (it.chunk) |chunk| chunk.node else null; + while (node_) |node| { + node_ = node.next; + self.destroyNode(node); + } + } // Set our new page as the only page. This orphans the existing pages // in the list, but that's fine since we're gonna delete them anyway. - self.pages.first = dst_node; - self.pages.last = dst_node; + self.pages.first = first_rewritten_node; + self.pages.last = first_rewritten_node; // Reflow all our rows. { - var dst_cursor = ReflowCursor.init(dst_node); + var reflow_cursor: ReflowCursor = .init(first_rewritten_node); while (it.next()) |row| { - try dst_cursor.reflowRow(self, row); + try reflow_cursor.reflowRow(self, row); - // Once we're done reflowing a page, destroy it. + // Once we're done reflowing a page, destroy it immediately. + // This frees memory and makes it more likely in memory + // constrained environments that the next reflow will work. if (row.y == row.node.data.size.rows - 1) { self.destroyNode(row.node); } } // At the end of the reflow, setup our total row cache - // log.warn("total old={} new={}", .{ self.total_rows, dst_cursor.total_rows }); - self.total_rows = dst_cursor.total_rows; + // log.warn("total old={} new={}", .{ self.total_rows, reflow_cursor.total_rows }); + self.total_rows = reflow_cursor.total_rows; } // If our total rows is less than our active rows, we need to grow. @@ -1110,20 +1164,15 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, row: Pin, - ) !void { + ) Allocator.Error!void { const src_page: *Page = &row.node.data; const src_row = row.rowAndCell().row; const src_y = row.y; - - // Inherit increased styles or grapheme bytes from - // the src page we're reflowing from for new pages. - const cap = try src_page.capacity.adjust(.{ .cols = self.page.size.cols }); - const cells = src_row.cells.ptr(src_page.memory)[0..src_page.size.cols]; + // Calculate the columns in this row. First up we trim non-semantic + // rightmost blanks. var cols_len = src_page.size.cols; - - // If the row is wrapped, all empty cells are meaningful. if (!src_row.wrap) { while (cols_len > 0) { if (!cells[cols_len - 1].isEmpty()) break; @@ -1145,9 +1194,10 @@ const ReflowCursor = struct { // If this pin is in the blanks on the right and past the end // of the dst col width then we move it to the end of the dst // col width instead. - if (p.x >= cols_len) { - p.x = @min(p.x, cap.cols - 1 - self.x); - } + if (p.x >= cols_len) p.x = @min( + p.x, + self.page.size.cols - 1 - self.x, + ); // We increase our col len to at least include this pin. // This ensures that blank rows with pins are processed, @@ -1162,16 +1212,29 @@ const ReflowCursor = struct { // If this blank row was a wrap continuation somehow // then we won't need to write it since it should be // a part of the previously written row. - if (!src_row.wrap_continuation) { - self.new_rows += 1; - } + if (!src_row.wrap_continuation) self.new_rows += 1; return; } + // Inherit increased styles or grapheme bytes from the src page + // we're reflowing from for new pages. + const cap = src_page.capacity.adjust( + .{ .cols = self.page.size.cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); + + var cap = src_page.capacity; + cap.cols = self.page.size.cols; + // We're already a non-standard page. We don't want to + // inherit a massive set of rows, so cap it at our std size. + cap.rows = @min(src_page.size.rows, std_capacity.rows); + break :err cap; + }; + // Our row isn't blank, write any new rows we deferred. while (self.new_rows > 0) { - self.new_rows -= 1; try self.cursorScrollOrNewPage(list, cap); + self.new_rows -= 1; } self.copyRowMetadata(src_row); @@ -1199,8 +1262,93 @@ const ReflowCursor = struct { } } - const cell = &cells[x]; - x += 1; + if (self.writeCell( + list, + &cells[x], + src_page, + )) |result| switch (result) { + // Wrote the cell, move to the next. + .success => x += 1, + + // Wrote the cell but request to skip the next so skip it. + // This is used for things like spacers. + .skip_next => { + // Remap any tracked pins at the skipped position (x+1) + // since we won't process that cell in the loop. + const pin_keys = list.tracked_pins.keys(); + for (pin_keys) |p| { + if (&p.node.data != src_page or + p.y != src_y or + p.x != x + 1) continue; + + p.node = self.node; + p.x = self.x; + p.y = self.y; + } + + x += 2; + }, + + // Didn't write the cell, repeat writing this same cell. + .repeat => {}, + } else |err| switch (err) { + // System out of memory, we can't fix this. + error.OutOfMemory => return error.OutOfMemory, + + // We reached the capacity of a single page and can't + // add any more of some type of managed memory. When this + // happens we split out the current row we're working on + // into a new page and continue from there. + error.OutOfSpace => if (self.y == 0) { + // If we're already on the first-row, we can't split + // any further, so we just ignore bad cells and take + // corrupted (but valid) cell contents. + log.warn("reflowRow OutOfSpace on first row, discarding cell managed memory", .{}); + x += 1; + self.cursorForward(); + } else { + // Move our last row to a new page. + try self.moveLastRowToNewPage(list, cap); + + // Do NOT increment x so that we retry writing + // the same existing cell. + }, + } + } + + // If the source row isn't wrapped then we should scroll afterwards. + if (!src_row.wrap) { + self.new_rows += 1; + } + } + + /// Write a cell. On error, this will not unwrite the cell but + /// the cell may be incomplete (but valid). For example, if the source + /// cell is styled and we failed to allocate space for styles, the + /// written cell may not be styled but it is valid. + /// + /// The key failure to recognize for callers is when we can't increase + /// capacity in our destination page. In this case, the caller may want + /// to split the page at this row, rewrite the row into a new page + /// and continue from there. + /// + /// But this function guarantees the terminal/page will be in a + /// coherent state even on error. + fn writeCell( + self: *ReflowCursor, + list: *PageList, + cell: *const pagepkg.Cell, + src_page: *const Page, + ) IncreaseCapacityError!enum { + success, + repeat, + skip_next, + } { + // Initialize self.page_cell with basic, unmanaged memory contents. + { + // This must not fail because we want to make sure we atomically + // setup our page cell to be valid. + errdefer comptime unreachable; // Copy cell contents. switch (cell.content_tag) { @@ -1220,16 +1368,15 @@ const ReflowCursor = struct { .wide = .spacer_head, }; - // Decrement the source position so that when we - // loop we'll process this source cell again, - // since we can't copy it into a spacer head. - x -= 1; - // Move to the next row (this sets pending wrap // which will cause us to wrap on the next // iteration). self.cursorForward(); - continue; + + // Decrement the source position so that when we + // loop we'll process this source cell again, + // since we can't copy it into a spacer head. + return .repeat; } else { self.page_cell.* = cell.*; } @@ -1240,9 +1387,9 @@ const ReflowCursor = struct { self.page_cell.content.codepoint = 0; self.page_cell.wide = .narrow; self.cursorForward(); + // Skip spacer tail so it doesn't cause a wrap. - x += 1; - continue; + return .skip_next; }, .spacer_tail => if (self.page.size.cols > 1) { @@ -1252,14 +1399,14 @@ const ReflowCursor = struct { // characters are just destroyed and replaced // with empty narrow cells, so we should just // discard any spacer tails. - continue; + return .success; }, .spacer_head => { // Spacer heads should be ignored. If we need a // spacer head in our reflowed page, it is added // when processing the wide cell it belongs to. - continue; + return .success; }, }, @@ -1270,7 +1417,7 @@ const ReflowCursor = struct { // data associated with them so we can fast path them. self.page_cell.* = cell.*; self.cursorForward(); - continue; + return .success; }, } @@ -1281,185 +1428,279 @@ const ReflowCursor = struct { self.page_cell.hyperlink = false; self.page_cell.style_id = stylepkg.default_id; - // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{ - // src_y, - // x, - // self.y, - // self.x, - // self.page.size.cols, - // cell.content.codepoint, - // cell.wide, - // self.page_cell.wide, - // }); - - // Copy grapheme data. - if (cell.content_tag == .codepoint_grapheme) { - // Copy the graphemes - const cps = src_page.lookupGrapheme(cell).?; - - // If our page can't support an additional cell - // with graphemes then we increase capacity. - if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { - try self.adjustCapacity(list, .{ - .grapheme_bytes = cap.grapheme_bytes * 2, - }); - } - - // Attempt to allocate the space that would be required - // for these graphemes, and if it's not available, then - // increase capacity. - if (self.page.grapheme_alloc.alloc( - u21, - self.page.memory, - cps.len, - )) |slice| { - self.page.grapheme_alloc.free(self.page.memory, slice); - } else |_| { - // Grow our capacity until we can - // definitely fit the extra bytes. - const required = cps.len * @sizeOf(u21); - var new_grapheme_capacity: usize = cap.grapheme_bytes; - while (new_grapheme_capacity - cap.grapheme_bytes < required) { - new_grapheme_capacity *= 2; - } - try self.adjustCapacity(list, .{ - .grapheme_bytes = new_grapheme_capacity, - }); - } - - // This shouldn't fail since we made sure we have space above. - try self.page.setGraphemes(self.page_row, self.page_cell, cps); - } - - // Copy hyperlink data. - if (cell.hyperlink) { - const src_id = src_page.lookupHyperlink(cell).?; - const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); - - // If our page can't support an additional cell - // with a hyperlink then we increase capacity. - if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { - try self.adjustCapacity(list, .{ - .hyperlink_bytes = cap.hyperlink_bytes * 2, - }); - } - - // Ensure that the string alloc has sufficient capacity - // to dupe the link (and the ID if it's not implicit). - const additional_required_string_capacity = - src_link.uri.len + - switch (src_link.id) { - .explicit => |v| v.len, - .implicit => 0, - }; - if (self.page.string_alloc.alloc( - u8, - self.page.memory, - additional_required_string_capacity, - )) |slice| { - // We have enough capacity, free the test alloc. - self.page.string_alloc.free(self.page.memory, slice); - } else |_| { - // Grow our capacity until we can - // definitely fit the extra bytes. - var new_string_capacity: usize = cap.string_bytes; - while (new_string_capacity - cap.string_bytes < additional_required_string_capacity) { - new_string_capacity *= 2; - } - try self.adjustCapacity(list, .{ - .string_bytes = new_string_capacity, - }); - } - - const dst_id = self.page.hyperlink_set.addWithIdContext( - self.page.memory, - // We made sure there was enough capacity for this above. - try src_link.dupe(src_page, self.page), - src_id, - .{ .page = self.page }, - ) catch |err| id: { - // If the add failed then either the set needs to grow - // or it needs to be rehashed. Either one of those can - // be accomplished by adjusting capacity, either with - // no actual change or with an increased hyperlink cap. - try self.adjustCapacity(list, switch (err) { - error.OutOfMemory => .{ - .hyperlink_bytes = cap.hyperlink_bytes * 2, - }, - error.NeedsRehash => .{}, - }); - - // We assume this one will succeed. We dupe the link - // again, and don't have to worry about the other one - // because adjusting the capacity naturally clears up - // any managed memory not associated with a cell yet. - break :id try self.page.hyperlink_set.addWithIdContext( - self.page.memory, - try src_link.dupe(src_page, self.page), - src_id, - .{ .page = self.page }, - ); - } orelse src_id; - - // We expect this to succeed due to the - // hyperlinkCapacity check we did before. - try self.page.setHyperlink( - self.page_row, - self.page_cell, - dst_id, - ); - } - - // Copy style data. - if (cell.hasStyling()) { - const style = src_page.styles.get( - src_page.memory, - cell.style_id, - ).*; - - const id = self.page.styles.addWithId( - self.page.memory, - style, - cell.style_id, - ) catch |err| id: { - // If the add failed then either the set needs to grow - // or it needs to be rehashed. Either one of those can - // be accomplished by adjusting capacity, either with - // no actual change or with an increased style cap. - try self.adjustCapacity(list, switch (err) { - error.OutOfMemory => .{ - .styles = cap.styles * 2, - }, - error.NeedsRehash => .{}, - }); - - // We assume this one will succeed. - break :id try self.page.styles.addWithId( - self.page.memory, - style, - cell.style_id, - ); - } orelse cell.style_id; - - self.page_row.styled = true; - - self.page_cell.style_id = id; - } - if (comptime build_options.kitty_graphics) { // Copy Kitty virtual placeholder status if (cell.codepoint() == kitty.graphics.unicode.placeholder) { self.page_row.kitty_virtual_placeholder = true; } } - - self.cursorForward(); } - // If the source row isn't wrapped then we should scroll afterwards. - if (!src_row.wrap) { - self.new_rows += 1; + // std.log.warn("\nsrc_y={} src_x={} dst_y={} dst_x={} dst_cols={} cp={X} wide={} page_cell_wide={}", .{ + // src_y, + // x, + // self.y, + // self.x, + // self.page.size.cols, + // cell.content.codepoint, + // cell.wide, + // self.page_cell.wide, + // }); + + // From this point on we're moving on to failable, managed memory. + // If we reach an error, we do the minimal cleanup necessary to + // not leave dangling memory but otherwise we gracefully degrade + // into some functional but not strictly correct cell. + + // Copy grapheme data. + if (cell.content_tag == .codepoint_grapheme) { + // Copy the graphemes + const cps = src_page.lookupGrapheme(cell).?; + + // If our page can't support an additional cell + // with graphemes then we increase capacity. + if (self.page.graphemeCount() >= self.page.graphemeCapacity()) { + try self.increaseCapacity( + list, + .grapheme_bytes, + ); + } + + // Attempt to allocate the space that would be required + // for these graphemes, and if it's not available, then + // increase capacity. Keep trying until we succeed. + while (true) { + if (self.page.grapheme_alloc.alloc( + u21, + self.page.memory, + cps.len, + )) |slice| { + self.page.grapheme_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity(list, .grapheme_bytes); + } + } + + self.page.setGraphemes( + self.page_row, + self.page_cell, + cps, + ) catch |err| { + // This shouldn't fail since we made sure we have space + // above. There is no reasonable behavior we can take here + // so we have a warn level log. This is ALMOST non-recoverable, + // though we choose to recover by corrupting the cell + // to a non-grapheme codepoint. + log.err("setGraphemes failed after capacity increase err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away grapheme data! + self.page_cell.content_tag = .codepoint; + self.page_cell.content = .{ .codepoint = 0xFFFD }; + }; } + + // Copy hyperlink data. + if (cell.hyperlink) hyperlink: { + const src_id = src_page.lookupHyperlink(cell).?; + const src_link = src_page.hyperlink_set.get(src_page.memory, src_id); + + // If our page can't support an additional cell + // with a hyperlink then we increase capacity. + if (self.page.hyperlinkCount() >= self.page.hyperlinkCapacity()) { + try self.increaseCapacity(list, .hyperlink_bytes); + } + + // Ensure that the string alloc has sufficient capacity + // to dupe the link (and the ID if it's not implicit). + const additional_required_string_capacity = + src_link.uri.len + + switch (src_link.id) { + .explicit => |v| v.len, + .implicit => 0, + }; + // Keep trying until we have enough capacity. + while (true) { + if (self.page.string_alloc.alloc( + u8, + self.page.memory, + additional_required_string_capacity, + )) |slice| { + // We have enough capacity, free the test alloc. + self.page.string_alloc.free( + self.page.memory, + slice, + ); + break; + } else |_| { + // Grow our capacity until we can fit the extra bytes. + try self.increaseCapacity( + list, + .string_bytes, + ); + } + } + + const dst_link = src_link.dupe( + src_page, + self.page, + ) catch |err| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + break :hyperlink; + }; + + const dst_id = self.page.hyperlink_set.addWithIdContext( + self.page.memory, + dst_link, + src_id, + .{ .page = self.page }, + ) catch |err| id: { + // Always free our original link in case the increaseCap + // call fails so we aren't leaking memory. + dst_link.free(self.page); + + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by increasing capacity, either with + // no actual change or with an increased hyperlink cap. + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .hyperlink_bytes, + error.NeedsRehash => null, + }); + + // We need to recreate the link into the new page. + const dst_link2 = src_link.dupe( + src_page, + self.page, + ) catch |err2| { + // This shouldn't fail since we did a capacity + // check above. + log.err("link dupe failed with capacity check err={}", .{err2}); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + break :hyperlink; + }; + + // We assume this one will succeed. We dupe the link + // again, and don't have to worry about the other one + // because increasing the capacity naturally clears up + // any managed memory not associated with a cell yet. + break :id self.page.hyperlink_set.addWithIdContext( + self.page.memory, + dst_link2, + src_id, + .{ .page = self.page }, + ) catch |err2| { + // This shouldn't happen since we increased capacity + // above so we handle it like the other similar + // cases and log it, crash in safe builds, and + // remove the hyperlink in unsafe builds. + log.err( + "addWithIdContext failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + dst_link2.free(self.page); + break :hyperlink; + }; + } orelse src_id; + + // We expect this to succeed due to the hyperlinkCapacity + // check we did before. If it doesn't succeed let's + // log it, crash (in safe builds), and clear our state. + self.page.setHyperlink( + self.page_row, + self.page_cell, + dst_id, + ) catch |err| { + log.err( + "setHyperlink failed after capacity increase err={}", + .{err}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + // Unsafe builds we throw away hyperlink data! + self.page.hyperlink_set.release(self.page.memory, dst_id); + self.page_cell.hyperlink = false; + break :hyperlink; + }; + } + + // Copy style data. + if (cell.hasStyling()) style: { + const style = src_page.styles.get( + src_page.memory, + cell.style_id, + ).*; + + const id = self.page.styles.addWithId( + self.page.memory, + style, + cell.style_id, + ) catch |err| id: { + // If the add failed then either the set needs to grow + // or it needs to be rehashed. Either one of those can + // be accomplished by increasing capacity, either with + // no actual change or with an increased style cap. + try self.increaseCapacity(list, switch (err) { + error.OutOfMemory => .styles, + error.NeedsRehash => null, + }); + + // We assume this one will succeed. + break :id self.page.styles.addWithId( + self.page.memory, + style, + cell.style_id, + ) catch |err2| { + // Should not fail since we just modified capacity + // above. Log it, crash in safe builds, clear style + // in unsafe builds. + log.err( + "addWithId failed after capacity increase err={}", + .{err2}, + ); + if (comptime std.debug.runtime_safety) { + // Force a crash with safe builds. + unreachable; + } + + self.page_cell.style_id = stylepkg.default_id; + break :style; + }; + } orelse cell.style_id; + + self.page_row.styled = true; + self.page_cell.style_id = id; + } + + self.cursorForward(); + return .success; } /// Create a new page in the provided list with the provided @@ -1478,7 +1719,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { assert(self.y == self.page.size.rows - 1); assert(!self.pending_wrap); @@ -1487,16 +1728,50 @@ const ReflowCursor = struct { const old_row = self.page_row; const old_x = self.x; + // Our total row count never changes, because we're removing one + // row from the last page and moving it into a new page. + const old_total_rows = self.total_rows; + defer self.total_rows = old_total_rows; + try self.cursorNewPage(list, cap); + assert(self.node != old_node); + assert(self.y == 0); + + // We have no cleanup for our old state from here on out. No failures! + errdefer comptime unreachable; // Restore the x position of the cursor. self.cursorAbsolute(old_x, 0); - // We expect to have enough capacity to clone the row. - try self.page.cloneRowFrom(old_page, self.page_row, old_row); + // Copy our old data. This should NOT fail because we have the + // capacity of the old page which already fits the data we requested. + self.page.cloneRowFrom( + old_page, + self.page_row, + old_row, + ) catch |err| { + log.err( + "error cloning single row for moveLastRowToNewPage err={}", + .{err}, + ); + @panic("unexpected copy row failure"); + }; + + // Move any tracked pins from that last row into this new node. + { + const pin_keys = list.tracked_pins.keys(); + for (pin_keys) |p| { + if (&p.node.data != old_page or + p.y != old_page.size.rows - 1) continue; + + p.node = self.node; + p.y = self.y; + // p.x remains the same since we're copying the row as-is + } + } // Clear the row from the old page and truncate it. - old_page.clearCells(old_row, 0, self.page.size.cols); + old_page.clearCells(old_row, 0, old_page.size.cols); old_page.size.rows -= 1; // If that was the last row in that page @@ -1507,27 +1782,31 @@ const ReflowCursor = struct { } } - /// Adjust the capacity of the current page. - fn adjustCapacity( + /// Increase the capacity of the current page. + fn increaseCapacity( self: *ReflowCursor, list: *PageList, - adjustment: AdjustCapacity, - ) !void { + adjustment: ?IncreaseCapacity, + ) IncreaseCapacityError!void { const old_x = self.x; const old_y = self.y; const old_total_rows = self.total_rows; - self.* = .init(node: { + const node = node: { // Pause integrity checks because the total row count won't // be correct during a reflow. list.pauseIntegrityChecks(true); defer list.pauseIntegrityChecks(false); - break :node try list.adjustCapacity( + break :node try list.increaseCapacity( self.node, adjustment, ); - }); + }; + // We must not fail after this, we've modified our self.node + // and we need to fix it up. + errdefer comptime unreachable; + self.* = .init(node); self.cursorAbsolute(old_x, old_y); self.total_rows = old_total_rows; } @@ -1578,17 +1857,17 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { // Remember our new row count so we can restore it // after reinitializing our cursor on the new page. const new_rows = self.new_rows; const node = try list.createPage(cap); + errdefer comptime unreachable; node.data.size.rows = 1; list.pages.insertAfter(self.node, node); self.* = .init(node); - self.new_rows = new_rows; } @@ -1598,7 +1877,7 @@ const ReflowCursor = struct { self: *ReflowCursor, list: *PageList, cap: Capacity, - ) !void { + ) Allocator.Error!void { // The functions below may overwrite self so we need to cache // our total rows. We add one because no matter what when this // returns we'll have one more row added. @@ -1656,7 +1935,7 @@ const ReflowCursor = struct { } }; -fn resizeWithoutReflow(self: *PageList, opts: Resize) !void { +fn resizeWithoutReflow(self: *PageList, opts: Resize) Allocator.Error!void { // We only set the new min_max_size if we're not reflowing. If we are // reflowing, then resize handles this for us. const old_min_max_size = self.min_max_size; @@ -1816,26 +2095,44 @@ fn resizeWithoutReflowGrowCols( self: *PageList, cols: size.CellCountInt, chunk: PageIterator.Chunk, -) !void { +) Allocator.Error!void { assert(cols > self.cols); const page = &chunk.node.data; - const cap = try page.capacity.adjust(.{ .cols = cols }); // Update our col count const old_cols = self.cols; - self.cols = cap.cols; + self.cols = cols; errdefer self.cols = old_cols; // Unlikely fast path: we have capacity in the page. This // is only true if we resized to less cols earlier. - if (page.capacity.cols >= cap.cols) { - page.size.cols = cap.cols; + if (page.capacity.cols >= cols) { + page.size.cols = cols; return; } // Likely slow path: we don't have capacity, so we need // to allocate a page, and copy the old data into it. + // Try to fit our new column size into our existing page capacity. + // If that doesn't work then use a non-standard page with the + // given columns. + const cap = page.capacity.adjust( + .{ .cols = cols }, + ) catch |err| err: { + comptime assert(@TypeOf(err) == error{OutOfMemory}); + + // We verify all maxed out page layouts don't overflow, + var cap = page.capacity; + cap.cols = cols; + + // We're growing columns so we can only get less rows so use + // the lesser of our capacity and size so we minimize wasted + // rows. + cap.rows = @min(page.size.rows, cap.rows); + break :err cap; + }; + // On error, we need to undo all the pages we've added. const prev = chunk.node.prev; errdefer { @@ -1897,6 +2194,39 @@ fn resizeWithoutReflowGrowCols( assert(copied == len); assert(prev_page.size.rows <= prev_page.capacity.rows); + + // Remap any tracked pins that pointed to rows we just copied to prev. + const pin_keys = self.tracked_pins.keys(); + for (pin_keys) |p| { + if (p.node != chunk.node or p.y >= len) continue; + p.node = prev_node; + p.y += prev_page.size.rows - len; + } + } + + // If we have an error, we clear the rows we just added to our prev page. + const prev_copied = copied; + errdefer if (prev_copied > 0) { + const prev_page = &prev.?.data; + const prev_size = prev_page.size.rows - prev_copied; + const prev_rows = prev_page.rows.ptr(prev_page.memory)[prev_size..prev_page.size.rows]; + for (prev_rows) |*row| prev_page.clearCells( + row, + 0, + prev_page.size.cols, + ); + prev_page.size.rows = prev_size; + }; + + // We delete any of the nodes we added. + errdefer { + var it = chunk.node.prev; + while (it) |node| { + if (node == prev) break; + it = node.prev; + self.pages.remove(node); + self.destroyNode(node); + } } // We need to loop because our col growth may force us @@ -1911,19 +2241,33 @@ fn resizeWithoutReflowGrowCols( // Perform the copy const y_start = copied; - const y_end = copied + len; - const src_rows = page.rows.ptr(page.memory)[y_start..y_end]; + const src_rows = page.rows.ptr(page.memory)[y_start .. copied + len]; const dst_rows = new_node.data.rows.ptr(new_node.data.memory)[0..len]; for (dst_rows, src_rows) |*dst_row, *src_row| { new_node.data.size.rows += 1; - errdefer new_node.data.size.rows -= 1; - try new_node.data.cloneRowFrom( + if (new_node.data.cloneRowFrom( page, dst_row, src_row, - ); + )) |_| { + copied += 1; + } else |err| { + // I don't THINK this should be possible, because while our + // row count may diminish due to the adjustment, our + // prior capacity should have been sufficient to hold all the + // managed memory. + log.warn( + "unexpected cloneRowFrom failure during resizeWithoutReflowGrowCols: {}", + .{err}, + ); + + // We can actually safely handle this though by exiting + // this loop early and cutting our copy short. + new_node.data.size.rows -= 1; + break; + } } - copied = y_end; + const y_end = copied; // Insert our new page self.pages.insertBefore(chunk.node, new_node); @@ -1940,6 +2284,10 @@ fn resizeWithoutReflowGrowCols( } assert(copied == page.size.rows); + // Our prior errdeferes are invalid after this point so ensure + // we don't have any more errors. + errdefer comptime unreachable; + // Remove the old page. // Deallocate the old page. self.pages.remove(chunk.node); @@ -2337,6 +2685,166 @@ pub fn scrollClear(self: *PageList) !void { for (0..non_empty) |_| _ = try self.grow(); } +/// Compact a page to use the minimum required memory for the contents +/// it stores. Returns the new node pointer if compaction occurred, or null +/// if the page was already compact or compaction would not provide meaningful +/// savings. +/// +/// The current design of PageList at the time of writing this doesn't +/// allow for smaller than `std_size` nodes so if the current node's backing +/// page is standard size or smaller, no compaction will occur. In the +/// future we should fix this up. +/// +/// If this returns OOM, the PageList is left unchanged and no dangling +/// memory references exist. It is safe to ignore the error and continue using +/// the uncompacted page. +pub fn compact(self: *PageList, node: *List.Node) Allocator.Error!?*List.Node { + defer self.assertIntegrity(); + const page: *Page = &node.data; + + // We should never have empty rows in our pagelist anyways... + assert(page.size.rows > 0); + + // We never compact standard size or smaller pages because changing + // the capacity to something smaller won't save memory. + if (page.memory.len <= std_size) return null; + + // Compute the minimum capacity required for this page's content + const req_cap = page.exactRowCapacity(0, page.size.rows); + const new_size = Page.layout(req_cap).total_size; + const old_size = page.memory.len; + if (new_size >= old_size) return null; + + // Create the new smaller page + const new_node = try self.createPage(req_cap); + errdefer self.destroyNode(new_node); + const new_page: *Page = &new_node.data; + new_page.size = page.size; + new_page.dirty = page.dirty; + new_page.cloneFrom( + page, + 0, + page.size.rows, + ) catch |err| { + // cloneFrom should not fail when compacting since req_cap is + // computed to exactly fit the source content and our expectation + // of exactRowCapacity ensures it can fit all the requested + // data. + log.err("compact clone failed err={}", .{err}); + + // In this case, let's gracefully degrade by pretending we + // didn't need to compact. + self.destroyNode(new_node); + return null; + }; + + // Fix up all tracked pins to point to the new page + const pin_keys = self.tracked_pins.keys(); + for (pin_keys) |p| { + if (p.node != node) continue; + p.node = new_node; + } + + // Insert the new page and destroy the old one + self.pages.insertBefore(node, new_node); + self.pages.remove(node); + self.destroyNode(node); + + new_page.assertIntegrity(); + return new_node; +} + +pub const SplitError = error{ + // Allocator OOM + OutOfMemory, + // Page can't be split further because it is already a single row. + OutOfSpace, +}; + +/// Split the given node in the PageList at the given pin. +/// +/// The row at the pin and after will be moved into a new page with +/// the same capacity as the original page. Alternatively, you can "split +/// above" by splitting the row following the desired split row. +/// +/// Since the split happens below the pin, the pin remains valid. +pub fn split( + self: *PageList, + p: Pin, +) SplitError!void { + if (build_options.slow_runtime_safety) assert(self.pinIsValid(p)); + + // Ran into a bug that I can only explain via aliasing. If a tracked + // pin is passed in, its possible Zig will alias the memory and then + // when we modify it later it updates our p here. Copying the node + // fixes this. + const original_node = p.node; + const page: *Page = &original_node.data; + + // A page that is already 1 row can't be split. In the future we can + // theoretically maybe split by soft-wrapping multiple pages but that + // seems crazy and the rest of our PageList can't handle heterogeneously + // sized pages today. + if (page.size.rows <= 1) return error.OutOfSpace; + + // Splitting at row 0 is a no-op since there's nothing before the split point. + if (p.y == 0) return; + + // At this point we're doing actual modification so make sure + // on the return that we're good. + defer self.assertIntegrity(); + + // Create a new node with the same capacity of managed memory. + const target = try self.createPage(page.capacity); + errdefer self.destroyNode(target); + + // Determine how many rows we're copying + const y_start = p.y; + const y_end = page.size.rows; + target.data.size.rows = y_end - y_start; + assert(target.data.size.rows <= target.data.capacity.rows); + + // Copy our old data. This should NOT fail because we have the + // capacity of the old page which already fits the data we requested. + target.data.cloneFrom(page, y_start, y_end) catch |err| { + log.err( + "error cloning rows for split err={}", + .{err}, + ); + + // Rather than crash, we return an OutOfSpace to show that + // we couldn't split and let our callers gracefully handle it. + // Realistically though... this should not happen. + return error.OutOfSpace; + }; + + // From this point forward there is no going back. We have no + // error handling. It is possible but we haven't written it. + errdefer comptime unreachable; + + // Move any tracked pins from the copied rows + for (self.tracked_pins.keys()) |tracked| { + if (&tracked.node.data != page or + tracked.y < p.y) continue; + + tracked.node = target; + tracked.y -= p.y; + // p.x remains the same since we're copying the row as-is + } + + // Clear our rows + for (page.rows.ptr(page.memory)[y_start..y_end]) |*row| { + page.clearCells( + row, + 0, + page.size.cols, + ); + } + page.size.rows -= y_end - y_start; + + self.pages.insertAfter(original_node, target); +} + /// This represents the state necessary to render a scrollbar for this /// PageList. It has the total size, the offset, and the size of the viewport. pub const Scrollbar = struct { @@ -2499,18 +3007,6 @@ pub fn maxSize(self: *const PageList) usize { return @max(self.explicit_max_size, self.min_max_size); } -/// Returns true if we need to grow into our active area. -inline fn growRequiredForActive(self: *const PageList) bool { - var rows: usize = 0; - var page = self.pages.last; - while (page) |p| : (page = p.prev) { - rows += p.data.size.rows; - if (rows >= self.rows) return false; - } - - return true; -} - /// Grow the active area by exactly one row. /// /// This may allocate, but also may not if our current page has more @@ -2518,7 +3014,7 @@ inline fn growRequiredForActive(self: *const PageList) bool { /// adhere to max_size. /// /// This returns the newly allocated page node if there is one. -pub fn grow(self: *PageList) !?*List.Node { +pub fn grow(self: *PageList) Allocator.Error!?*List.Node { defer self.assertIntegrity(); const last = self.pages.last.?; @@ -2537,7 +3033,7 @@ pub fn grow(self: *PageList) !?*List.Node { // Get the layout first so our failable work is done early. // We'll need this for both paths. - const cap = try std_capacity.adjust(.{ .cols = self.cols }); + const cap = initialCapacity(self.cols); // If allocation would exceed our max size, we prune the first page. // We don't need to reallocate because we can simply reuse that first @@ -2551,16 +3047,22 @@ pub fn grow(self: *PageList) !?*List.Node { self.pages.first != self.pages.last and self.page_size + PagePool.item_size > self.maxSize()) prune: { - // If we need to add more memory to ensure our active area is - // satisfied then we do not prune. - if (self.growRequiredForActive()) break :prune; - const first = self.pages.popFirst().?; assert(first != last); // Decrease our total row count from the pruned page self.total_rows -= first.data.size.rows; + // If our total row count is now less than our required + // rows then we can't prune. The "+ 1" is because we'll add one + // more row below. + if (self.total_rows + 1 < self.rows) { + self.pages.prepend(first); + assert(self.pages.first == first); + self.total_rows += first.data.size.rows; + break :prune; + } + // If we have a pin viewport cache then we need to update it. if (self.viewport == .pin) viewport: { if (self.viewport_pin_row_offset) |*v| { @@ -2589,12 +3091,8 @@ pub fn grow(self: *PageList) !?*List.Node { } self.viewport_pin.garbage = false; - // If our first node has non-standard memory size, we can't reuse - // it. This is because our initBuf below would change the underlying - // memory length which would break our memory free outside the pool. - // It is easiest in this case to prune the node. + // Non-standard pages can't be reused, just destroy them. if (first.data.memory.len > std_size) { - // Node is already removed so we can just destroy it. self.destroyNode(first); break :prune; } @@ -2642,75 +3140,82 @@ pub fn grow(self: *PageList) !?*List.Node { return next_node; } -/// Adjust the capacity of the given page in the list. -pub const AdjustCapacity = struct { - /// Adjust the number of styles in the page. This may be - /// rounded up if necessary to fit alignment requirements, - /// but it will never be rounded down. - styles: ?usize = null, - - /// Adjust the number of available grapheme bytes in the page. - grapheme_bytes: ?usize = null, - - /// Adjust the number of available hyperlink bytes in the page. - hyperlink_bytes: ?usize = null, - - /// Adjust the number of available string bytes in the page. - string_bytes: ?usize = null, +/// Possible dimensions to increase capacity for. +pub const IncreaseCapacity = enum { + styles, + grapheme_bytes, + hyperlink_bytes, + string_bytes, }; -pub const AdjustCapacityError = Allocator.Error || Page.CloneFromError; +pub const IncreaseCapacityError = error{ + // An actual system OOM trying to allocate memory. + OutOfMemory, -/// Adjust the capacity of the given page in the list. This should -/// be used in cases where OutOfMemory is returned by some operation -/// i.e to increase style counts, grapheme counts, etc. + // The existing page is already at max capacity for the given + // adjustment. The caller must create a new page, remove data from + // the old page, etc. (up to the caller). + OutOfSpace, +}; + +/// Increase the capacity of the given page node in the given direction. +/// This will always allocate a new node and remove the old node, so the +/// existing node pointer will be invalid after this call. The newly created +/// node on success is returned. /// -/// Adjustment works by increasing the capacity of the desired -/// dimension to a certain amount and increases the memory allocation -/// requirement for the backing memory of the page. We currently -/// never split pages or anything like that. Because increased allocation -/// has to happen outside our memory pool, its generally much slower -/// so pages should be sized to be large enough to handle all but -/// exceptional cases. +/// The increase amount is at the control of the PageList implementation, +/// but is guaranteed to always increase by at least one unit in the +/// given dimension. Practically, we'll always increase by much more +/// (we currently double every time) but callers shouldn't depend on that. +/// The only guarantee is some amount of growth. /// -/// This can currently only INCREASE capacity size. It cannot -/// decrease capacity size. This limitation is only because we haven't -/// yet needed that use case. If we ever do, this can be added. Currently -/// any requests to decrease will be ignored. -pub fn adjustCapacity( +/// Adjustment can be null if you want to recreate, reclone the page +/// with the same capacity. This is a special case used for rehashing since +/// the logic is otherwise the same. In this case, OutOfMemory is the +/// only possible error. +pub fn increaseCapacity( self: *PageList, node: *List.Node, - adjustment: AdjustCapacity, -) AdjustCapacityError!*List.Node { + adjustment: ?IncreaseCapacity, +) IncreaseCapacityError!*List.Node { defer self.assertIntegrity(); const page: *Page = &node.data; - // We always start with the base capacity of the existing page. This - // ensures we never shrink from what we need. + // Apply our adjustment var cap = page.capacity; + if (adjustment) |v| switch (v) { + inline else => |tag| { + const field_name = @tagName(tag); + const Int = @FieldType(Capacity, field_name); + const old = @field(cap, field_name); - // All ceilPowerOfTwo is unreachable because we're always same or less - // bit width so maxInt is always possible. - if (adjustment.styles) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; - cap.styles = @max(cap.styles, aligned); - } - if (adjustment.grapheme_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; - cap.grapheme_bytes = @max(cap.grapheme_bytes, aligned); - } - if (adjustment.hyperlink_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; - cap.hyperlink_bytes = @max(cap.hyperlink_bytes, aligned); - } - if (adjustment.string_bytes) |v| { - comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize)); - const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable; - cap.string_bytes = @max(cap.string_bytes, aligned); - } + // We use checked math to prevent overflow. If there is an + // overflow it means we're out of space in this dimension, + // since pages can take up to their maxInt capacity in any + // category. + const new = std.math.mul( + Int, + old, + 2, + ) catch |err| overflow: { + comptime assert(@TypeOf(err) == error{Overflow}); + // Our final doubling would overflow since maxInt is + // 2^N - 1 for an unsignged int of N bits. So, if we overflow + // and we haven't used all the bits, use all the bits. + if (old < std.math.maxInt(Int)) break :overflow std.math.maxInt(Int); + return error.OutOfSpace; + }; + @field(cap, field_name) = new; + + // If our capacity exceeds the maximum page size, treat it + // as an OutOfSpace because things like page splitting will + // help. + const layout = Page.layout(cap); + if (layout.total_size > size.max_page_size) { + return error.OutOfSpace; + } + }, + }; log.info("adjusting page capacity={}", .{cap}); @@ -2722,7 +3227,25 @@ pub fn adjustCapacity( assert(new_page.capacity.cols >= page.capacity.cols); new_page.size.rows = page.size.rows; new_page.size.cols = page.size.cols; - try new_page.cloneFrom(page, 0, page.size.rows); + new_page.cloneFrom( + page, + 0, + page.size.rows, + ) catch |err| { + // cloneFrom only errors if there isn't capacity for the data + // from the source page but we're only increasing capacity so + // this should never be possible. If it happens, we should crash + // because we're in no man's land and can't safely recover. + log.err("increaseCapacity clone failed err={}", .{err}); + @panic("unexpected clone failure"); + }; + + // Preserve page-level dirty flag (cloneFrom only copies row data) + new_page.dirty = page.dirty; + + // Must not fail after this because the operations we do after this + // can't be recovered. + errdefer comptime unreachable; // Fix up all our tracked pins to point to the new page. const pin_keys = self.tracked_pins.keys(); @@ -2768,6 +3291,11 @@ inline fn createPageExt( const pooled = layout.total_size <= std_size; const page_alloc = pool.pages.arena.child_allocator; + // It would be better to encode this into the Zig error handling + // system but that is a big undertaking and we only have a few + // centralized call sites so it is handled on its own currently. + assert(layout.total_size <= size.max_page_size); + // Our page buffer comes from our standard memory pool if it // is within our standard size since this is what the pool // dispenses. Otherwise, we use the heap allocator to allocate. @@ -4891,21 +5419,14 @@ test "PageList grow prune required with a single page" { // behavior during a refactor. This is setting up a scenario that is // possible to trigger a bug (#2280). { - // Adjust our capacity until our page is larger than the standard size. + // Increase our capacity until our page is larger than the standard size. // This is important because it triggers a scenario where our calculated // minSize() which is supposed to accommodate 2 pages is no longer true. - var cap = std_capacity; while (true) { - cap.grapheme_bytes *= 2; - const layout = Page.layout(cap); + const layout = Page.layout(s.pages.first.?.data.capacity); if (layout.total_size > std_size) break; + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); } - - // Adjust to that capacity. After we should still have one page. - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .grapheme_bytes = cap.grapheme_bytes }, - ); try testing.expect(s.pages.first != null); try testing.expect(s.pages.first == s.pages.last); } @@ -6324,12 +6845,15 @@ test "PageList eraseRowBounded exhausts pages invalidates viewport offset cache" }, s.scrollbar()); } -test "PageList adjustCapacity to increase styles" { +test "PageList increaseCapacity to increase styles" { const testing = std.testing; const alloc = testing.allocator; var s = try init(alloc, 2, 2, 0); defer s.deinit(); + + const original_styles_cap = s.pages.first.?.data.capacity.styles; + { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; @@ -6347,14 +6871,19 @@ test "PageList adjustCapacity to increase styles" { } // Increase our styles - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .styles = std_capacity.styles * 2 }, - ); + _ = try s.increaseCapacity(s.pages.first.?, .styles); { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; + + // Verify capacity doubled + try testing.expectEqual( + original_styles_cap * 2, + page.capacity.styles, + ); + + // Verify data preserved for (0..s.rows) |y| { for (0..s.cols) |x| { const rac = page.getRowAndCell(x, y); @@ -6367,17 +6896,19 @@ test "PageList adjustCapacity to increase styles" { } } -test "PageList adjustCapacity to increase graphemes" { +test "PageList increaseCapacity to increase graphemes" { const testing = std.testing; const alloc = testing.allocator; var s = try init(alloc, 2, 2, 0); defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.grapheme_bytes; + { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; - // Write all our data so we can assert its the same after for (0..s.rows) |y| { for (0..s.cols) |x| { const rac = page.getRowAndCell(x, y); @@ -6389,15 +6920,14 @@ test "PageList adjustCapacity to increase graphemes" { } } - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .grapheme_bytes = std_capacity.grapheme_bytes * 2 }, - ); + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.grapheme_bytes); + for (0..s.rows) |y| { for (0..s.cols) |x| { const rac = page.getRowAndCell(x, y); @@ -6410,17 +6940,19 @@ test "PageList adjustCapacity to increase graphemes" { } } -test "PageList adjustCapacity to increase hyperlinks" { +test "PageList increaseCapacity to increase hyperlinks" { const testing = std.testing; const alloc = testing.allocator; var s = try init(alloc, 2, 2, 0); defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.hyperlink_bytes; + { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; - // Write all our data so we can assert its the same after for (0..s.rows) |y| { for (0..s.cols) |x| { const rac = page.getRowAndCell(x, y); @@ -6432,15 +6964,14 @@ test "PageList adjustCapacity to increase hyperlinks" { } } - // Increase our graphemes - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .hyperlink_bytes = @max(std_capacity.hyperlink_bytes * 2, 2048) }, - ); + _ = try s.increaseCapacity(s.pages.first.?, .hyperlink_bytes); { try testing.expect(s.pages.first == s.pages.last); const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.hyperlink_bytes); + for (0..s.rows) |y| { for (0..s.cols) |x| { const rac = page.getRowAndCell(x, y); @@ -6453,39 +6984,193 @@ test "PageList adjustCapacity to increase hyperlinks" { } } -test "PageList adjustCapacity after col shrink" { +test "PageList increaseCapacity to increase string_bytes" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + const original_cap = s.pages.first.?.data.capacity.string_bytes; + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x) }, + }; + } + } + } + + _ = try s.increaseCapacity(s.pages.first.?, .string_bytes); + + { + try testing.expect(s.pages.first == s.pages.last); + const page = &s.pages.first.?.data; + + try testing.expectEqual(original_cap * 2, page.capacity.string_bytes); + + for (0..s.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x)), + rac.cell.content.codepoint, + ); + } + } + } +} + +test "PageList increaseCapacity tracked pins" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + // Create a tracked pin on the first page + const tracked = try s.trackPin(s.pin(.{ .active = .{ .x = 1, .y = 1 } }).?); + defer s.untrackPin(tracked); + + const old_node = s.pages.first.?; + try testing.expectEqual(old_node, tracked.node); + + // Increase capacity + const new_node = try s.increaseCapacity(s.pages.first.?, .styles); + + // Pin should now point to the new node + try testing.expectEqual(new_node, tracked.node); + try testing.expectEqual(@as(size.CellCountInt, 1), tracked.x); + try testing.expectEqual(@as(size.CellCountInt, 1), tracked.y); +} + +test "PageList increaseCapacity returns OutOfSpace at max capacity" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, 0); + defer s.deinit(); + + // Keep increasing styles capacity until we get OutOfSpace + const max_styles = std.math.maxInt(size.StyleCountInt); + while (true) { + _ = s.increaseCapacity( + s.pages.first.?, + .styles, + ) catch |err| { + // Before OutOfSpace, we should have reached maxInt + try testing.expectEqual(error.OutOfSpace, err); + try testing.expectEqual(max_styles, s.pages.first.?.data.capacity.styles); + break; + }; + } +} + +test "PageList increaseCapacity after col shrink" { const testing = std.testing; const alloc = testing.allocator; var s = try init(alloc, 10, 2, 0); defer s.deinit(); - // Shrink columns - this updates size.cols but not capacity.cols + // Shrink columns try s.resize(.{ .cols = 5, .reflow = false }); try testing.expectEqual(5, s.cols); { const page = &s.pages.first.?.data; - // capacity.cols is still 10, but size.cols should be 5 try testing.expectEqual(5, page.size.cols); try testing.expect(page.capacity.cols >= 10); } - // Now adjust capacity (e.g., to increase styles) - // This should preserve the current size.cols, not revert to capacity.cols - _ = try s.adjustCapacity( - s.pages.first.?, - .{ .styles = std_capacity.styles * 2 }, - ); + // Increase capacity + _ = try s.increaseCapacity(s.pages.first.?, .styles); { const page = &s.pages.first.?.data; - // After adjustCapacity, size.cols should still be 5, not 10 + // size.cols should still be 5, not reverted to capacity.cols try testing.expectEqual(5, page.size.cols); try testing.expectEqual(5, s.cols); } } +test "PageList increaseCapacity multi-page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + + // Grow to create a second page + const page1_node = s.pages.last.?; + page1_node.data.pauseIntegrityChecks(true); + for (0..page1_node.data.capacity.rows - page1_node.data.size.rows) |_| { + try testing.expect(try s.grow() == null); + } + page1_node.data.pauseIntegrityChecks(false); + try testing.expect(try s.grow() != null); + + // Now we have two pages + try testing.expect(s.pages.first != s.pages.last); + const page2_node = s.pages.last.?; + + const page1_styles_cap = s.pages.first.?.data.capacity.styles; + const page2_styles_cap = page2_node.data.capacity.styles; + + // Increase capacity on the first page only + _ = try s.increaseCapacity(s.pages.first.?, .styles); + + // First page capacity should be doubled + try testing.expectEqual( + page1_styles_cap * 2, + s.pages.first.?.data.capacity.styles, + ); + + // Second page should be unchanged + try testing.expectEqual( + page2_styles_cap, + s.pages.last.?.data.capacity.styles, + ); +} + +test "PageList increaseCapacity preserves dirty flag" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 4, 0); + defer s.deinit(); + + // Set page dirty flag and mark some rows as dirty + const page = &s.pages.first.?.data; + page.dirty = true; + + const rows = page.rows.ptr(page.memory); + rows[0].dirty = true; + rows[1].dirty = false; + rows[2].dirty = true; + rows[3].dirty = false; + + // Increase capacity + const new_node = try s.increaseCapacity(s.pages.first.?, .styles); + + // The page dirty flag should be preserved + try testing.expect(new_node.data.dirty); + + // Row dirty flags should be preserved + const new_rows = new_node.data.rows.ptr(new_node.data.memory); + try testing.expect(new_rows[0].dirty); + try testing.expect(!new_rows[1].dirty); + try testing.expect(new_rows[2].dirty); + try testing.expect(!new_rows[3].dirty); +} + test "PageList pageIterator single page" { const testing = std.testing; const alloc = testing.allocator; @@ -11090,12 +11775,10 @@ test "PageList grow reuses non-standard page without leak" { var s = try init(alloc, 80, 24, 3 * std_size); defer s.deinit(); - // Save the first page node before adjustment - const first_before = s.pages.first.?; - - // Adjust the first page to have non-standard capacity. We use a small - // increase that makes it just slightly larger than std_size. - _ = try s.adjustCapacity(first_before, .{ .grapheme_bytes = std_size + 1 }); + // Increase the first page capacity to make it non-standard (larger than std_size). + while (s.pages.first.?.data.memory.len <= std_size) { + _ = try s.increaseCapacity(s.pages.first.?, .grapheme_bytes); + } // The first page should now have non-standard memory size. try testing.expect(s.pages.first.?.data.memory.len > std_size); @@ -11121,7 +11804,7 @@ test "PageList grow reuses non-standard page without leak" { try testing.expect(s.pages.first.?.data.memory.len > std_size); // Verify we have enough rows for active area (so prune path isn't skipped) - try testing.expect(!s.growRequiredForActive()); + try testing.expect(s.totalRows() >= s.rows); // Verify last page is full (so grow will need to allocate/reuse) try testing.expect(s.pages.last.?.data.size.rows == s.pages.last.?.data.capacity.rows); @@ -11155,3 +11838,863 @@ test "PageList grow reuses non-standard page without leak" { try testing.expectEqual(0, tracked_pin.y); try testing.expect(tracked_pin.garbage); } + +test "PageList grow non-standard page prune protection" { + const testing = std.testing; + const alloc = testing.allocator; + + // This test specifically verifies the fix for the bug where pruning a + // non-standard page would cause totalRows() < self.rows. + // + // Bug trigger conditions (all must be true simultaneously): + // 1. first page is non-standard (memory.len > std_size) + // 2. page_size + PagePool.item_size > maxSize() (triggers prune consideration) + // 3. pages.first != pages.last (have multiple pages) + // 4. total_rows >= self.rows (have enough rows for active area) + // 5. total_rows - first.size.rows + 1 < self.rows (prune would lose too many) + + // This is kind of magic and likely depends on std_size. + const rows_count = 600; + var s = try init(alloc, 80, rows_count, std_size); + defer s.deinit(); + + // Make the first page non-standard + while (s.pages.first.?.data.memory.len <= std_size) { + _ = try s.increaseCapacity( + s.pages.first.?, + .grapheme_bytes, + ); + } + try testing.expect(s.pages.first.?.data.memory.len > std_size); + + const first_page_node = s.pages.first.?; + const first_page_cap = first_page_node.data.capacity.rows; + + // Fill first page to capacity + while (first_page_node.data.size.rows < first_page_cap) _ = try s.grow(); + + // Grow until we have a second page (first page fills up first) + var second_node: ?*List.Node = null; + while (s.pages.first == s.pages.last) second_node = try s.grow(); + try testing.expect(s.pages.first != s.pages.last); + + // Fill the second page to capacity so that the next grow() triggers prune + const last_node = s.pages.last.?; + const second_cap = last_node.data.capacity.rows; + while (last_node.data.size.rows < second_cap) _ = try s.grow(); + + // Now the last page is full. The next grow must either: + // 1. Prune the first page and reuse it, OR + // 2. Allocate a new page + const total = s.totalRows(); + const would_remain = total - first_page_cap + 1; + + // Verify the bug condition is present: pruning first page would leave < rows + try testing.expect(would_remain < s.rows); + + // Verify prune path conditions are met + try testing.expect(s.pages.first != s.pages.last); + try testing.expect(s.page_size + PagePool.item_size > s.maxSize()); + try testing.expect(s.totalRows() >= s.rows); + + // Verify last page is at capacity (so grow must prune or allocate new) + try testing.expectEqual(second_cap, last_node.data.size.rows); + + // The next grow should trigger prune consideration. + // Without the fix, this would destroy the non-standard first page, + // leaving only second_cap + 1 rows, which is < self.rows. + _ = try s.grow(); + + // Verify the invariant holds - the fix prevents the destructive prune + try testing.expect(s.totalRows() >= s.rows); +} + +test "PageList resize (no reflow) more cols remaps pins in backfill path" { + // Regression test: when resizeWithoutReflowGrowCols copies rows to a previous + // page with spare capacity, tracked pins in those rows must be remapped. + // Without the fix, pins become dangling pointers when the original page is destroyed. + const testing = std.testing; + const alloc = testing.allocator; + + const cols: size.CellCountInt = 5; + const cap = try std_capacity.adjust(.{ .cols = cols }); + var s = try init(alloc, cols, cap.rows, null); + defer s.deinit(); + + // Grow until we have two pages. + while (s.pages.first == s.pages.last) { + _ = try s.grow(); + } + const first_page = s.pages.first.?; + const second_page = s.pages.last.?; + try testing.expect(first_page != second_page); + + // Trim a history row so the first page has spare capacity. + // This triggers the backfill path in resizeWithoutReflowGrowCols. + s.eraseRows(.{ .history = .{} }, .{ .history = .{ .y = 0 } }); + try testing.expect(first_page.data.size.rows < first_page.data.capacity.rows); + + // Ensure the resize takes the slow path (new capacity > current capacity). + const new_cols: size.CellCountInt = cols + 1; + const adjusted = try second_page.data.capacity.adjust(.{ .cols = new_cols }); + try testing.expect(second_page.data.capacity.cols < adjusted.cols); + + // Track a pin in row 0 of the second page. This row will be copied + // to the first page during backfill and the pin must be remapped. + const tracked = try s.trackPin(.{ .node = second_page, .x = 0, .y = 0 }); + defer s.untrackPin(tracked); + + // Write a marker character to the tracked cell so we can verify + // the pin points to the correct cell after resize. + const marker: u21 = 'X'; + tracked.rowAndCell().cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = marker }, + }; + + try s.resize(.{ .cols = new_cols, .reflow = false }); + + // Verify the pin points to a valid node still in the page list. + var found = false; + var it = s.pages.first; + while (it) |node| : (it = node.next) { + if (node == tracked.node) { + found = true; + break; + } + } + try testing.expect(found); + try testing.expect(tracked.y < tracked.node.data.size.rows); + + // Verify the pin still points to the cell with our marker content. + const cell = tracked.rowAndCell().cell; + try testing.expectEqual(.codepoint, cell.content_tag); + try testing.expectEqual(marker, cell.content.codepoint); +} + +test "PageList compact std_size page returns null" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, 0); + defer s.deinit(); + + // A freshly created page should be at std_size + const node = s.pages.first.?; + try testing.expect(node.data.memory.len <= std_size); + + // compact should return null since there's nothing to compact + const result = try s.compact(node); + try testing.expectEqual(null, result); + + // Page should still be the same + try testing.expectEqual(node, s.pages.first.?); +} + +test "PageList compact oversized page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + + // Grow until we have multiple pages + const page1_node = s.pages.first.?; + page1_node.data.pauseIntegrityChecks(true); + for (0..page1_node.data.capacity.rows - page1_node.data.size.rows) |_| { + _ = try s.grow(); + } + page1_node.data.pauseIntegrityChecks(false); + _ = try s.grow(); + try testing.expect(s.pages.first != s.pages.last); + + var node = s.pages.first.?; + + // Write content to verify it's preserved + { + const page = &node.data; + for (0..page.size.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(x + y * s.cols) }, + }; + } + } + } + + // Create a tracked pin on this page + const tracked = try s.trackPin(.{ .node = node, .x = 5, .y = 10 }); + defer s.untrackPin(tracked); + + // Make the page oversized + while (node.data.memory.len <= std_size) { + node = try s.increaseCapacity(node, .grapheme_bytes); + } + try testing.expect(node.data.memory.len > std_size); + const oversized_len = node.data.memory.len; + const original_size = node.data.size; + const second_node = node.next.?; + + // Set dirty flag after increaseCapacity + node.data.dirty = true; + + // Compact the page + const new_node = try s.compact(node); + try testing.expect(new_node != null); + + // Verify memory is smaller + try testing.expect(new_node.?.data.memory.len < oversized_len); + + // Verify size preserved + try testing.expectEqual(original_size.rows, new_node.?.data.size.rows); + try testing.expectEqual(original_size.cols, new_node.?.data.size.cols); + + // Verify dirty flag preserved + try testing.expect(new_node.?.data.dirty); + + // Verify linked list integrity + try testing.expectEqual(new_node.?, s.pages.first.?); + try testing.expectEqual(null, new_node.?.prev); + try testing.expectEqual(second_node, new_node.?.next); + try testing.expectEqual(new_node.?, second_node.prev); + + // Verify pin updated correctly + try testing.expectEqual(new_node.?, tracked.node); + try testing.expectEqual(@as(size.CellCountInt, 5), tracked.x); + try testing.expectEqual(@as(size.CellCountInt, 10), tracked.y); + + // Verify content preserved + const page = &new_node.?.data; + for (0..page.size.rows) |y| { + for (0..s.cols) |x| { + const rac = page.getRowAndCell(x, y); + try testing.expectEqual( + @as(u21, @intCast(x + y * s.cols)), + rac.cell.content.codepoint, + ); + } + } +} + +test "PageList compact insufficient savings returns null" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, 0); + defer s.deinit(); + + var node = s.pages.first.?; + + // Make the page slightly oversized (just one increase) + // This might not provide enough savings to justify compaction + node = try s.increaseCapacity(node, .grapheme_bytes); + + // If the page is still at or below std_size, compact returns null + if (node.data.memory.len <= std_size) { + const result = try s.compact(node); + try testing.expectEqual(null, result); + } else { + // If it did grow beyond std_size, verify that compaction + // works or returns null based on savings calculation + const result = try s.compact(node); + // Either it compacted or determined insufficient savings + if (result) |new_node| { + try testing.expect(new_node.data.memory.len < node.data.memory.len); + } + } +} + +test "PageList split at middle row" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Write content to rows: row 0 gets codepoint 0, row 1 gets 1, etc. + for (0..page.size.rows) |y| { + const rac = page.getRowAndCell(0, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(y) }, + }; + } + + // Split at row 5 (middle) + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + // Verify two pages exist + try testing.expect(s.pages.first != null); + try testing.expect(s.pages.first.?.next != null); + + const first_page = &s.pages.first.?.data; + const second_page = &s.pages.first.?.next.?.data; + + // First page should have rows 0-4 (5 rows) + try testing.expectEqual(@as(usize, 5), first_page.size.rows); + // Second page should have rows 5-9 (5 rows) + try testing.expectEqual(@as(usize, 5), second_page.size.rows); + + // Verify content in first page is preserved (rows 0-4 have codepoints 0-4) + for (0..5) |y| { + const rac = first_page.getRowAndCell(0, y); + try testing.expectEqual(@as(u21, @intCast(y)), rac.cell.content.codepoint); + } + + // Verify content in second page (original rows 5-9, now at y=0-4) + for (0..5) |y| { + const rac = second_page.getRowAndCell(0, y); + try testing.expectEqual(@as(u21, @intCast(y + 5)), rac.cell.content.codepoint); + } +} + +test "PageList split at row 0 is no-op" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Write content to all rows + for (0..page.size.rows) |y| { + const rac = page.getRowAndCell(0, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(y) }, + }; + } + + // Split at row 0 should be a no-op + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 0, .x = 0 }; + try s.split(split_pin); + + // Verify only one page exists (no split occurred) + try testing.expect(s.pages.first != null); + try testing.expect(s.pages.first.?.next == null); + + // Verify all content is still in the original page + try testing.expectEqual(@as(usize, 10), page.size.rows); + for (0..10) |y| { + const rac = page.getRowAndCell(0, y); + try testing.expectEqual(@as(u21, @intCast(y)), rac.cell.content.codepoint); + } +} + +test "PageList split at last row" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Write content to all rows + for (0..page.size.rows) |y| { + const rac = page.getRowAndCell(0, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast(y) }, + }; + } + + // Split at last row (row 9) + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 9, .x = 0 }; + try s.split(split_pin); + + // Verify two pages exist + try testing.expect(s.pages.first != null); + try testing.expect(s.pages.first.?.next != null); + + const first_page = &s.pages.first.?.data; + const second_page = &s.pages.first.?.next.?.data; + + // First page should have 9 rows + try testing.expectEqual(@as(usize, 9), first_page.size.rows); + // Second page should have 1 row + try testing.expectEqual(@as(usize, 1), second_page.size.rows); + + // Verify content in second page (original row 9, now at y=0) + const rac = second_page.getRowAndCell(0, 0); + try testing.expectEqual(@as(u21, 9), rac.cell.content.codepoint); +} + +test "PageList split single row page returns OutOfSpace" { + const testing = std.testing; + const alloc = testing.allocator; + + // Initialize with 1 row + var s = try init(alloc, 10, 1, 0); + defer s.deinit(); + + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 0, .x = 0 }; + const result = s.split(split_pin); + + try testing.expectError(error.OutOfSpace, result); +} + +test "PageList split moves tracked pins" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + // Track a pin at row 7 + const tracked = try s.trackPin(.{ .node = s.pages.first.?, .y = 7, .x = 3 }); + defer s.untrackPin(tracked); + + // Split at row 5 + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + // The tracked pin should now be in the second page + try testing.expect(tracked.node == s.pages.first.?.next.?); + // y should be adjusted: was 7, split at 5, so new y = 7 - 5 = 2 + try testing.expectEqual(@as(usize, 2), tracked.y); + // x should remain unchanged + try testing.expectEqual(@as(usize, 3), tracked.x); +} + +test "PageList split tracked pin before split point unchanged" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const original_node = s.pages.first.?; + + // Track a pin at row 2 (before the split point) + const tracked = try s.trackPin(.{ .node = original_node, .y = 2, .x = 5 }); + defer s.untrackPin(tracked); + + // Split at row 5 + const split_pin: Pin = .{ .node = original_node, .y = 5, .x = 0 }; + try s.split(split_pin); + + // The tracked pin should remain in the original page + try testing.expect(tracked.node == s.pages.first.?); + // y and x should be unchanged + try testing.expectEqual(@as(usize, 2), tracked.y); + try testing.expectEqual(@as(usize, 5), tracked.x); +} + +test "PageList split tracked pin at split point moves to new page" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const original_node = s.pages.first.?; + + // Track a pin at the exact split point (row 5) + const tracked = try s.trackPin(.{ .node = original_node, .y = 5, .x = 4 }); + defer s.untrackPin(tracked); + + // Split at row 5 + const split_pin: Pin = .{ .node = original_node, .y = 5, .x = 0 }; + try s.split(split_pin); + + // The tracked pin should be in the new page + try testing.expect(tracked.node == s.pages.first.?.next.?); + // y should be 0 since it was at the split point: 5 - 5 = 0 + try testing.expectEqual(@as(usize, 0), tracked.y); + // x should remain unchanged + try testing.expectEqual(@as(usize, 4), tracked.x); +} + +test "PageList split multiple tracked pins across regions" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const original_node = s.pages.first.?; + + // Track multiple pins in different regions + const pin_before = try s.trackPin(.{ .node = original_node, .y = 1, .x = 0 }); + defer s.untrackPin(pin_before); + const pin_at_split = try s.trackPin(.{ .node = original_node, .y = 5, .x = 2 }); + defer s.untrackPin(pin_at_split); + const pin_after1 = try s.trackPin(.{ .node = original_node, .y = 7, .x = 3 }); + defer s.untrackPin(pin_after1); + const pin_after2 = try s.trackPin(.{ .node = original_node, .y = 9, .x = 8 }); + defer s.untrackPin(pin_after2); + + // Split at row 5 + const split_pin: Pin = .{ .node = original_node, .y = 5, .x = 0 }; + try s.split(split_pin); + + const first_page = s.pages.first.?; + const second_page = first_page.next.?; + + // Pin before split point stays in original page + try testing.expect(pin_before.node == first_page); + try testing.expectEqual(@as(usize, 1), pin_before.y); + try testing.expectEqual(@as(usize, 0), pin_before.x); + + // Pin at split point moves to new page with y=0 + try testing.expect(pin_at_split.node == second_page); + try testing.expectEqual(@as(usize, 0), pin_at_split.y); + try testing.expectEqual(@as(usize, 2), pin_at_split.x); + + // Pins after split point move to new page with adjusted y + try testing.expect(pin_after1.node == second_page); + try testing.expectEqual(@as(usize, 2), pin_after1.y); // 7 - 5 = 2 + try testing.expectEqual(@as(usize, 3), pin_after1.x); + + try testing.expect(pin_after2.node == second_page); + try testing.expectEqual(@as(usize, 4), pin_after2.y); // 9 - 5 = 4 + try testing.expectEqual(@as(usize, 8), pin_after2.x); +} + +test "PageList split tracked viewport_pin in split region moves correctly" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const original_node = s.pages.first.?; + + // Set viewport_pin to row 7 (after split point) + s.viewport_pin.node = original_node; + s.viewport_pin.y = 7; + s.viewport_pin.x = 6; + + // Split at row 5 + const split_pin: Pin = .{ .node = original_node, .y = 5, .x = 0 }; + try s.split(split_pin); + + // viewport_pin should be in the new page + try testing.expect(s.viewport_pin.node == s.pages.first.?.next.?); + // y should be adjusted: 7 - 5 = 2 + try testing.expectEqual(@as(usize, 2), s.viewport_pin.y); + // x should remain unchanged + try testing.expectEqual(@as(usize, 6), s.viewport_pin.x); +} + +test "PageList split middle page preserves linked list order" { + const testing = std.testing; + const alloc = testing.allocator; + + // Create a single page with 12 rows + var s = try init(alloc, 10, 12, 0); + defer s.deinit(); + + // Split at row 4 to create: page1 (rows 0-3), page2 (rows 4-11) + const first_node = s.pages.first.?; + const split_pin1: Pin = .{ .node = first_node, .y = 4, .x = 0 }; + try s.split(split_pin1); + + // Now we have 2 pages + const page1 = s.pages.first.?; + const page2 = s.pages.first.?.next.?; + try testing.expectEqual(@as(usize, 4), page1.data.size.rows); + try testing.expectEqual(@as(usize, 8), page2.data.size.rows); + + // Split page2 at row 4 to create: page1 -> page2 (rows 0-3) -> page3 (rows 4-7) + const split_pin2: Pin = .{ .node = page2, .y = 4, .x = 0 }; + try s.split(split_pin2); + + // Now we have 3 pages + const first = s.pages.first.?; + const middle = first.next.?; + const last = middle.next.?; + + // Verify linked list order: first -> middle -> last + try testing.expectEqual(page1, first); + try testing.expectEqual(page2, middle); + try testing.expectEqual(s.pages.last.?, last); + + // Verify prev pointers + try testing.expect(first.prev == null); + try testing.expectEqual(first, middle.prev.?); + try testing.expectEqual(middle, last.prev.?); + + // Verify next pointers + try testing.expectEqual(middle, first.next.?); + try testing.expectEqual(last, middle.next.?); + try testing.expect(last.next == null); + + // Verify row counts + try testing.expectEqual(@as(usize, 4), first.data.size.rows); + try testing.expectEqual(@as(usize, 4), middle.data.size.rows); + try testing.expectEqual(@as(usize, 4), last.data.size.rows); +} + +test "PageList split last page makes new page the last" { + const testing = std.testing; + const alloc = testing.allocator; + + // Create a single page with 10 rows + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + // Split to create 2 pages first + const first_node = s.pages.first.?; + const split_pin1: Pin = .{ .node = first_node, .y = 5, .x = 0 }; + try s.split(split_pin1); + + // Now split the last page + const last_before_split = s.pages.last.?; + try testing.expectEqual(@as(usize, 5), last_before_split.data.size.rows); + + const split_pin2: Pin = .{ .node = last_before_split, .y = 2, .x = 0 }; + try s.split(split_pin2); + + // The new page should be the new last + const new_last = s.pages.last.?; + try testing.expect(new_last != last_before_split); + try testing.expectEqual(last_before_split, new_last.prev.?); + try testing.expect(new_last.next == null); + + // Verify row counts: original last has 2 rows, new last has 3 rows + try testing.expectEqual(@as(usize, 2), last_before_split.data.size.rows); + try testing.expectEqual(@as(usize, 3), new_last.data.size.rows); +} + +test "PageList split first page keeps original as first" { + const testing = std.testing; + const alloc = testing.allocator; + + // Create 2 pages by splitting + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const original_first = s.pages.first.?; + const split_pin1: Pin = .{ .node = original_first, .y = 5, .x = 0 }; + try s.split(split_pin1); + + // Get second page (created by first split) + const second_page = s.pages.first.?.next.?; + + // Now split the first page again + const split_pin2: Pin = .{ .node = s.pages.first.?, .y = 2, .x = 0 }; + try s.split(split_pin2); + + // Original first should still be first + try testing.expectEqual(original_first, s.pages.first.?); + try testing.expect(s.pages.first.?.prev == null); + + // New page should be inserted between first and second + const inserted = s.pages.first.?.next.?; + try testing.expect(inserted != second_page); + try testing.expectEqual(second_page, inserted.next.?); + + // Verify row counts: first has 2, inserted has 3, second has 5 + try testing.expectEqual(@as(usize, 2), s.pages.first.?.data.size.rows); + try testing.expectEqual(@as(usize, 3), inserted.data.size.rows); + try testing.expectEqual(@as(usize, 5), second_page.data.size.rows); +} + +test "PageList split preserves wrap flags" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Set wrap flags on rows that will be in the second page after split + // Row 5: wrap = true (this is the start of a wrapped line) + // Row 6: wrap_continuation = true (this continues the wrap) + // Row 7: wrap = true, wrap_continuation = true (wrapped and continues) + { + const rac5 = page.getRowAndCell(0, 5); + rac5.row.wrap = true; + + const rac6 = page.getRowAndCell(0, 6); + rac6.row.wrap_continuation = true; + + const rac7 = page.getRowAndCell(0, 7); + rac7.row.wrap = true; + rac7.row.wrap_continuation = true; + } + + // Split at row 5 + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + const second_page = &s.pages.first.?.next.?.data; + + // Verify wrap flags are preserved in new page + // Original row 5 is now row 0 in second page + { + const rac0 = second_page.getRowAndCell(0, 0); + try testing.expect(rac0.row.wrap); + try testing.expect(!rac0.row.wrap_continuation); + } + + // Original row 6 is now row 1 in second page + { + const rac1 = second_page.getRowAndCell(0, 1); + try testing.expect(!rac1.row.wrap); + try testing.expect(rac1.row.wrap_continuation); + } + + // Original row 7 is now row 2 in second page + { + const rac2 = second_page.getRowAndCell(0, 2); + try testing.expect(rac2.row.wrap); + try testing.expect(rac2.row.wrap_continuation); + } +} + +test "PageList split preserves styled cells" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Create a style and apply it to cells in rows 5-7 (which will be in the second page) + const style: stylepkg.Style = .{ .flags = .{ .bold = true } }; + const style_id = try page.styles.add(page.memory, style); + + for (5..8) |y| { + const rac = page.getRowAndCell(0, y); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'S' }, + .style_id = style_id, + }; + rac.row.styled = true; + page.styles.use(page.memory, style_id); + } + // Release the extra ref from add + page.styles.release(page.memory, style_id); + + // Split at row 5 + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + const first_page = &s.pages.first.?.data; + const second_page = &s.pages.first.?.next.?.data; + + // First page should have no styles (all styled rows moved to second page) + try testing.expectEqual(@as(usize, 0), first_page.styles.count()); + + // Second page should have exactly 1 style (the bold style, used by 3 cells) + try testing.expectEqual(@as(usize, 1), second_page.styles.count()); + + // Verify styled cells are preserved in new page + for (0..3) |y| { + const rac = second_page.getRowAndCell(0, y); + try testing.expectEqual(@as(u21, 'S'), rac.cell.content.codepoint); + try testing.expect(rac.cell.style_id != 0); + + const got_style = second_page.styles.get(second_page.memory, rac.cell.style_id); + try testing.expect(got_style.flags.bold); + try testing.expect(rac.row.styled); + } +} + +test "PageList split preserves grapheme clusters" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Add a grapheme cluster to row 6 (will be row 1 in second page after split at 5) + { + const rac = page.getRowAndCell(0, 6); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 0x1F468 }, // Man emoji + }; + try page.setGraphemes(rac.row, rac.cell, &.{ + 0x200D, // ZWJ + 0x1F469, // Woman emoji + }); + } + + // Split at row 5 + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + const first_page = &s.pages.first.?.data; + const second_page = &s.pages.first.?.next.?.data; + + // First page should have no graphemes (the grapheme row moved to second page) + try testing.expectEqual(@as(usize, 0), first_page.graphemeCount()); + + // Second page should have exactly 1 grapheme + try testing.expectEqual(@as(usize, 1), second_page.graphemeCount()); + + // Verify grapheme is preserved in new page (original row 6 is now row 1) + { + const rac = second_page.getRowAndCell(0, 1); + try testing.expectEqual(@as(u21, 0x1F468), rac.cell.content.codepoint); + try testing.expect(rac.row.grapheme); + + const cps = second_page.lookupGrapheme(rac.cell).?; + try testing.expectEqual(@as(usize, 2), cps.len); + try testing.expectEqual(@as(u21, 0x200D), cps[0]); + try testing.expectEqual(@as(u21, 0x1F469), cps[1]); + } +} + +test "PageList split preserves hyperlinks" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 10, 10, 0); + defer s.deinit(); + + const page = &s.pages.first.?.data; + + // Add a hyperlink to row 7 (will be row 2 in second page after split at 5) + const hyperlink_id = try page.insertHyperlink(.{ + .id = .{ .implicit = 0 }, + .uri = "https://example.com", + }); + { + const rac = page.getRowAndCell(0, 7); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = 'L' }, + }; + try page.setHyperlink(rac.row, rac.cell, hyperlink_id); + } + + // Split at row 5 + const split_pin: Pin = .{ .node = s.pages.first.?, .y = 5, .x = 0 }; + try s.split(split_pin); + + const first_page = &s.pages.first.?.data; + const second_page = &s.pages.first.?.next.?.data; + + // First page should have no hyperlinks (the hyperlink row moved to second page) + try testing.expectEqual(@as(usize, 0), first_page.hyperlink_set.count()); + + // Second page should have exactly 1 hyperlink + try testing.expectEqual(@as(usize, 1), second_page.hyperlink_set.count()); + + // Verify hyperlink is preserved in new page (original row 7 is now row 2) + { + const rac = second_page.getRowAndCell(0, 2); + try testing.expectEqual(@as(u21, 'L'), rac.cell.content.codepoint); + try testing.expect(rac.cell.hyperlink); + + const link_id = second_page.lookupHyperlink(rac.cell).?; + const link = second_page.hyperlink_set.get(second_page.memory, link_id); + try testing.expectEqualStrings("https://example.com", link.uri.slice(second_page.memory)); + } +} diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 2861e02e5..d36cdac2a 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -517,39 +517,38 @@ pub fn clone( result.assertIntegrity(); return result; } - -/// Adjust the capacity of a page within the pagelist of this screen. -/// This handles some accounting if the page being modified is the -/// cursor page. -pub fn adjustCapacity( +pub fn increaseCapacity( self: *Screen, node: *PageList.List.Node, - adjustment: PageList.AdjustCapacity, -) PageList.AdjustCapacityError!*PageList.List.Node { + adjustment: ?PageList.IncreaseCapacity, +) PageList.IncreaseCapacityError!*PageList.List.Node { // If the page being modified isn't our cursor page then // this is a quick operation because we have no additional - // accounting. - if (node != self.cursor.page_pin.node) { - return try self.pages.adjustCapacity(node, adjustment); - } + // accounting. We have to do this check here BEFORE calling + // increaseCapacity because increaseCapacity will update all + // our tracked pins (including our cursor). + if (node != self.cursor.page_pin.node) return try self.pages.increaseCapacity( + node, + adjustment, + ); - // We're modifying the cursor page. When we adjust the + // We're modifying the cursor page. When we increase the // capacity below it will be short the ref count on our // current style and hyperlink, so we need to init those. - const new_node = try self.pages.adjustCapacity(node, adjustment); + const new_node = try self.pages.increaseCapacity(node, adjustment); const new_page: *Page = &new_node.data; // Re-add the style, if the page somehow doesn't have enough // memory to add it, we emit a warning and gracefully degrade // to the default style for the cursor. - if (self.cursor.style_id != 0) { + if (self.cursor.style_id != style.default_id) { self.cursor.style_id = new_page.styles.add( new_page.memory, self.cursor.style, ) catch |err| id: { // TODO: Should we increase the capacity further in this case? log.warn( - "(Screen.adjustCapacity) Failed to add cursor style back to page, err={}", + "(Screen.increaseCapacity) Failed to add cursor style back to page, err={}", .{err}, ); @@ -571,7 +570,7 @@ pub fn adjustCapacity( self.startHyperlinkOnce(link.*) catch |err| { // TODO: Should we increase the capacity further in this case? log.warn( - "(Screen.adjustCapacity) Failed to add cursor hyperlink back to page, err={}", + "(Screen.increaseCapacity) Failed to add cursor hyperlink back to page, err={}", .{err}, ); }; @@ -640,9 +639,8 @@ pub fn cursorUp(self: *Screen, n: size.CellCountInt) void { defer self.assertIntegrity(); self.cursor.y -= n; // Must be set before cursorChangePin - const page_pin = self.cursor.page_pin.up(n).?; - self.cursorChangePin(page_pin); - const page_rac = page_pin.rowAndCell(); + self.cursorChangePin(self.cursor.page_pin.up(n).?); + const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; } @@ -667,9 +665,8 @@ pub fn cursorDown(self: *Screen, n: size.CellCountInt) void { // We move the offset into our page list to the next row and then // get the pointers to the row/cell and set all the cursor state up. - const page_pin = self.cursor.page_pin.down(n).?; - self.cursorChangePin(page_pin); - const page_rac = page_pin.rowAndCell(); + self.cursorChangePin(self.cursor.page_pin.down(n).?); + const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; } @@ -800,31 +797,37 @@ pub fn cursorDownScroll(self: *Screen) !void { // allocate, prune scrollback, whatever. _ = try self.pages.grow(); - // If our pin page change it means that the page that the pin - // was on was pruned. In this case, grow() moves the pin to - // the top-left of the new page. This effectively moves it by - // one already, we just need to fix up the x value. - const page_pin = if (old_pin.node == self.cursor.page_pin.node) - self.cursor.page_pin.down(1).? - else reuse: { - var pin = self.cursor.page_pin.*; - pin.x = self.cursor.x; - break :reuse pin; - }; + self.cursorChangePin(new_pin: { + // We do this all in a block here because referencing this pin + // after cursorChangePin is unsafe, and we want to keep it out + // of scope. - // These assertions help catch some pagelist math errors. Our - // x/y should be unchanged after the grow. - if (build_options.slow_runtime_safety) { - const active = self.pages.pointFromPin( - .active, - page_pin, - ).?.active; - assert(active.x == self.cursor.x); - assert(active.y == self.cursor.y); - } + // If our pin page change it means that the page that the pin + // was on was pruned. In this case, grow() moves the pin to + // the top-left of the new page. This effectively moves it by + // one already, we just need to fix up the x value. + const page_pin = if (old_pin.node == self.cursor.page_pin.node) + self.cursor.page_pin.down(1).? + else reuse: { + var pin = self.cursor.page_pin.*; + pin.x = self.cursor.x; + break :reuse pin; + }; - self.cursorChangePin(page_pin); - const page_rac = page_pin.rowAndCell(); + // These assertions help catch some pagelist math errors. Our + // x/y should be unchanged after the grow. + if (build_options.slow_runtime_safety) { + const active = self.pages.pointFromPin( + .active, + page_pin, + ).?.active; + assert(active.x == self.cursor.x); + assert(active.y == self.cursor.y); + } + + break :new_pin page_pin; + }); + const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; @@ -834,7 +837,7 @@ pub fn cursorDownScroll(self: *Screen) !void { // Clear the new row so it gets our bg color. We only do this // if we have a bg color at all. if (self.cursor.style.bg_color != .none) { - const page: *Page = &page_pin.node.data; + const page: *Page = &self.cursor.page_pin.node.data; self.clearCells( page, self.cursor.page_row, @@ -1081,6 +1084,11 @@ pub fn cursorCopy(self: *Screen, other: Cursor, opts: struct { /// page than the old AND we have a style or hyperlink set. In that case, /// we must release our old one and insert the new one, since styles are /// stored per-page. +/// +/// Note that this can change the cursor pin AGAIN if the process of +/// setting up our cursor forces a capacity adjustment of the underlying +/// cursor page, so any references to the page pin should be re-read +/// from `self.cursor.page_pin` after calling this. inline fn cursorChangePin(self: *Screen, new: Pin) void { // Moving the cursor affects text run splitting (ligatures) so // we must mark the old and new page dirty. We do this as long @@ -1097,14 +1105,19 @@ inline fn cursorChangePin(self: *Screen, new: Pin) void { return; } - // If we have a old style then we need to release it from the old page. + // If we have an old style then we need to release it from the old page. const old_style_: ?style.Style = if (self.cursor.style_id == style.default_id) null else self.cursor.style; if (old_style_ != null) { + // Release the style directly from the old page instead of going through + // manualStyleUpdate, because the cursor position may have already been + // updated but the pin has not, which would fail integrity checks. + const old_page: *Page = &self.cursor.page_pin.node.data; + old_page.styles.release(old_page.memory, self.cursor.style_id); self.cursor.style = .{}; - self.manualStyleUpdate() catch unreachable; // Removing a style should never fail + self.cursor.style_id = style.default_id; } // If we have a hyperlink then we need to release it from the old page. @@ -1776,9 +1789,25 @@ fn resizeInternal( /// Set a style attribute for the current cursor. /// -/// This can cause a page split if the current page cannot fit this style. -/// This is the only scenario an error return is possible. -pub fn setAttribute(self: *Screen, attr: sgr.Attribute) !void { +/// If the style can't be set due to any internal errors (memory-related), +/// then this will revert back to the existing style and return an error. +pub fn setAttribute( + self: *Screen, + attr: sgr.Attribute, +) PageList.IncreaseCapacityError!void { + // If we fail to set our style for any reason, we should revert + // back to the old style. If we fail to do that, we revert back to + // the default style. + const old_style = self.cursor.style; + errdefer { + self.cursor.style = old_style; + self.manualStyleUpdate() catch |err| { + log.warn("setAttribute error restoring old style after failure err={}", .{err}); + self.cursor.style = .{}; + self.manualStyleUpdate() catch unreachable; + }; + } + switch (attr) { .unset => { self.cursor.style = .{}; @@ -1921,7 +1950,21 @@ pub fn setAttribute(self: *Screen, attr: sgr.Attribute) !void { } /// Call this whenever you manually change the cursor style. -pub fn manualStyleUpdate(self: *Screen) !void { +/// +/// This function can NOT fail if the cursor style is changing to the +/// default style. +/// +/// If this returns an error, the style change did not take effect and +/// the cursor style is reverted back to the default. The only scenario +/// this returns an error is if there is a physical memory allocation failure +/// or if there is no possible way to increase style capacity to store +/// the style. +/// +/// This function WILL split pages as necessary to accommodate the new style. +/// So if OutOfSpace is returned, it means that even after splitting the page +/// there was still no room for the new style. +pub fn manualStyleUpdate(self: *Screen) PageList.IncreaseCapacityError!void { + defer self.assertIntegrity(); var page: *Page = &self.cursor.page_pin.node.data; // std.log.warn("active styles={}", .{page.styles.count()}); @@ -1940,6 +1983,9 @@ pub fn manualStyleUpdate(self: *Screen) !void { // 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. + // + // This also ensures that if anything fails below, we fall back to + // clearing our style. self.cursor.style_id = style.default_id; // After setting the style, we need to update our style map. @@ -1951,30 +1997,115 @@ pub fn manualStyleUpdate(self: *Screen) !void { page.memory, self.cursor.style, ) catch |err| id: { - // Our style map is full or needs to be rehashed, - // so we allocate a new page, which will rehash, - // and double the style capacity for it if it was - // full. - const node = try self.adjustCapacity( + // Our style map is full or needs to be rehashed, so we need to + // increase style capacity (or rehash). + const node = self.increaseCapacity( self.cursor.page_pin.node, switch (err) { - error.OutOfMemory => .{ .styles = page.capacity.styles * 2 }, - error.NeedsRehash => .{}, + error.OutOfMemory => .styles, + error.NeedsRehash => null, }, - ); + ) catch |increase_err| switch (increase_err) { + error.OutOfMemory => return error.OutOfMemory, + error.OutOfSpace => space: { + // Out of space, we need to split the page. Split wherever + // is using less capacity and hope that works. If it doesn't + // work, we tried. + try self.splitForCapacity(self.cursor.page_pin.*); + break :space self.cursor.page_pin.node; + }, + }; page = &node.data; - break :id try page.styles.add( + break :id page.styles.add( page.memory, self.cursor.style, - ); + ) catch |err2| switch (err2) { + error.OutOfMemory => { + // This shouldn't happen because increaseCapacity is + // guaranteed to increase our capacity by at least one and + // we only need one space, but again, I don't want to crash + // here so let's log loudly and reset. + log.err("style addition failed after capacity increase", .{}); + return error.OutOfMemory; + }, + error.NeedsRehash => { + // This should be impossible because we rehash above + // and rehashing should never result in a duplicate. But + // we don't want to simply hard crash so log it and + // clear our style. + log.err("style rehash resulted in needs rehash", .{}); + return; + }, + }; }; + errdefer page.styles.release(page.memory, id); + self.cursor.style_id = id; - self.assertIntegrity(); +} + +/// Split at the given pin so that the pinned row moves to the page +/// with less used capacity after the split. +/// +/// The primary use case for this is to handle IncreaseCapacityError +/// OutOfSpace conditions where we need to split the page in order +/// to make room for more managed memory. +/// +/// If the caller cares about where the pin moves to, they should +/// setup a tracked pin before calling this and then check that. +/// In many calling cases, the input pin is tracked (e.g. the cursor +/// pin). +/// +/// If this returns OOM then its a system OOM. If this returns OutOfSpace +/// then it means the page can't be split further. +fn splitForCapacity( + self: *Screen, + pin: Pin, +) PageList.SplitError!void { + // Get our capacities. We include our target row because its + // capacity will be preserved. + const bytes_above = Page.layout(pin.node.data.exactRowCapacity( + 0, + pin.y + 1, + )).total_size; + const bytes_below = Page.layout(pin.node.data.exactRowCapacity( + pin.y, + pin.node.data.size.rows, + )).total_size; + + // We need to track the old cursor pin because if our split + // moves the cursor pin we need to update our accounting. + const old_cursor = self.cursor.page_pin.*; + + // If our bytes above are less than bytes below, we move the pin + // to split down one since splitting includes the pinned row in + // the new node. + try self.pages.split(if (bytes_above < bytes_below) + pin.down(1) orelse pin + else + pin); + + // Cursor didn't change nodes, we're done. + if (self.cursor.page_pin.node == old_cursor.node) return; + + // Cursor changed, we need to restore the old pin then use + // cursorChangePin to move to the new pin. The old node is guaranteed + // to still exist, just not the row. + // + // Note that page_row and all that will be invalid, it points to the + // new node, but at the time of writing this we don't need any of that + // to be right in cursorChangePin. + const new_cursor = self.cursor.page_pin.*; + self.cursor.page_pin.* = old_cursor; + self.cursorChangePin(new_cursor); } /// Append a grapheme to the given cell within the current cursor row. -pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { +pub fn appendGrapheme( + self: *Screen, + cell: *Cell, + cp: u21, +) PageList.IncreaseCapacityError!void { defer self.cursor.page_pin.node.data.assertIntegrity(); self.cursor.page_pin.node.data.appendGrapheme( self.cursor.page_row, @@ -1994,11 +2125,9 @@ pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { // Adjust our capacity. This will update our cursor page pin and // force us to reload. - const original_node = self.cursor.page_pin.node; - const new_bytes = original_node.data.capacity.grapheme_bytes * 2; - _ = try self.adjustCapacity( - original_node, - .{ .grapheme_bytes = new_bytes }, + _ = try self.increaseCapacity( + self.cursor.page_pin.node, + .grapheme_bytes, ); // The cell pointer is now invalid, so we need to get it from @@ -2009,17 +2138,22 @@ pub fn appendGrapheme(self: *Screen, cell: *Cell, cp: u21) !void { .gt => self.cursorCellRight(@intCast(cell_idx - self.cursor.x)), }; - try self.cursor.page_pin.node.data.appendGrapheme( + self.cursor.page_pin.node.data.appendGrapheme( self.cursor.page_row, reloaded_cell, cp, - ); + ) catch |err2| { + comptime assert(@TypeOf(err2) == error{OutOfMemory}); + // This should never happen because we just increased capacity. + // Log loudly but still return an error so we don't just + // crash. + log.err("grapheme append failed after capacity increase", .{}); + return err2; + }; }, }; } -pub const StartHyperlinkError = Allocator.Error || PageList.AdjustCapacityError; - /// Start the hyperlink state. Future cells will be marked as hyperlinks with /// this state. Note that various terminal operations may clear the hyperlink /// state, such as switching screens (alt screen). @@ -2027,7 +2161,7 @@ pub fn startHyperlink( self: *Screen, uri: []const u8, id_: ?[]const u8, -) StartHyperlinkError!void { +) PageList.IncreaseCapacityError!void { // Create our pending entry. const link: hyperlink.Hyperlink = .{ .uri = uri, @@ -2052,21 +2186,21 @@ pub fn startHyperlink( error.OutOfMemory => return error.OutOfMemory, // strings table is out of memory, adjust it up - error.StringsOutOfMemory => _ = try self.adjustCapacity( + error.StringsOutOfMemory => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ .string_bytes = self.cursor.page_pin.node.data.capacity.string_bytes * 2 }, + .string_bytes, ), // hyperlink set is out of memory, adjust it up - error.SetOutOfMemory => _ = try self.adjustCapacity( + error.SetOutOfMemory => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ .hyperlink_bytes = self.cursor.page_pin.node.data.capacity.hyperlink_bytes * 2 }, + .hyperlink_bytes, ), // hyperlink set is too full, rehash it - error.SetNeedsRehash => _ = try self.adjustCapacity( + error.SetNeedsRehash => _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{}, + null, ), } @@ -2128,7 +2262,7 @@ pub fn endHyperlink(self: *Screen) void { } /// Set the current hyperlink state on the current cell. -pub fn cursorSetHyperlink(self: *Screen) !void { +pub fn cursorSetHyperlink(self: *Screen) PageList.IncreaseCapacityError!void { assert(self.cursor.hyperlink_id != 0); var page = &self.cursor.page_pin.node.data; @@ -2143,40 +2277,38 @@ pub fn cursorSetHyperlink(self: *Screen) !void { } else |err| switch (err) { // hyperlink_map is out of space, realloc the page to be larger error.HyperlinkMapOutOfMemory => { - const uri_size = if (self.cursor.hyperlink) |link| link.uri.len else 0; - - var string_bytes = page.capacity.string_bytes; - // Attempt to allocate the space that would be required to // insert a new copy of the cursor hyperlink uri in to the - // string alloc, since right now adjustCapacity always just + // string alloc, since right now increaseCapacity always just // adds an extra copy even if one already exists in the page. // If this alloc fails then we know we also need to grow our // string bytes. // - // FIXME: This SUCKS - if (page.string_alloc.alloc( - u8, - page.memory, - uri_size, - )) |slice| { - // We don't bother freeing because we're - // about to free the entire page anyway. - _ = &slice; - } else |_| { - // We didn't have enough room, let's just double our - // string bytes until there's definitely enough room - // for our uri. - const before = string_bytes; - while (string_bytes - before < uri_size) string_bytes *= 2; + // FIXME: increaseCapacity should not do this. + while (self.cursor.hyperlink) |link| { + if (page.string_alloc.alloc( + u8, + page.memory, + link.uri.len, + )) |slice| { + // We don't bother freeing because we're + // about to free the entire page anyway. + _ = slice; + break; + } else |_| {} + + // We didn't have enough room, let's increase string bytes + const new_node = try self.increaseCapacity( + self.cursor.page_pin.node, + .string_bytes, + ); + assert(new_node == self.cursor.page_pin.node); + page = &new_node.data; } - _ = try self.adjustCapacity( + _ = try self.increaseCapacity( self.cursor.page_pin.node, - .{ - .hyperlink_bytes = page.capacity.hyperlink_bytes * 2, - .string_bytes = string_bytes, - }, + .hyperlink_bytes, ); // Retry @@ -2998,15 +3130,15 @@ pub fn testWriteString(self: *Screen, text: []const u8) !void { .protected = self.cursor.protected, }; - // If we have a hyperlink, add it to the cell. - if (self.cursor.hyperlink_id > 0) try self.cursorSetHyperlink(); - // If we have a ref-counted style, increase. if (self.cursor.style_id != style.default_id) { const page = self.cursor.page_pin.node.data; page.styles.use(page.memory, self.cursor.style_id); self.cursor.page_row.styled = true; } + + // If we have a hyperlink, add it to the cell. + if (self.cursor.hyperlink_id > 0) try self.cursorSetHyperlink(); }, 2 => { @@ -8887,132 +9019,437 @@ test "Screen: cursorSetHyperlink OOM + URI too large for string alloc" { try testing.expect(base_string_bytes < s.cursor.page_pin.node.data.capacity.string_bytes); } -test "Screen: adjustCapacity cursor style ref count" { +test "Screen: increaseCapacity cursor style ref count preserved" { const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); defer s.deinit(); - - try s.setAttribute(.{ .bold = {} }); + try s.setAttribute(.bold); try s.testWriteString("1ABCD"); + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); + + const old_style = s.cursor.style; + { const page = &s.pages.pages.last.?.data; + // 5 chars + cursor = 6 refs try testing.expectEqual( - 6, // All chars + cursor + 6, page.styles.refCount(page.memory, s.cursor.style_id), ); } - // This forces the page to change. - _ = try s.adjustCapacity( + // This forces the page to change via increaseCapacity. + const new_node = try s.increaseCapacity( s.cursor.page_pin.node, - .{ .grapheme_bytes = s.cursor.page_pin.node.data.capacity.grapheme_bytes * 2 }, + .grapheme_bytes, ); - // Our ref counts should still be the same + // Cursor's page_pin should now point to the new node + try testing.expect(s.cursor.page_pin.node == new_node); + + // Verify cursor's page_cell and page_row are correctly reloaded from the pin + const page_rac = s.cursor.page_pin.rowAndCell(); + try testing.expect(s.cursor.page_row == page_rac.row); + try testing.expect(s.cursor.page_cell == page_rac.cell); + + // Style should be preserved + try testing.expectEqual(old_style, s.cursor.style); + try testing.expect(s.cursor.style_id != style.default_id); + + // After increaseCapacity, the 5 chars are cloned (5 refs) and + // the cursor's style is re-added (1 ref) = 6 total. { const page = &s.pages.pages.last.?.data; - try testing.expectEqual( - 6, // All chars + cursor - page.styles.refCount(page.memory, s.cursor.style_id), - ); + const ref_count = page.styles.refCount(page.memory, s.cursor.style_id); + try testing.expectEqual(6, ref_count); } } -test "Screen: adjustCapacity cursor hyperlink exceeds string alloc size" { +test "Screen: increaseCapacity cursor hyperlink ref count preserved" { const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, .{ .cols = 80, .rows = 24, .max_scrollback = 0 }); + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); defer s.deinit(); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("1ABCD"); - // Start a hyperlink with a URI that just barely fits in the string alloc. - // This will ensure that the redundant copy added in `adjustCapacity` won't - // fit in the available string alloc space. - const uri = "a" ** (pagepkg.std_capacity.string_bytes - 8); - try s.startHyperlink(uri, null); + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); - // Write some characters with this so that the URI - // is copied to the new page when adjusting capacity. - try s.testWriteString("Hello"); + { + const page = &s.pages.pages.last.?.data; + // Cursor has the hyperlink active = 1 count in hyperlink_set + try testing.expectEqual(1, page.hyperlink_set.count()); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expect(s.cursor.hyperlink != null); + } - // Adjust the capacity, right now this will cause a redundant copy of - // the URI to be added to the string alloc, but since there isn't room - // for this this will clear the cursor hyperlink. - _ = try s.adjustCapacity(s.cursor.page_pin.node, .{}); + // This forces the page to change via increaseCapacity. + _ = try s.increaseCapacity( + s.cursor.page_pin.node, + .grapheme_bytes, + ); - // The cursor hyperlink should have been cleared by the `adjustCapacity` - // call, because there isn't enough room to add the redundant URI string. - // - // This behavior will change, causing this test to fail, if any of these - // changes are made: - // - // - The string alloc is changed to intern strings. - // - // - The adjustCapacity function is changed to ensure the new - // capacity will fit the redundant copy of the hyperlink uri. - // - // - The cursor managed memory handling is reworked so that it - // doesn't reside in the pages anymore and doesn't need this - // accounting. - // - // In such a case, adjust this test accordingly. - try testing.expectEqual(null, s.cursor.hyperlink); - try testing.expectEqual(0, s.cursor.hyperlink_id); + // Hyperlink should be preserved with correct URI + try testing.expect(s.cursor.hyperlink != null); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); + + // After increaseCapacity, the hyperlink is re-added to the new page. + { + const page = &s.pages.pages.last.?.data; + try testing.expectEqual(1, page.hyperlink_set.count()); + } } -test "Screen: adjustCapacity cursor style exceeds style set capacity" { +test "Screen: increaseCapacity cursor with both style and hyperlink preserved" { const testing = std.testing; const alloc = testing.allocator; - var s = try init(alloc, .{ .cols = 80, .rows = 24, .max_scrollback = 1000 }); + var s = try init(alloc, .{ + .cols = 5, + .rows = 5, + .max_scrollback = 0, + }); defer s.deinit(); + // Set both a non-default style AND an active hyperlink. + // Write one character first with bold to mark the row as styled, + // then start the hyperlink and write more characters. + try s.setAttribute(.bold); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("1ABCD"); + + // We should have one page and it should be our cursor page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.pages.pages.first == s.cursor.page_pin.node); + + const old_style = s.cursor.style; + + { + const page = &s.pages.pages.last.?.data; + // 5 chars + cursor = 6 refs for bold style + try testing.expectEqual( + 6, + page.styles.refCount(page.memory, s.cursor.style_id), + ); + // Cursor has the hyperlink active = 1 count in hyperlink_set + try testing.expectEqual(1, page.hyperlink_set.count()); + try testing.expect(s.cursor.style_id != style.default_id); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expect(s.cursor.hyperlink != null); + } + + // This forces the page to change via increaseCapacity. + _ = try s.increaseCapacity( + s.cursor.page_pin.node, + .grapheme_bytes, + ); + + // Style should be preserved + try testing.expectEqual(old_style, s.cursor.style); + try testing.expect(s.cursor.style_id != style.default_id); + + // Hyperlink should be preserved with correct URI + try testing.expect(s.cursor.hyperlink != null); + try testing.expect(s.cursor.hyperlink_id != 0); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); + + // After increaseCapacity, both style and hyperlink are re-added to the new page. + { + const page = &s.pages.pages.last.?.data; + const ref_count = page.styles.refCount(page.memory, s.cursor.style_id); + try testing.expectEqual(6, ref_count); + try testing.expectEqual(1, page.hyperlink_set.count()); + } +} + +test "Screen: increaseCapacity non-cursor page returns early" { + // Test that calling increaseCapacity on a page that is NOT the cursor's + // page properly delegates to pages.increaseCapacity without doing the + // extra cursor accounting (style/hyperlink re-adding). + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 80, + .rows = 24, + .max_scrollback = 10000, + }); + defer s.deinit(); + + // Set up a custom style and hyperlink on the cursor + try s.setAttribute(.bold); + try s.startHyperlink("https://example.com/", null); + try s.testWriteString("Hello"); + + // Store cursor state before growing pages + const old_style = s.cursor.style; + const old_style_id = s.cursor.style_id; + const old_hyperlink = s.cursor.hyperlink; + const old_hyperlink_id = s.cursor.hyperlink_id; + + // The cursor is on the first (and only) page + try testing.expect(s.pages.pages.first == s.pages.pages.last); + try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + + // Grow pages until we have multiple pages. The cursor's pin stays on + // the first page since we're just adding rows. + const first_page_node = s.pages.pages.first.?; + first_page_node.data.pauseIntegrityChecks(true); + for (0..first_page_node.data.capacity.rows - first_page_node.data.size.rows) |_| { + _ = try s.pages.grow(); + } + first_page_node.data.pauseIntegrityChecks(false); + _ = try s.pages.grow(); + + // Now we have two pages + try testing.expect(s.pages.pages.first != s.pages.pages.last); + const second_page = s.pages.pages.last.?; + + // Cursor should still be on the first page (where it was created) + try testing.expect(s.cursor.page_pin.node == s.pages.pages.first.?); + try testing.expect(s.cursor.page_pin.node != second_page); + + const second_page_styles_cap = second_page.data.capacity.styles; + const cursor_page_styles_cap = s.cursor.page_pin.node.data.capacity.styles; + + // Call increaseCapacity on the second page (NOT the cursor's page) + const new_second_page = try s.increaseCapacity(second_page, .styles); + + // The second page should have increased capacity + try testing.expectEqual( + second_page_styles_cap * 2, + new_second_page.data.capacity.styles, + ); + + // The cursor's page (first page) should be unchanged + try testing.expectEqual( + cursor_page_styles_cap, + s.cursor.page_pin.node.data.capacity.styles, + ); + + // Cursor state should be completely unchanged since we didn't touch its page + try testing.expectEqual(old_style, s.cursor.style); + try testing.expectEqual(old_style_id, s.cursor.style_id); + try testing.expectEqual(old_hyperlink, s.cursor.hyperlink); + try testing.expectEqual(old_hyperlink_id, s.cursor.hyperlink_id); + + // Verify hyperlink is still valid + try testing.expect(s.cursor.hyperlink != null); + try testing.expectEqualStrings("https://example.com/", s.cursor.hyperlink.?.uri); +} + +test "Screen: cursorDown to page with insufficient capacity" { + // Regression test for https://github.com/ghostty-org/ghostty/issues/10282 + // + // This test exposes a use-after-realloc bug in cursorDown (and similar + // cursor movement functions). The bug pattern: + // + // 1. cursorDown creates a by-value copy of the pin via page_pin.down(n) + // 2. cursorChangePin is called, which may trigger adjustCapacity + // if the target page's style map is full + // 3. adjustCapacity frees the old page and creates a new one + // 4. The local pin copy still points to the freed page + // 5. rowAndCell() on the stale pin accesses freed memory + + const testing = std.testing; + const alloc = testing.allocator; + + // Small screen to make page boundary crossing easy to set up + var s = try init(alloc, .{ .cols = 10, .rows = 3, .max_scrollback = 1 }); + defer s.deinit(); + + // Scroll down enough to create a second 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); + + // Cursor should now be on a new page + const new_page = &s.cursor.page_pin.node.data; + try testing.expect(start_page != new_page); + + // Fill new_page's style map to capacity. When we move INTO this page + // with a style set, adjustCapacity will be triggered. + { + new_page.pauseIntegrityChecks(true); + defer new_page.pauseIntegrityChecks(false); + defer new_page.assertIntegrity(); + + var n: u24 = 1; + while (new_page.styles.add( + new_page.memory, + .{ .bg_color = .{ .rgb = @bitCast(n) } }, + )) |_| n += 1 else |_| {} + } + + // Move cursor to start of active area and set a style + s.cursorAbsolute(0, 0); + try s.setAttribute(.bold); + try testing.expect(s.cursor.style.flags.bold); + try testing.expect(s.cursor.style_id != style.default_id); + + // Find the row just before the page boundary + for (0..s.pages.rows - 1) |row| { + s.cursorAbsolute(0, @intCast(row)); + const cur_node = s.cursor.page_pin.node; + if (s.cursor.page_pin.down(1)) |next_pin| { + if (next_pin.node != cur_node) { + // Cursor is at 'row', moving down crosses to new_page + try testing.expect(&next_pin.node.data == new_page); + + // This cursorDown triggers the bug: the local page_pin copy + // becomes stale after adjustCapacity, causing rowAndCell() + // to access freed memory. + s.cursorDown(1); + + // If the fix is applied, verify correct state + try testing.expect(s.cursor.y == row + 1); + try testing.expect(s.cursor.style.flags.bold); + + break; + } + } + } else { + // Didn't find boundary + try testing.expect(false); + } +} + +test "Screen setAttribute increases capacity when style map is full" { + // Tests that setAttribute succeeds when the style map is full by + // increasing page capacity. When capacity is at max and increaseCapacity + // returns OutOfSpace, manualStyleUpdate will split the page instead. + const testing = std.testing; + const alloc = testing.allocator; + + // Use a small screen with multiple rows + var s = try init(alloc, .{ .cols = 10, .rows = 5, .max_scrollback = 10 }); + defer s.deinit(); + + // Write content to multiple rows + try s.testWriteString("line1\nline2\nline3\nline4\nline5"); + + // Get the page and fill its style map to capacity const page = &s.cursor.page_pin.node.data; + const original_styles_capacity = page.capacity.styles; - // We add unique styles to the page until no more will fit. - fill: for (0..255) |bg| { - for (0..255) |fg| { - const st: style.Style = .{ - .bg_color = .{ .palette = @intCast(bg) }, - .fg_color = .{ .palette = @intCast(fg) }, - }; + // Fill the style map to capacity using the StyleSet's layout capacity + // which accounts for the load factor + { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + defer page.assertIntegrity(); - s.cursor.style = st; - - // Try to insert the new style, if it doesn't fit then - // we succeeded in filling the style set, so we break. - s.cursor.style_id = page.styles.add( + const max_items = page.styles.layout.cap; + var n: usize = 1; + while (n < max_items) : (n += 1) { + _ = page.styles.add( page.memory, - s.cursor.style, - ) catch break :fill; - - try s.testWriteString("a"); + .{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } }, + ) catch break; } } - // Adjust the capacity, this should cause the style set to reach the - // same state it was in to begin with, since it will clone the page - // in the same order as the styles were added to begin with, meaning - // the cursor style will not be able to be added to the set, which - // should, right now, result in the cursor style being cleared. - _ = try s.adjustCapacity(s.cursor.page_pin.node, .{}); + // Now try to set a new unique attribute that would require a new style slot + // This should succeed by increasing capacity (or splitting if at max capacity) + try s.setAttribute(.bold); - // The cursor style should have been cleared by the `adjustCapacity`. - // - // This behavior will change, causing this test to fail, if either - // of these changes are made: - // - // - The adjustCapacity function is changed to ensure the - // new capacity will definitely fit the cursor style. - // - // - The cursor managed memory handling is reworked so that it - // doesn't reside in the pages anymore and doesn't need this - // accounting. - // - // In such a case, adjust this test accordingly. - try testing.expect(s.cursor.style.default()); - try testing.expectEqual(style.default_id, s.cursor.style_id); + // The style should have been applied (bold flag set) + try testing.expect(s.cursor.style.flags.bold); + + // The cursor should have a valid non-default style_id + try testing.expect(s.cursor.style_id != style.default_id); + + // Either the capacity increased or the page was split/changed + const current_page = &s.cursor.page_pin.node.data; + const capacity_increased = current_page.capacity.styles > original_styles_capacity; + const page_changed = current_page != page; + try testing.expect(capacity_increased or page_changed); +} + +test "Screen setAttribute splits page on OutOfSpace at max styles" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ + .cols = 10, + .rows = 10, + .max_scrollback = 0, + }); + defer s.deinit(); + + // Write content to multiple rows so we have something to split + try s.testWriteString("line1\nline2\nline3\nline4\nline5"); + + // Remember the original node + const original_node = s.cursor.page_pin.node; + + // Increase the page's style capacity to max by repeatedly calling increaseCapacity + // Use Screen.increaseCapacity to properly maintain cursor state + const max_styles = std.math.maxInt(size.CellCountInt); + while (s.cursor.page_pin.node.data.capacity.styles < max_styles) { + _ = s.increaseCapacity( + s.cursor.page_pin.node, + .styles, + ) catch break; + } + + // Get the page reference after increaseCapacity - cursor may have moved + var page = &s.cursor.page_pin.node.data; + try testing.expectEqual(max_styles, page.capacity.styles); + + // Fill the style map to capacity using the StyleSet's layout capacity + // which accounts for the load factor + { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + defer page.assertIntegrity(); + + const max_items = page.styles.layout.cap; + var n: usize = 1; + while (n < max_items) : (n += 1) { + _ = page.styles.add( + page.memory, + .{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } }, + ) catch break; + } + } + + // Track the node before setAttribute + const node_before_set = s.cursor.page_pin.node; + + // Now try to set a new unique attribute that would require a new style slot + // At max capacity, increaseCapacity will return OutOfSpace, triggering page split + try s.setAttribute(.bold); + + // The style should have been applied (bold flag set) + try testing.expect(s.cursor.style.flags.bold); + + // The cursor should have a valid non-default style_id + try testing.expect(s.cursor.style_id != style.default_id); + + // The page should have been split + const page_was_split = s.cursor.page_pin.node != node_before_set or + node_before_set.next != null or + node_before_set.prev != null or + s.cursor.page_pin.node != original_node; + try testing.expect(page_was_split); } diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index d717a9724..7e02e3a24 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -996,7 +996,7 @@ pub fn saveCursor(self: *Terminal) void { /// /// The primary and alternate screen have distinct save state. /// If no save was done before values are reset to their initial values. -pub fn restoreCursor(self: *Terminal) !void { +pub fn restoreCursor(self: *Terminal) void { const saved: Screen.SavedCursor = self.screens.active.saved_cursor orelse .{ .x = 0, .y = 0, @@ -1008,10 +1008,17 @@ pub fn restoreCursor(self: *Terminal) !void { }; // Set the style first because it can fail - const old_style = self.screens.active.cursor.style; self.screens.active.cursor.style = saved.style; - errdefer self.screens.active.cursor.style = old_style; - try self.screens.active.manualStyleUpdate(); + self.screens.active.manualStyleUpdate() catch |err| { + // Regardless of the error here, we revert back to an unstyled + // cursor. It is more important that the restore succeeds in + // other attributes because terminals have no way to communicate + // failure back. + log.warn("restoreCursor error updating style err={}", .{err}); + const screen: *Screen = self.screens.active; + screen.cursor.style = .{}; + self.screens.active.manualStyleUpdate() catch unreachable; + }; self.screens.active.charset = saved.charset; self.modes.set(.origin, saved.origin); @@ -1634,54 +1641,48 @@ pub fn insertLines(self: *Terminal, count: usize) void { self.scrolling_region.left, self.scrolling_region.right + 1, ) catch |err| { - const cap = dst_p.node.data.capacity; // Adjust our page capacity to make // room for we didn't have space for - _ = self.screens.active.adjustCapacity( + _ = self.screens.active.increaseCapacity( dst_p.node, switch (err) { // Rehash the sets error.StyleSetNeedsRehash, error.HyperlinkSetNeedsRehash, - => .{}, + => null, // Increase style memory error.StyleSetOutOfMemory, - => .{ .styles = cap.styles * 2 }, + => .styles, // Increase string memory error.StringAllocOutOfMemory, - => .{ .string_bytes = cap.string_bytes * 2 }, + => .string_bytes, // Increase hyperlink memory error.HyperlinkSetOutOfMemory, error.HyperlinkMapOutOfMemory, - => .{ .hyperlink_bytes = cap.hyperlink_bytes * 2 }, + => .hyperlink_bytes, // Increase grapheme memory error.GraphemeMapOutOfMemory, error.GraphemeAllocOutOfMemory, - => .{ .grapheme_bytes = cap.grapheme_bytes * 2 }, + => .grapheme_bytes, }, ) catch |e| switch (e) { - // This shouldn't be possible because above we're only - // adjusting capacity _upwards_. So it should have all - // the existing capacity it had to fit the adjusted - // data. Panic since we don't expect this. - error.StyleSetOutOfMemory, - error.StyleSetNeedsRehash, - error.StringAllocOutOfMemory, - error.HyperlinkSetOutOfMemory, - error.HyperlinkSetNeedsRehash, - error.HyperlinkMapOutOfMemory, - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => @panic("adjustCapacity resulted in capacity errors"), - - // The system allocator is OOM. We can't currently do - // anything graceful here. We panic. + // System OOM. We have no way to recover from this + // currently. We should probably change insertLines + // to raise an error here. error.OutOfMemory, - => @panic("adjustCapacity system allocator OOM"), + => @panic("increaseCapacity system allocator OOM"), + + // The page can't accommodate the managed memory required + // for this operation. We previously just corrupted + // memory here so a crash is better. The right long + // term solution is to allocate a new page here + // move this row to the new page, and start over. + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), }; // Continue the loop to try handling this row again. @@ -1834,49 +1835,41 @@ pub fn deleteLines(self: *Terminal, count: usize) void { self.scrolling_region.left, self.scrolling_region.right + 1, ) catch |err| { - const cap = dst_p.node.data.capacity; // Adjust our page capacity to make // room for we didn't have space for - _ = self.screens.active.adjustCapacity( + _ = self.screens.active.increaseCapacity( dst_p.node, switch (err) { // Rehash the sets error.StyleSetNeedsRehash, error.HyperlinkSetNeedsRehash, - => .{}, + => null, // Increase style memory error.StyleSetOutOfMemory, - => .{ .styles = cap.styles * 2 }, + => .styles, // Increase string memory error.StringAllocOutOfMemory, - => .{ .string_bytes = cap.string_bytes * 2 }, + => .string_bytes, // Increase hyperlink memory error.HyperlinkSetOutOfMemory, error.HyperlinkMapOutOfMemory, - => .{ .hyperlink_bytes = cap.hyperlink_bytes * 2 }, + => .hyperlink_bytes, // Increase grapheme memory error.GraphemeMapOutOfMemory, error.GraphemeAllocOutOfMemory, - => .{ .grapheme_bytes = cap.grapheme_bytes * 2 }, + => .grapheme_bytes, }, ) catch |e| switch (e) { - // See insertLines which has the same error capture. - error.StyleSetOutOfMemory, - error.StyleSetNeedsRehash, - error.StringAllocOutOfMemory, - error.HyperlinkSetOutOfMemory, - error.HyperlinkSetNeedsRehash, - error.HyperlinkMapOutOfMemory, - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => @panic("adjustCapacity resulted in capacity errors"), - + // See insertLines error.OutOfMemory, - => @panic("adjustCapacity system allocator OOM"), + => @panic("increaseCapacity system allocator OOM"), + + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), }; // Continue the loop to try handling this row again. @@ -2761,12 +2754,7 @@ pub fn switchScreenMode( } } else { assert(self.screens.active_key == .primary); - self.restoreCursor() catch |err| { - log.warn( - "restore cursor on switch screen failed to={} err={}", - .{ to, err }, - ); - }; + self.restoreCursor(); }, } } @@ -4821,7 +4809,7 @@ test "Terminal: horizontal tab back with cursor before left margin" { t.saveCursor(); t.modes.set(.enable_left_and_right_margin, true); t.setLeftAndRightMargin(5, 0); - try t.restoreCursor(); + t.restoreCursor(); try t.horizontalTabBack(); try t.print('X'); @@ -9887,7 +9875,7 @@ test "Terminal: saveCursor" { t.screens.active.charset.gr = .G0; try t.setAttribute(.{ .unset = {} }); t.modes.set(.origin, false); - try t.restoreCursor(); + t.restoreCursor(); try testing.expect(t.screens.active.cursor.style.flags.bold); try testing.expect(t.screens.active.charset.gr == .G3); try testing.expect(t.modes.get(.origin)); @@ -9903,7 +9891,7 @@ test "Terminal: saveCursor position" { t.saveCursor(); t.setCursorPos(1, 1); try t.print('B'); - try t.restoreCursor(); + t.restoreCursor(); try t.print('X'); { @@ -9923,7 +9911,7 @@ test "Terminal: saveCursor pending wrap state" { t.saveCursor(); t.setCursorPos(1, 1); try t.print('B'); - try t.restoreCursor(); + t.restoreCursor(); try t.print('X'); { @@ -9943,7 +9931,7 @@ test "Terminal: saveCursor origin mode" { t.modes.set(.enable_left_and_right_margin, true); t.setLeftAndRightMargin(3, 5); t.setTopAndBottomMargin(2, 4); - try t.restoreCursor(); + t.restoreCursor(); try t.print('X'); { @@ -9961,7 +9949,7 @@ test "Terminal: saveCursor resize" { t.setCursorPos(1, 10); t.saveCursor(); try t.resize(alloc, 5, 5); - try t.restoreCursor(); + t.restoreCursor(); try t.print('X'); { @@ -9982,7 +9970,7 @@ test "Terminal: saveCursor protected pen" { t.saveCursor(); t.setProtectedMode(.off); try testing.expect(!t.screens.active.cursor.protected); - try t.restoreCursor(); + t.restoreCursor(); try testing.expect(t.screens.active.cursor.protected); } @@ -9995,10 +9983,67 @@ test "Terminal: saveCursor doesn't modify hyperlink state" { const id = t.screens.active.cursor.hyperlink_id; t.saveCursor(); try testing.expectEqual(id, t.screens.active.cursor.hyperlink_id); - try t.restoreCursor(); + t.restoreCursor(); try testing.expectEqual(id, t.screens.active.cursor.hyperlink_id); } +test "Terminal: restoreCursor uses default style on OutOfSpace" { + // Tests that restoreCursor falls back to default style when + // manualStyleUpdate fails with OutOfSpace (can't split a 1-row page + // and styles are at max capacity). + const alloc = testing.allocator; + + // Use a single row so the page can't be split + var t = try init(alloc, .{ .cols = 10, .rows = 1 }); + defer t.deinit(alloc); + + // Set a style and save the cursor + try t.setAttribute(.{ .bold = {} }); + t.saveCursor(); + + // Clear the style + try t.setAttribute(.{ .unset = {} }); + try testing.expect(!t.screens.active.cursor.style.flags.bold); + + // Fill the style map to max capacity + const max_styles = std.math.maxInt(size.CellCountInt); + while (t.screens.active.cursor.page_pin.node.data.capacity.styles < max_styles) { + _ = t.screens.active.increaseCapacity( + t.screens.active.cursor.page_pin.node, + .styles, + ) catch break; + } + + const page = &t.screens.active.cursor.page_pin.node.data; + try testing.expectEqual(max_styles, page.capacity.styles); + + // Fill all style slots using the StyleSet's layout capacity which accounts + // for the load factor. The capacity in the layout is the actual max number + // of items that can be stored. + { + page.pauseIntegrityChecks(true); + defer page.pauseIntegrityChecks(false); + defer page.assertIntegrity(); + + const max_items = page.styles.layout.cap; + var n: usize = 1; + while (n < max_items) : (n += 1) { + _ = page.styles.add( + page.memory, + .{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } }, + ) catch break; + } + } + + // Restore cursor - should fall back to default style since page + // can't be split (1 row) and styles are at max capacity + t.restoreCursor(); + + // The style should be reset to default because OutOfSpace occurred + try testing.expect(!t.screens.active.cursor.style.flags.bold); + try testing.expectEqual(style.default_id, t.screens.active.cursor.style_id); +} + test "Terminal: setProtectedMode" { const alloc = testing.allocator; var t = try init(alloc, .{ .cols = 3, .rows = 3 }); @@ -11390,7 +11435,7 @@ test "Terminal: resize with reflow and saved cursor" { t.saveCursor(); try t.resize(alloc, 5, 3); - try t.restoreCursor(); + t.restoreCursor(); { const str = try t.plainString(testing.allocator); @@ -11431,7 +11476,7 @@ test "Terminal: resize with reflow and saved cursor pending wrap" { t.saveCursor(); try t.resize(alloc, 5, 3); - try t.restoreCursor(); + t.restoreCursor(); { const str = try t.plainString(testing.allocator); diff --git a/src/terminal/bitmap_allocator.zig b/src/terminal/bitmap_allocator.zig index 258d73071..23a5048e1 100644 --- a/src/terminal/bitmap_allocator.zig +++ b/src/terminal/bitmap_allocator.zig @@ -63,6 +63,14 @@ pub fn BitmapAllocator(comptime chunk_size: comptime_int) type { }; } + /// Returns the number of bytes required to allocate n elements of + /// type T. This accounts for the chunk size alignment used by the + /// bitmap allocator. + pub fn bytesRequired(comptime T: type, n: usize) usize { + const byte_count = @sizeOf(T) * n; + return alignForward(usize, byte_count, chunk_size); + } + /// Allocate n elements of type T. This will return error.OutOfMemory /// if there isn't enough space in the backing buffer. /// @@ -955,3 +963,45 @@ test "BitmapAllocator alloc and free two 1.5 bitmaps offset 0.75" { bm.bitmap.ptr(buf)[0..4], ); } + +test "BitmapAllocator bytesRequired" { + const testing = std.testing; + + // Chunk size of 16 bytes (like grapheme_chunk in page.zig) + { + const Alloc = BitmapAllocator(16); + + // Single byte rounds up to chunk size + try testing.expectEqual(16, Alloc.bytesRequired(u8, 1)); + try testing.expectEqual(16, Alloc.bytesRequired(u8, 16)); + try testing.expectEqual(32, Alloc.bytesRequired(u8, 17)); + + // u21 (4 bytes each) + try testing.expectEqual(16, Alloc.bytesRequired(u21, 1)); // 4 bytes -> 16 + try testing.expectEqual(16, Alloc.bytesRequired(u21, 4)); // 16 bytes -> 16 + try testing.expectEqual(32, Alloc.bytesRequired(u21, 5)); // 20 bytes -> 32 + try testing.expectEqual(32, Alloc.bytesRequired(u21, 6)); // 24 bytes -> 32 + } + + // Chunk size of 4 bytes + { + const Alloc = BitmapAllocator(4); + + try testing.expectEqual(4, Alloc.bytesRequired(u8, 1)); + try testing.expectEqual(4, Alloc.bytesRequired(u8, 4)); + try testing.expectEqual(8, Alloc.bytesRequired(u8, 5)); + + // u32 (4 bytes each) - exactly one chunk per element + try testing.expectEqual(4, Alloc.bytesRequired(u32, 1)); + try testing.expectEqual(8, Alloc.bytesRequired(u32, 2)); + } + + // Chunk size of 32 bytes (like string_chunk in page.zig) + { + const Alloc = BitmapAllocator(32); + + try testing.expectEqual(32, Alloc.bytesRequired(u8, 1)); + try testing.expectEqual(32, Alloc.bytesRequired(u8, 32)); + try testing.expectEqual(64, Alloc.bytesRequired(u8, 33)); + } +} diff --git a/src/terminal/hyperlink.zig b/src/terminal/hyperlink.zig index 975e6f30e..94f86466c 100644 --- a/src/terminal/hyperlink.zig +++ b/src/terminal/hyperlink.zig @@ -13,8 +13,9 @@ const autoHash = std.hash.autoHash; const autoHashStrat = std.hash.autoHashStrat; /// The unique identifier for a hyperlink. This is at most the number of cells -/// that can fit in a single terminal page. -pub const Id = size.CellCountInt; +/// that can fit in a single terminal page, since each cell can only contain +/// at most one hyperlink. +pub const Id = size.HyperlinkCountInt; // The mapping of cell to hyperlink. We use an offset hash map to save space // since its very unlikely a cell is a hyperlink, so its a waste to store diff --git a/src/terminal/osc/parsers/semantic_prompt.zig b/src/terminal/osc/parsers/semantic_prompt.zig index 510fe3447..652fe34da 100644 --- a/src/terminal/osc/parsers/semantic_prompt.zig +++ b/src/terminal/osc/parsers/semantic_prompt.zig @@ -33,64 +33,69 @@ pub fn parse(parser: *Parser, _: ?u8) ?*Command { return null; }; while (it.next()) |kv| { - if (std.mem.eql(u8, kv.key, "aid")) { + const key = kv.key orelse continue; + if (std.mem.eql(u8, key, "aid")) { parser.command.prompt_start.aid = kv.value; - } else if (std.mem.eql(u8, kv.key, "redraw")) redraw: { + } else if (std.mem.eql(u8, key, "redraw")) redraw: { // https://sw.kovidgoyal.net/kitty/shell-integration/#notes-for-shell-developers // Kitty supports a "redraw" option for prompt_start. I can't find // this documented anywhere but can see in the code that this is used // by shell environments to tell the terminal that the shell will NOT // redraw the prompt so we should attempt to resize it. parser.command.prompt_start.redraw = (value: { - if (kv.value.len != 1) break :value null; - switch (kv.value[0]) { + const value = kv.value orelse break :value null; + if (value.len != 1) break :value null; + switch (value[0]) { '0' => break :value false, '1' => break :value true, else => break :value null, } }) orelse { - log.info("OSC 133 A: invalid redraw value: {s}", .{kv.value}); + log.info("OSC 133 A: invalid redraw value: {?s}", .{kv.value}); break :redraw; }; - } else if (std.mem.eql(u8, kv.key, "special_key")) redraw: { + } else if (std.mem.eql(u8, key, "special_key")) redraw: { // https://sw.kovidgoyal.net/kitty/shell-integration/#notes-for-shell-developers parser.command.prompt_start.special_key = (value: { - if (kv.value.len != 1) break :value null; - switch (kv.value[0]) { + const value = kv.value orelse break :value null; + if (value.len != 1) break :value null; + switch (value[0]) { '0' => break :value false, '1' => break :value true, else => break :value null, } }) orelse { - log.info("OSC 133 A invalid special_key value: {s}", .{kv.value}); + log.info("OSC 133 A invalid special_key value: {?s}", .{kv.value}); break :redraw; }; - } else if (std.mem.eql(u8, kv.key, "click_events")) redraw: { + } else if (std.mem.eql(u8, key, "click_events")) redraw: { // https://sw.kovidgoyal.net/kitty/shell-integration/#notes-for-shell-developers parser.command.prompt_start.click_events = (value: { - if (kv.value.len != 1) break :value null; - switch (kv.value[0]) { + const value = kv.value orelse break :value null; + if (value.len != 1) break :value null; + switch (value[0]) { '0' => break :value false, '1' => break :value true, else => break :value null, } }) orelse { - log.info("OSC 133 A invalid click_events value: {s}", .{kv.value}); + log.info("OSC 133 A invalid click_events value: {?s}", .{kv.value}); break :redraw; }; - } else if (std.mem.eql(u8, kv.key, "k")) k: { + } else if (std.mem.eql(u8, key, "k")) k: { // The "k" marks the kind of prompt, or "primary" if we don't know. // This can be used to distinguish between the first (initial) prompt, // a continuation, etc. - if (kv.value.len != 1) break :k; - parser.command.prompt_start.kind = switch (kv.value[0]) { + const value = kv.value orelse break :k; + if (value.len != 1) break :k; + parser.command.prompt_start.kind = switch (value[0]) { 'c' => .continuation, 's' => .secondary, 'r' => .right, 'i' => .primary, else => .primary, }; - } else log.info("OSC 133 A: unknown semantic prompt option: {s}", .{kv.key}); + } else log.info("OSC 133 A: unknown semantic prompt option: {?s}", .{kv.key}); } }, 'B' => prompt_end: { @@ -105,7 +110,7 @@ pub fn parse(parser: *Parser, _: ?u8) ?*Command { return null; }; while (it.next()) |kv| { - log.info("OSC 133 B: unknown semantic prompt option: {s}", .{kv.key}); + log.info("OSC 133 B: unknown semantic prompt option: {?s}", .{kv.key}); } }, 'C' => end_of_input: { @@ -122,12 +127,13 @@ pub fn parse(parser: *Parser, _: ?u8) ?*Command { return null; }; while (it.next()) |kv| { - if (std.mem.eql(u8, kv.key, "cmdline")) { - parser.command.end_of_input.cmdline = string_encoding.printfQDecode(kv.value) catch null; - } else if (std.mem.eql(u8, kv.key, "cmdline_url")) { - parser.command.end_of_input.cmdline = string_encoding.urlPercentDecode(kv.value) catch null; + const key = kv.key orelse continue; + if (std.mem.eql(u8, key, "cmdline")) { + parser.command.end_of_input.cmdline = if (kv.value) |value| string_encoding.printfQDecode(value) catch null else null; + } else if (std.mem.eql(u8, key, "cmdline_url")) { + parser.command.end_of_input.cmdline = if (kv.value) |value| string_encoding.urlPercentDecode(value) catch null else null; } else { - log.info("OSC 133 C: unknown semantic prompt option: {s}", .{kv.key}); + log.info("OSC 133 C: unknown semantic prompt option: {s}", .{key}); } } }, @@ -159,8 +165,8 @@ const SemanticPromptKVIterator = struct { string: []u8, pub const SemanticPromptKV = struct { - key: [:0]u8, - value: [:0]u8, + key: ?[:0]u8, + value: ?[:0]u8, }; pub fn init(writer: *std.Io.Writer) std.Io.Writer.Error!SemanticPromptKVIterator { @@ -186,8 +192,24 @@ const SemanticPromptKVIterator = struct { break :kv kv; }; + // If we have an empty item, we return a null key and value. + // + // This allows for trailing semicolons, but also lets us parse + // (or rather, ignore) empty fields; for example `a=b;;e=f`. + if (kv.len < 1) return .{ + .key = null, + .value = null, + }; + const key = key: { - const index = std.mem.indexOfScalar(u8, kv, '=') orelse break :key kv; + const index = std.mem.indexOfScalar(u8, kv, '=') orelse { + // If there is no '=' return entire `kv` string as the key and + // a null value. + return .{ + .key = kv, + .value = null, + }; + }; kv[index] = 0; const key = kv[0..index :0]; break :key key; @@ -348,6 +370,18 @@ test "OSC 133: prompt_start with special_key empty" { try testing.expect(cmd.prompt_start.special_key == false); } +test "OSC 133: prompt_start with trailing ;" { + const testing = std.testing; + + var p: Parser = .init(null); + + const input = "133;A;"; + for (input) |ch| p.next(ch); + + const cmd = p.end(null).?.*; + try testing.expect(cmd == .prompt_start); +} + test "OSC 133: prompt_start with click_events true" { const testing = std.testing; @@ -387,6 +421,36 @@ test "OSC 133: prompt_start with click_events empty" { try testing.expect(cmd.prompt_start.click_events == false); } +test "OSC 133: prompt_start with click_events bare key" { + const testing = std.testing; + + var p: Parser = .init(null); + + const input = "133;A;click_events"; + for (input) |ch| p.next(ch); + + const cmd = p.end(null).?.*; + try testing.expect(cmd == .prompt_start); + try testing.expect(cmd.prompt_start.click_events == false); +} + +test "OSC 133: prompt_start with invalid bare key" { + const testing = std.testing; + + var p: Parser = .init(null); + + const input = "133;A;barekey"; + for (input) |ch| p.next(ch); + + const cmd = p.end(null).?.*; + try testing.expect(cmd == .prompt_start); + try testing.expect(cmd.prompt_start.aid == null); + try testing.expectEqual(.primary, cmd.prompt_start.kind); + try testing.expect(cmd.prompt_start.redraw == true); + try testing.expect(cmd.prompt_start.special_key == false); + try testing.expect(cmd.prompt_start.click_events == false); +} + test "OSC 133: end_of_command no exit code" { const testing = std.testing; @@ -692,3 +756,16 @@ test "OSC 133: end_of_input with cmdline_url 9" { try testing.expect(cmd == .end_of_input); try testing.expect(cmd.end_of_input.cmdline == null); } + +test "OSC 133: end_of_input with bare key" { + const testing = std.testing; + + var p: Parser = .init(null); + + const input = "133;C;cmdline_url"; + for (input) |ch| p.next(ch); + + const cmd = p.end(null).?.*; + try testing.expect(cmd == .end_of_input); + try testing.expect(cmd.end_of_input.cmdline == null); +} diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 848123405..6a5958681 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -633,6 +633,114 @@ pub const Page = struct { HyperlinkError || GraphemeError; + /// Compute the exact capacity required to store a range of rows from + /// this page. + /// + /// The returned capacity will have the same number of columns as this + /// page and the number of rows equal to the range given. The returned + /// capacity is by definition strictly less than or equal to this + /// page's capacity, so the layout is guaranteed to succeed. + /// + /// Preconditions: + /// - Range must be at least 1 row + /// - Start and end must be valid for this page + pub fn exactRowCapacity( + self: *const Page, + y_start: usize, + y_end: usize, + ) Capacity { + assert(y_start < y_end); + assert(y_end <= self.size.rows); + + // Track unique IDs using a bitset. Both style IDs and hyperlink IDs + // are CellCountInt (u16), so we reuse this set for both to save + // stack memory (~8KB instead of ~16KB). + const CellCountSet = std.StaticBitSet(std.math.maxInt(size.CellCountInt) + 1); + comptime assert(size.StyleCountInt == size.CellCountInt); + comptime assert(size.HyperlinkCountInt == size.CellCountInt); + + // Accumulators + var id_set: CellCountSet = .initEmpty(); + var grapheme_bytes: usize = 0; + var string_bytes: usize = 0; + + // First pass: count styles and grapheme bytes + const rows = self.rows.ptr(self.memory)[y_start..y_end]; + for (rows) |*row| { + const cells = row.cells.ptr(self.memory)[0..self.size.cols]; + for (cells) |*cell| { + if (cell.style_id != stylepkg.default_id) { + id_set.set(cell.style_id); + } + + if (cell.hasGrapheme()) { + if (self.lookupGrapheme(cell)) |cps| { + grapheme_bytes += GraphemeAlloc.bytesRequired(u21, cps.len); + } + } + } + } + const styles_cap = StyleSet.capacityForCount(id_set.count()); + + // Second pass: count hyperlinks and string bytes + // We count both unique hyperlinks (for hyperlink_set) and total + // hyperlink cells (for hyperlink_map capacity). + id_set = .initEmpty(); + var hyperlink_cells: usize = 0; + for (rows) |*row| { + const cells = row.cells.ptr(self.memory)[0..self.size.cols]; + for (cells) |*cell| { + if (cell.hyperlink) { + hyperlink_cells += 1; + if (self.lookupHyperlink(cell)) |id| { + // Only count each unique hyperlink once for set sizing + if (!id_set.isSet(id)) { + id_set.set(id); + + // Get the hyperlink entry to compute string bytes + const entry = self.hyperlink_set.get(self.memory, id); + string_bytes += StringAlloc.bytesRequired(u8, entry.uri.len); + + switch (entry.id) { + .implicit => {}, + .explicit => |slice| { + string_bytes += StringAlloc.bytesRequired(u8, slice.len); + }, + } + } + } + } + } + } + + // The hyperlink_map capacity in layout() is computed as: + // hyperlink_count * hyperlink_cell_multiplier (rounded to power of 2) + // We need enough hyperlink_bytes so that when layout() computes + // the map capacity, it can accommodate all hyperlink cells. This + // is unit tested. + const hyperlink_cap = cap: { + const hyperlink_count = id_set.count(); + const hyperlink_set_cap = hyperlink.Set.capacityForCount(hyperlink_count); + const hyperlink_map_min = std.math.divCeil( + usize, + hyperlink_cells, + hyperlink_cell_multiplier, + ) catch 0; + break :cap @max(hyperlink_set_cap, hyperlink_map_min); + }; + + // All the intCasts below are safe because we should have a + // capacity strictly less than or equal to this page's capacity. + return .{ + .cols = self.size.cols, + .rows = @intCast(y_end - y_start), + .styles = @intCast(styles_cap), + .grapheme_bytes = @intCast(grapheme_bytes), + .hyperlink_bytes = @intCast(hyperlink_cap * @sizeOf(hyperlink.Set.Item)), + .string_bytes = @intCast(string_bytes), + }; + } + /// Clone the contents of another page into this page. The capacities /// can be different, but the size of the other page must fit into /// this page. @@ -1569,7 +1677,13 @@ pub const Page = struct { const grapheme_alloc_start = alignForward(usize, styles_end, GraphemeAlloc.base_align.toByteUnits()); const grapheme_alloc_end = grapheme_alloc_start + grapheme_alloc_layout.total_size; - const grapheme_count = @divFloor(cap.grapheme_bytes, grapheme_chunk); + const grapheme_count: usize = count: { + if (cap.grapheme_bytes == 0) break :count 0; + // Use divCeil to match GraphemeAlloc.layout() which uses alignForward, + // ensuring grapheme_map has capacity when grapheme_alloc has chunks. + const base = std.math.divCeil(usize, cap.grapheme_bytes, grapheme_chunk) catch unreachable; + break :count std.math.ceilPowerOfTwo(usize, base) catch unreachable; + }; const grapheme_map_layout = GraphemeMap.layout(@intCast(grapheme_count)); const grapheme_map_start = alignForward(usize, grapheme_alloc_end, GraphemeMap.base_align.toByteUnits()); const grapheme_map_end = grapheme_map_start + grapheme_map_layout.total_size; @@ -1639,25 +1753,33 @@ pub const Size = struct { }; /// Capacity of this page. +/// +/// This capacity can be maxed out (every field max) and still fit +/// within a 64-bit memory space. If you need more than this, you will +/// need to split data across separate pages. +/// +/// For 32-bit systems, it is possible to overflow the addressable +/// space and this is something we still need to address in the future +/// likely by limiting the maximum capacity on 32-bit systems further. pub const Capacity = struct { /// Number of columns and rows we can know about. cols: size.CellCountInt, rows: size.CellCountInt, /// Number of unique styles that can be used on this page. - styles: usize = 16, + styles: size.StyleCountInt = 16, /// Number of bytes to allocate for hyperlink data. Note that the /// amount of data used for hyperlinks in total is more than this because /// hyperlinks use string data as well as a small amount of lookup metadata. /// This number is a rough approximation. - hyperlink_bytes: usize = hyperlink_bytes_default, + hyperlink_bytes: size.HyperlinkCountInt = hyperlink_bytes_default, /// Number of bytes to allocate for grapheme data. - grapheme_bytes: usize = grapheme_bytes_default, + grapheme_bytes: size.GraphemeBytesInt = grapheme_bytes_default, /// Number of bytes to allocate for strings. - string_bytes: usize = string_bytes_default, + string_bytes: size.StringBytesInt = string_bytes_default, pub const Adjustment = struct { cols: ?size.CellCountInt = null, @@ -2025,6 +2147,21 @@ pub const Cell = packed struct(u64) { // //const pages = total_size / std.heap.page_size_min; // } +test "Page.layout can take a maxed capacity" { + // Our intention is for a maxed-out capacity to always fit + // within a page layout without triggering runtime safety on any + // overflow. This simplifies some of our handling downstream of the + // call (relevant to: https://github.com/ghostty-org/ghostty/issues/10258) + var cap: Capacity = undefined; + inline for (@typeInfo(Capacity).@"struct".fields) |field| { + @field(cap, field.name) = std.math.maxInt(field.type); + } + + // Note that a max capacity will exceed our max_page_size so we + // can't init a page with it, but it should layout. + _ = Page.layout(cap); +} + test "Cell is zero by default" { const cell = Cell.init(0); const cell_int: u64 = @bitCast(cell); @@ -3191,3 +3328,512 @@ test "Page verifyIntegrity zero cols" { page.verifyIntegrity(testing.allocator), ); } + +test "Page exactRowCapacity empty rows" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + .hyperlink_bytes = 32 * @sizeOf(hyperlink.Set.Item), + .string_bytes = 512, + }); + defer page.deinit(); + + // Empty page: all capacity fields should be 0 (except cols/rows) + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(10, cap.cols); + try testing.expectEqual(5, cap.rows); + try testing.expectEqual(0, cap.styles); + try testing.expectEqual(0, cap.grapheme_bytes); + try testing.expectEqual(0, cap.hyperlink_bytes); + try testing.expectEqual(0, cap.string_bytes); +} + +test "Page exactRowCapacity styles" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + }); + defer page.deinit(); + + // No styles: capacity should be 0 + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(0, cap.styles); + } + + // Add one style to a cell + const style1_id = try page.styles.add(page.memory, .{ .flags = .{ .bold = true } }); + { + const rac = page.getRowAndCell(0, 0); + rac.row.styled = true; + rac.cell.style_id = style1_id; + } + + // One unique style - capacity accounts for load factor + const cap_one_style = page.exactRowCapacity(0, 5); + { + try testing.expectEqual(StyleSet.capacityForCount(1), cap_one_style.styles); + } + + // Add same style to another cell (duplicate) - capacity unchanged + { + const rac = page.getRowAndCell(1, 0); + rac.cell.style_id = style1_id; + } + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(cap_one_style.styles, cap.styles); + } + + // Add a different style + const style2_id = try page.styles.add(page.memory, .{ .flags = .{ .italic = true } }); + { + const rac = page.getRowAndCell(2, 0); + rac.cell.style_id = style2_id; + } + + // Two unique styles - capacity accounts for load factor + const cap_two_styles = page.exactRowCapacity(0, 5); + { + try testing.expectEqual(StyleSet.capacityForCount(2), cap_two_styles.styles); + try testing.expect(cap_two_styles.styles > cap_one_style.styles); + } + + // Style outside the row range should not be counted + { + const rac = page.getRowAndCell(0, 7); + rac.row.styled = true; + rac.cell.style_id = try page.styles.add(page.memory, .{ .flags = .{ .underline = .single } }); + } + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(cap_two_styles.styles, cap.styles); + } + + // Full range includes the new style + { + const cap = page.exactRowCapacity(0, 10); + try testing.expectEqual(StyleSet.capacityForCount(3), cap.styles); + } + + // Verify clone works with exact capacity and produces same result + { + const cap = page.exactRowCapacity(0, 5); + var cloned = try Page.init(cap); + defer cloned.deinit(); + for (0..5) |y| { + const src_row = &page.rows.ptr(page.memory)[y]; + const dst_row = &cloned.rows.ptr(cloned.memory)[y]; + try cloned.cloneRowFrom(&page, dst_row, src_row); + } + const cloned_cap = cloned.exactRowCapacity(0, 5); + try testing.expectEqual(cap, cloned_cap); + } +} + +test "Page exactRowCapacity single style clone" { + // Regression test: verify a single style can be cloned with exact capacity. + // This tests that capacityForCount properly accounts for ID 0 being reserved. + var page = try Page.init(.{ + .cols = 10, + .rows = 2, + .styles = 8, + }); + defer page.deinit(); + + // Add exactly one style to row 0 + const style_id = try page.styles.add(page.memory, .{ .flags = .{ .bold = true } }); + { + const rac = page.getRowAndCell(0, 0); + rac.row.styled = true; + rac.cell.style_id = style_id; + } + + // exactRowCapacity for just row 0 should give capacity for 1 style + const cap = page.exactRowCapacity(0, 1); + try testing.expectEqual(StyleSet.capacityForCount(1), cap.styles); + + // Create a new page with exact capacity and clone + var cloned = try Page.init(cap); + defer cloned.deinit(); + + const src_row = &page.rows.ptr(page.memory)[0]; + const dst_row = &cloned.rows.ptr(cloned.memory)[0]; + + // This must not fail with StyleSetOutOfMemory + try cloned.cloneRowFrom(&page, dst_row, src_row); + + // Verify the style was cloned correctly + const cloned_cell = &cloned.rows.ptr(cloned.memory)[0].cells.ptr(cloned.memory)[0]; + try testing.expect(cloned_cell.style_id != stylepkg.default_id); +} + +test "Page exactRowCapacity styles max single row" { + var page = try Page.init(.{ + .cols = std.math.maxInt(size.CellCountInt), + .rows = 1, + .styles = std.math.maxInt(size.StyleCountInt), + }); + defer page.deinit(); + + // Style our first row + const row = &page.rows.ptr(page.memory)[0]; + row.styled = true; + + // Fill cells with styles until we get OOM, but limit to a reasonable count + // to avoid overflow when computing capacityForCount near maxInt + const cells = row.cells.ptr(page.memory)[0..page.size.cols]; + var count: usize = 0; + const max_count: usize = 1000; // Limit to avoid overflow in capacity calculation + for (cells, 0..) |*cell, i| { + if (count >= max_count) break; + const style_id = page.styles.add(page.memory, .{ + .fg_color = .{ .rgb = .{ + .r = @intCast(i & 0xFF), + .g = @intCast((i >> 8) & 0xFF), + .b = 0, + } }, + }) catch break; + cell.style_id = style_id; + count += 1; + } + + // Verify we added a meaningful number of styles + try testing.expect(count > 0); + + // Capacity should be at least count (adjusted for load factor) + const cap = page.exactRowCapacity(0, 1); + try testing.expectEqual(StyleSet.capacityForCount(count), cap.styles); +} + +test "Page exactRowCapacity grapheme_bytes" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + }); + defer page.deinit(); + + // No graphemes: capacity should be 0 + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(0, cap.grapheme_bytes); + } + + // Add one grapheme (1 codepoint) to a cell - rounds up to grapheme_chunk + { + const rac = page.getRowAndCell(0, 0); + rac.cell.* = .init('a'); + try page.appendGrapheme(rac.row, rac.cell, 0x0301); // combining acute accent + } + { + const cap = page.exactRowCapacity(0, 5); + // 1 codepoint = 4 bytes, rounds up to grapheme_chunk (16) + try testing.expectEqual(grapheme_chunk, cap.grapheme_bytes); + } + + // Add another grapheme to a different cell - should sum + { + const rac = page.getRowAndCell(1, 0); + rac.cell.* = .init('e'); + try page.appendGrapheme(rac.row, rac.cell, 0x0300); // combining grave accent + } + { + const cap = page.exactRowCapacity(0, 5); + // 2 graphemes, each 1 codepoint = 2 * grapheme_chunk + try testing.expectEqual(grapheme_chunk * 2, cap.grapheme_bytes); + } + + // Add a larger grapheme (multiple codepoints) that fits in one chunk + { + const rac = page.getRowAndCell(2, 0); + rac.cell.* = .init('o'); + try page.appendGrapheme(rac.row, rac.cell, 0x0301); + try page.appendGrapheme(rac.row, rac.cell, 0x0302); + try page.appendGrapheme(rac.row, rac.cell, 0x0303); + } + { + const cap = page.exactRowCapacity(0, 5); + // First two cells: 2 * grapheme_chunk + // Third cell: 3 codepoints = 12 bytes, rounds up to grapheme_chunk + try testing.expectEqual(grapheme_chunk * 3, cap.grapheme_bytes); + } + + // Grapheme outside the row range should not be counted + { + const rac = page.getRowAndCell(0, 7); + rac.cell.* = .init('x'); + try page.appendGrapheme(rac.row, rac.cell, 0x0304); + } + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(grapheme_chunk * 3, cap.grapheme_bytes); + } + + // Full range includes the new grapheme + { + const cap = page.exactRowCapacity(0, 10); + try testing.expectEqual(grapheme_chunk * 4, cap.grapheme_bytes); + } + + // Verify clone works with exact capacity and produces same result + { + const cap = page.exactRowCapacity(0, 5); + var cloned = try Page.init(cap); + defer cloned.deinit(); + for (0..5) |y| { + const src_row = &page.rows.ptr(page.memory)[y]; + const dst_row = &cloned.rows.ptr(cloned.memory)[y]; + try cloned.cloneRowFrom(&page, dst_row, src_row); + } + const cloned_cap = cloned.exactRowCapacity(0, 5); + try testing.expectEqual(cap, cloned_cap); + } +} + +test "Page exactRowCapacity grapheme_bytes larger than chunk" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + }); + defer page.deinit(); + + // Add a grapheme larger than one chunk (grapheme_chunk_len = 4 codepoints) + const rac = page.getRowAndCell(0, 0); + rac.cell.* = .init('a'); + + // Add 6 codepoints - requires 2 chunks (6 * 4 = 24 bytes, rounds up to 32) + for (0..6) |i| { + try page.appendGrapheme(rac.row, rac.cell, @intCast(0x0300 + i)); + } + + const cap = page.exactRowCapacity(0, 1); + // 6 codepoints = 24 bytes, alignForward(24, 16) = 32 + try testing.expectEqual(32, cap.grapheme_bytes); + + // Verify clone works with exact capacity and produces same result + var cloned = try Page.init(cap); + defer cloned.deinit(); + const src_row = &page.rows.ptr(page.memory)[0]; + const dst_row = &cloned.rows.ptr(cloned.memory)[0]; + try cloned.cloneRowFrom(&page, dst_row, src_row); + const cloned_cap = cloned.exactRowCapacity(0, 1); + try testing.expectEqual(cap, cloned_cap); +} + +test "Page exactRowCapacity hyperlinks" { + var page = try Page.init(.{ + .cols = 10, + .rows = 10, + .styles = 8, + .hyperlink_bytes = 32 * @sizeOf(hyperlink.Set.Item), + .string_bytes = 512, + }); + defer page.deinit(); + + // No hyperlinks: capacity should be 0 + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(0, cap.hyperlink_bytes); + try testing.expectEqual(0, cap.string_bytes); + } + + // Add one hyperlink with implicit ID + const uri1 = "https://example.com"; + const id1 = blk: { + const rac = page.getRowAndCell(0, 0); + + // Create and add hyperlink entry + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = uri1, + }); + try page.setHyperlink(rac.row, rac.cell, id); + break :blk id; + }; + // 1 hyperlink - capacity accounts for load factor + const cap_one_link = page.exactRowCapacity(0, 5); + { + try testing.expectEqual(hyperlink.Set.capacityForCount(1) * @sizeOf(hyperlink.Set.Item), cap_one_link.hyperlink_bytes); + // URI "https://example.com" = 19 bytes, rounds up to string_chunk (32) + try testing.expectEqual(string_chunk, cap_one_link.string_bytes); + } + + // Add same hyperlink to another cell (duplicate ID) - capacity unchanged + { + const rac = page.getRowAndCell(1, 0); + + // Use the same hyperlink ID for another cell + page.hyperlink_set.use(page.memory, id1); + try page.setHyperlink(rac.row, rac.cell, id1); + } + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(cap_one_link.hyperlink_bytes, cap.hyperlink_bytes); + try testing.expectEqual(cap_one_link.string_bytes, cap.string_bytes); + } + + // Add a different hyperlink with explicit ID + const uri2 = "https://other.example.org/path"; + const explicit_id = "my-link-id"; + { + const rac = page.getRowAndCell(2, 0); + + const id = try page.insertHyperlink(.{ + .id = .{ .explicit = explicit_id }, + .uri = uri2, + }); + try page.setHyperlink(rac.row, rac.cell, id); + } + // 2 hyperlinks - capacity accounts for load factor + const cap_two_links = page.exactRowCapacity(0, 5); + { + try testing.expectEqual(hyperlink.Set.capacityForCount(2) * @sizeOf(hyperlink.Set.Item), cap_two_links.hyperlink_bytes); + // First URI: 19 bytes -> 32, Second URI: 30 bytes -> 32, Explicit ID: 10 bytes -> 32 + try testing.expectEqual(string_chunk * 3, cap_two_links.string_bytes); + } + + // Hyperlink outside the row range should not be counted + { + const rac = page.getRowAndCell(0, 7); // row 7 is outside range [0, 5) + + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = 99 }, + .uri = "https://outside.example.com", + }); + try page.setHyperlink(rac.row, rac.cell, id); + } + { + const cap = page.exactRowCapacity(0, 5); + try testing.expectEqual(cap_two_links.hyperlink_bytes, cap.hyperlink_bytes); + try testing.expectEqual(cap_two_links.string_bytes, cap.string_bytes); + } + + // Full range includes the new hyperlink + { + const cap = page.exactRowCapacity(0, 10); + try testing.expectEqual(hyperlink.Set.capacityForCount(3) * @sizeOf(hyperlink.Set.Item), cap.hyperlink_bytes); + // Third URI: 27 bytes -> 32 + try testing.expectEqual(string_chunk * 4, cap.string_bytes); + } + + // Verify clone works with exact capacity and produces same result + { + const cap = page.exactRowCapacity(0, 5); + var cloned = try Page.init(cap); + defer cloned.deinit(); + for (0..5) |y| { + const src_row = &page.rows.ptr(page.memory)[y]; + const dst_row = &cloned.rows.ptr(cloned.memory)[y]; + try cloned.cloneRowFrom(&page, dst_row, src_row); + } + const cloned_cap = cloned.exactRowCapacity(0, 5); + try testing.expectEqual(cap, cloned_cap); + } +} + +test "Page exactRowCapacity single hyperlink clone" { + // Regression test: verify a single hyperlink can be cloned with exact capacity. + // This tests that capacityForCount properly accounts for ID 0 being reserved. + var page = try Page.init(.{ + .cols = 10, + .rows = 2, + .styles = 8, + .hyperlink_bytes = 32 * @sizeOf(hyperlink.Set.Item), + .string_bytes = 512, + }); + defer page.deinit(); + + // Add exactly one hyperlink to row 0 + const uri = "https://example.com"; + const id = blk: { + const rac = page.getRowAndCell(0, 0); + const link_id = try page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = uri, + }); + try page.setHyperlink(rac.row, rac.cell, link_id); + break :blk link_id; + }; + _ = id; + + // exactRowCapacity for just row 0 should give capacity for 1 hyperlink + const cap = page.exactRowCapacity(0, 1); + try testing.expectEqual(hyperlink.Set.capacityForCount(1) * @sizeOf(hyperlink.Set.Item), cap.hyperlink_bytes); + + // Create a new page with exact capacity and clone + var cloned = try Page.init(cap); + defer cloned.deinit(); + + const src_row = &page.rows.ptr(page.memory)[0]; + const dst_row = &cloned.rows.ptr(cloned.memory)[0]; + + // This must not fail with HyperlinkSetOutOfMemory + try cloned.cloneRowFrom(&page, dst_row, src_row); + + // Verify the hyperlink was cloned correctly + const cloned_cell = &cloned.rows.ptr(cloned.memory)[0].cells.ptr(cloned.memory)[0]; + try testing.expect(cloned_cell.hyperlink); +} + +test "Page exactRowCapacity hyperlink map capacity for many cells" { + // A single hyperlink spanning many cells requires hyperlink_map capacity + // based on cell count, not unique hyperlink count. + const cols = 50; + var page = try Page.init(.{ + .cols = cols, + .rows = 2, + .styles = 8, + .hyperlink_bytes = 32 * @sizeOf(hyperlink.Set.Item), + .string_bytes = 512, + }); + defer page.deinit(); + + // Add one hyperlink spanning all 50 columns in row 0 + const uri = "https://example.com"; + const id = blk: { + const rac = page.getRowAndCell(0, 0); + const link_id = try page.insertHyperlink(.{ + .id = .{ .implicit = 1 }, + .uri = uri, + }); + try page.setHyperlink(rac.row, rac.cell, link_id); + break :blk link_id; + }; + + // Apply same hyperlink to remaining cells in row 0 + for (1..cols) |x| { + const rac = page.getRowAndCell(@intCast(x), 0); + page.hyperlink_set.use(page.memory, id); + try page.setHyperlink(rac.row, rac.cell, id); + } + + // exactRowCapacity must account for 50 hyperlink cells, not just 1 unique hyperlink + const cap = page.exactRowCapacity(0, 1); + + // The hyperlink_bytes must be large enough that layout() computes sufficient + // hyperlink_map capacity. With hyperlink_cell_multiplier=16, we need at least + // ceil(50/16) = 4 hyperlink entries worth of bytes for the map. + const min_for_map = std.math.divCeil(usize, cols, hyperlink_cell_multiplier) catch 0; + const min_hyperlink_bytes = min_for_map * @sizeOf(hyperlink.Set.Item); + try testing.expect(cap.hyperlink_bytes >= min_hyperlink_bytes); + + // Create a new page with exact capacity and clone - must not fail + var cloned = try Page.init(cap); + defer cloned.deinit(); + + const src_row = &page.rows.ptr(page.memory)[0]; + const dst_row = &cloned.rows.ptr(cloned.memory)[0]; + + // This must not fail with HyperlinkMapOutOfMemory + try cloned.cloneRowFrom(&page, dst_row, src_row); + + // Verify all hyperlinks were cloned correctly + for (0..cols) |x| { + const cloned_cell = &cloned.rows.ptr(cloned.memory)[0].cells.ptr(cloned.memory)[x]; + try testing.expect(cloned_cell.hyperlink); + } +} diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index 883dd2f0d..8040039ae 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -64,6 +64,20 @@ pub fn RefCountedSet( @alignOf(Id), )); + /// This is the max load until the set returns OutOfMemory and + /// requires more capacity. + /// + /// Experimentally, this load factor works quite well. + pub const load_factor = 0.8125; + + /// Returns the minimum capacity needed to store `n` items, + /// accounting for the load factor and the reserved ID 0. + pub fn capacityForCount(n: usize) usize { + if (n == 0) return 0; + // +1 because ID 0 is reserved, so we need at least n+1 slots. + return @intFromFloat(@ceil(@as(f64, @floatFromInt(n + 1)) / load_factor)); + } + /// Set item pub const Item = struct { /// The value this item represents. @@ -154,9 +168,6 @@ pub fn RefCountedSet( /// The returned layout `cap` property will be 1 more than the number /// of items that the set can actually store, since ID 0 is reserved. pub fn init(cap: usize) Layout { - // Experimentally, this load factor works quite well. - const load_factor = 0.8125; - assert(cap <= @as(usize, @intCast(std.math.maxInt(Id))) + 1); // Zero-cap set is valid, return special case diff --git a/src/terminal/size.zig b/src/terminal/size.zig index 0dedfcc14..7be09739e 100644 --- a/src/terminal/size.zig +++ b/src/terminal/size.zig @@ -11,9 +11,32 @@ pub const max_page_size = std.math.maxInt(u32); /// derived from the maximum terminal page size. pub const OffsetInt = std.math.IntFittingRange(0, max_page_size - 1); -/// The int type that can contain the maximum number of cells in a page. -pub const CellCountInt = u16; // TODO: derive +/// Int types for maximum values of things. A lot of these sizes are +/// based on "X is enough for any reasonable use case" principles. +// The goal is that a user can have the maxInt amount of all of these +// present at one time and be able to address them in a single Page.zig. + +// Total number of cells that are possible in each dimension (row/col). +// Based on 2^16 being enough for any reasonable terminal size and allowing +// IDs to remain 16-bit. +pub const CellCountInt = u16; + +// Total number of styles and hyperlinks that are possible in a page. +// We match CellCountInt here because each cell in a single row can have at +// most one style, making it simple to split a page by splitting rows. // +// Note due to the way RefCountedSet works, we are short one value, but +// this is a theoretical limit we accept. A page with a single row max +// columns wide would be one short of having every cell have a unique style. +pub const StyleCountInt = CellCountInt; +pub const HyperlinkCountInt = CellCountInt; + +// Total number of bytes that can be taken up by grapheme data and string +// data. Both of these technically unlimited with malicious input, but +// we choose a reasonable limit of 2^32 (4GB) per. +pub const GraphemeBytesInt = u32; +pub const StringBytesInt = u32; + /// The offset from the base address of the page to the start of some data. /// This is typed for ease of use. /// diff --git a/src/terminal/stream_readonly.zig b/src/terminal/stream_readonly.zig index 1ee4f3f08..9b4999116 100644 --- a/src/terminal/stream_readonly.zig +++ b/src/terminal/stream_readonly.zig @@ -125,7 +125,7 @@ pub const Handler = struct { } }, .save_cursor => self.terminal.saveCursor(), - .restore_cursor => try self.terminal.restoreCursor(), + .restore_cursor => self.terminal.restoreCursor(), .invoke_charset => self.terminal.invokeCharset(value.bank, value.charset, value.locking), .configure_charset => self.terminal.configureCharset(value.slot, value.charset), .set_attribute => switch (value) { @@ -240,7 +240,7 @@ pub const Handler = struct { .save_cursor => if (enabled) { self.terminal.saveCursor(); } else { - try self.terminal.restoreCursor(); + self.terminal.restoreCursor(); }, .enable_mode_3 => {}, diff --git a/src/terminal/style.zig b/src/terminal/style.zig index e5c47b9fe..7908beefa 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -11,7 +11,7 @@ const RefCountedSet = @import("ref_counted_set.zig").RefCountedSet; /// The unique identifier for a style. This is at most the number of cells /// that can fit into a terminal page. -pub const Id = size.CellCountInt; +pub const Id = size.StyleCountInt; /// The Id to use for default styling. pub const default_id: Id = 0; diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index c647e3ba2..2b92c19e3 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -398,11 +398,16 @@ pub const StreamHandler = struct { break :tmux; }, - .exit => if (self.tmux_viewer) |viewer| { - // Free our viewer state - viewer.deinit(); - self.alloc.destroy(viewer); - self.tmux_viewer = null; + .exit => { + // Free our viewer state if we have one + if (self.tmux_viewer) |viewer| { + viewer.deinit(); + self.alloc.destroy(viewer); + self.tmux_viewer = null; + } + + // And always break since we assert below + // that we're not handling an exit command. break :tmux; }, @@ -716,7 +721,7 @@ pub const StreamHandler = struct { if (enabled) { self.terminal.saveCursor(); } else { - try self.terminal.restoreCursor(); + self.terminal.restoreCursor(); } }, @@ -928,7 +933,7 @@ pub const StreamHandler = struct { } pub inline fn restoreCursor(self: *StreamHandler) !void { - try self.terminal.restoreCursor(); + self.terminal.restoreCursor(); } pub fn enquiry(self: *StreamHandler) !void {