From e2133cbd92f5f8e8189820148ee987e554e881a3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 4 Jul 2024 09:42:46 -0700 Subject: [PATCH] terminal: row needs hyperlink state, test clearing hyperlink --- src/terminal/Terminal.zig | 43 +++++++++++++++++++++++++++++++++++++-- src/terminal/page.zig | 31 ++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 4b9899da9..51d7a7fb1 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -607,7 +607,11 @@ fn printCell( if (self.screen.cursor.hyperlink_id > 0) { // If we have a hyperlink configured, apply it to this cell var page = &self.screen.cursor.page_pin.page.data; - page.setHyperlink(cell, self.screen.cursor.hyperlink_id) catch |err| { + page.setHyperlink( + self.screen.cursor.page_row, + cell, + self.screen.cursor.hyperlink_id, + ) catch |err| { // TODO: an error can only happen if our page is out of space // so realloc the page here. log.err("failed to set hyperlink, ignoring err={}", .{err}); @@ -615,7 +619,7 @@ fn printCell( } else if (cell.hyperlink) { // If the previous cell had a hyperlink then we need to clear it. var page = &self.screen.cursor.page_pin.page.data; - page.clearHyperlink(cell); + page.clearHyperlink(self.screen.cursor.page_row, cell); } // We don't need to update the style refs unless the @@ -3807,6 +3811,8 @@ test "Terminal: print with hyperlink" { .x = @intCast(x), .y = 0, } }).?; + const row = list_cell.row; + try testing.expect(row.hyperlink); const cell = list_cell.cell; try testing.expect(cell.hyperlink); const id = list_cell.page.data.lookupHyperlink(cell).?; @@ -3832,6 +3838,8 @@ test "Terminal: print and end hyperlink" { .x = @intCast(x), .y = 0, } }).?; + const row = list_cell.row; + try testing.expect(row.hyperlink); const cell = list_cell.cell; try testing.expect(cell.hyperlink); const id = list_cell.page.data.lookupHyperlink(cell).?; @@ -3842,6 +3850,8 @@ test "Terminal: print and end hyperlink" { .x = @intCast(x), .y = 0, } }).?; + const row = list_cell.row; + try testing.expect(row.hyperlink); const cell = list_cell.cell; try testing.expect(!cell.hyperlink); } @@ -3884,6 +3894,35 @@ test "Terminal: print and change hyperlink" { try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); } +test "Terminal: overwrite hyperlink" { + var t = try init(testing.allocator, .{ .cols = 80, .rows = 80 }); + defer t.deinit(testing.allocator); + + // Setup our hyperlink and print + try t.screen.startHyperlink("http://one.example.com", null); + try t.printString("123"); + t.setCursorPos(1, 1); + t.screen.endHyperlink(); + try t.printString("456"); + + // Verify all our cells have a hyperlink + for (0..3) |x| { + const list_cell = t.screen.pages.getCell(.{ .screen = .{ + .x = @intCast(x), + .y = 0, + } }).?; + const page = &list_cell.page.data; + const row = list_cell.row; + try testing.expect(!row.hyperlink); + const cell = list_cell.cell; + try testing.expect(!cell.hyperlink); + try testing.expect(page.lookupHyperlink(cell) == null); + try testing.expectEqual(0, page.hyperlink_set.count()); + } + + try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); +} + test "Terminal: linefeed and carriage return" { var t = try init(testing.allocator, .{ .cols = 80, .rows = 80 }); defer t.deinit(testing.allocator); diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 86d3fefb4..2aa6f78e3 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -838,12 +838,20 @@ pub const Page = struct { defer self.assertIntegrity(); const cells = row.cells.ptr(self.memory)[left..end]; + if (row.grapheme) { for (cells) |*cell| { if (cell.hasGrapheme()) self.clearGrapheme(row, cell); } } + if (row.hyperlink) { + row.hyperlink = false; + for (cells) |*cell| { + if (cell.hyperlink) self.clearHyperlink(row, cell); + } + } + if (row.styled) { for (cells) |*cell| { if (cell.style_id == style.default_id) continue; @@ -867,7 +875,7 @@ pub const Page = struct { } /// Clear the hyperlink from the given cell. - pub fn clearHyperlink(self: *Page, cell: *Cell) void { + pub fn clearHyperlink(self: *Page, row: *Row, cell: *Cell) void { defer self.assertIntegrity(); // Get our ID @@ -875,17 +883,22 @@ pub const Page = struct { var map = self.hyperlink_map.map(self.memory); const entry = map.getEntry(cell_offset) orelse return; - // Release our usage of this + // Release our usage of this, free memory, unset flag self.hyperlink_set.release(self.memory, entry.value_ptr.*); - - // Free the memory map.removeByPtr(entry.key_ptr); + cell.hyperlink = false; + + // Mark that we no longer have graphemes, also search the row + // to make sure its state is correct. + const cells = row.cells.ptr(self.memory)[0..self.size.cols]; + for (cells) |c| if (c.hyperlink) return; + row.hyperlink = false; } /// Set the hyperlink for the given cell. If the cell already has a /// hyperlink, then this will handle memory management for the prior /// hyperlink. - pub fn setHyperlink(self: *Page, cell: *Cell, id: hyperlink.Id) !void { + pub fn setHyperlink(self: *Page, row: *Row, cell: *Cell, id: hyperlink.Id) !void { defer self.assertIntegrity(); const cell_offset = getOffset(Cell, self.memory, cell); @@ -904,6 +917,7 @@ pub const Page = struct { self.hyperlink_set.use(self.memory, id); gop.value_ptr.* = id; cell.hyperlink = true; + row.hyperlink = true; } /// Append a codepoint to the given cell as a grapheme. @@ -1280,11 +1294,16 @@ pub const Row = packed struct(u64) { /// At the time of writing this, the speed difference is around 4x. styled: bool = false, + /// True if any of the cells in this row are part of a hyperlink. + /// This is similar to styled: it can have false positives but never + /// false negatives. This is used to optimize hyperlink operations. + hyperlink: bool = false, + /// The semantic prompt type for this row as specified by the /// running program, or "unknown" if it was never set. semantic_prompt: SemanticPrompt = .unknown, - _padding: u25 = 0, + _padding: u24 = 0, /// Semantic prompt type. pub const SemanticPrompt = enum(u3) {