From 30bba9bf067ac9384ae1a93d28f8e3e97515f8ee Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Tue, 27 Aug 2024 00:54:22 -0400
Subject: [PATCH 01/11] terminal: move refcount responsibility out of
 setHyperlink

avoids double counting in several places
---
 src/terminal/Screen.zig |  3 ++-
 src/terminal/page.zig   | 23 ++++++++++++++++-------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index 2d87c6012..afb1510a5 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -1755,7 +1755,8 @@ pub fn cursorSetHyperlink(self: *Screen) !void {
         self.cursor.page_cell,
         self.cursor.hyperlink_id,
     )) {
-        // Success!
+        // Success, increase the refcount for the hyperlink.
+        page.hyperlink_set.use(page.memory, self.cursor.hyperlink_id);
         return;
     } else |err| switch (err) {
         // hyperlink_map is out of space, realloc the page to be larger
diff --git a/src/terminal/page.zig b/src/terminal/page.zig
index 18a5601b7..2110b531f 100644
--- a/src/terminal/page.zig
+++ b/src/terminal/page.zig
@@ -1080,8 +1080,14 @@ pub const Page = struct {
     }
 
     /// Set the hyperlink for the given cell. If the cell already has a
-    /// hyperlink, then this will handle memory management for the prior
-    /// hyperlink.
+    /// hyperlink, then this will handle memory management and refcount
+    /// update for the prior hyperlink.
+    ///
+    /// DOES NOT increment the reference count for the new hyperlink!
+    ///
+    /// Caller is responsible for updating the refcount in the hyperlink
+    /// set as necessary by calling `use` if the id was not acquired with
+    /// `add`.
     pub fn setHyperlink(self: *Page, row: *Row, cell: *Cell, id: hyperlink.Id) !void {
         defer self.assertIntegrity();
 
@@ -1090,6 +1096,13 @@ pub const Page = struct {
         const gop = try map.getOrPut(cell_offset);
 
         if (gop.found_existing) {
+            // Always release the old hyperlink, because even if it's actually
+            // the same as the one we're setting, we'd end up double-counting
+            // if we left the reference count be, because the caller does not
+            // know whether it's the same and will have increased the count
+            // outside of this function.
+            self.hyperlink_set.release(self.memory, gop.value_ptr.*);
+
             // If the hyperlink matches then we don't need to do anything.
             if (gop.value_ptr.* == id) {
                 // It is possible for cell hyperlink to be false but row
@@ -1102,13 +1115,9 @@ pub const Page = struct {
                 cell.hyperlink = true;
                 return;
             }
-
-            // Different hyperlink, we need to release the old one
-            self.hyperlink_set.release(self.memory, gop.value_ptr.*);
         }
 
-        // Increase ref count for our new hyperlink and set it
-        self.hyperlink_set.use(self.memory, id);
+        // Set the hyperlink on the cell and in the map.
         gop.value_ptr.* = id;
         cell.hyperlink = true;
         row.hyperlink = true;

From a6992baa30ce8c323f9b2f363b9445071dc7840f Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Tue, 27 Aug 2024 00:56:46 -0400
Subject: [PATCH 02/11] fix(terminal): don't MOVE hyperlinks in
 clonePartialRowFrom

---
 src/terminal/page.zig | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/terminal/page.zig b/src/terminal/page.zig
index 2110b531f..5f44d28a4 100644
--- a/src/terminal/page.zig
+++ b/src/terminal/page.zig
@@ -745,15 +745,17 @@ pub const Page = struct {
                 if (src_cell.hyperlink) hyperlink: {
                     dst_row.hyperlink = true;
 
-                    // Fast-path: same page we can move it directly
+                    const id = other.lookupHyperlink(src_cell).?;
+
+                    // Fast-path: same page we can add with the same id.
                     if (other == self) {
-                        self.moveHyperlink(src_cell, dst_cell);
+                        self.hyperlink_set.use(self.memory, id);
+                        try self.setHyperlink(dst_row, dst_cell, id);
                         break :hyperlink;
                     }
 
                     // Slow-path: get the hyperlink from the other page,
                     // add it, and migrate.
-                    const id = other.lookupHyperlink(src_cell).?;
 
                     const dst_link = dst_link: {
                         // Fast path is we just dupe the hyperlink because

From d43d5b26ee6625c03e7040e6a5614cd6cbba4eff Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Tue, 27 Aug 2024 01:00:12 -0400
Subject: [PATCH 03/11] fix(terminal): avoid trying to clone bad managed memory
 in reflow

If we call `moveLastRowToNewPage` at any point because we failed to copy
some managed memory, it tries to copy managed memory that hasn't been
cloned yet when moving our progress to a new page.

Avoid this by setting our content tag, hyperlink flag, and style id to
indicate no managed memory is present on the cell.
---
 src/terminal/PageList.zig | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig
index 523b0e195..5b960a8c1 100644
--- a/src/terminal/PageList.zig
+++ b/src/terminal/PageList.zig
@@ -948,17 +948,15 @@ const ReflowCursor = struct {
                 },
             }
 
-            // Prevent integrity checks from tripping
-            // while copying graphemes and hyperlinks.
-            if (comptime std.debug.runtime_safety) {
-                self.page_cell.style_id = stylepkg.default_id;
-            }
+            // These will create issues by trying to clone managed memory that
+            // isn't set if the current dst row needs to be moved to a new page.
+            // They'll be fixed once we do properly copy the relevant memory.
+            self.page_cell.content_tag = .codepoint;
+            self.page_cell.hyperlink = false;
+            self.page_cell.style_id = stylepkg.default_id;
 
             // Copy grapheme data.
             if (cell.content_tag == .codepoint_grapheme) {
-                // The tag is asserted to be .codepoint in setGraphemes.
-                self.page_cell.content_tag = .codepoint;
-
                 // Copy the graphemes
                 const cps = src_page.lookupGrapheme(cell).?;
 
@@ -981,8 +979,6 @@ const ReflowCursor = struct {
                     }
                 }
 
-                self.page_row.grapheme = true;
-
                 // This shouldn't fail since we made sure we have space above.
                 try self.page.setGraphemes(self.page_row, self.page_cell, cps);
             }
@@ -1015,8 +1011,6 @@ const ReflowCursor = struct {
                     );
                 } orelse src_id;
 
-                self.page_row.hyperlink = true;
-
                 // We expect this to succeed due to the
                 // hyperlinkCapacity check we did before.
                 try self.page.setHyperlink(

From cb1cb3526a1fc54a78e795cf83b71ac2efaa20bb Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Wed, 21 Aug 2024 19:32:49 -0400
Subject: [PATCH 04/11] test(Screen): add failing tests for `cursorCopy`
 hyperlink handling

---
 src/terminal/Screen.zig | 118 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index afb1510a5..354e074c8 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -2998,6 +2998,124 @@ test "Screen cursorCopy style copy" {
     try testing.expectEqual(@as(usize, 1), page.styles.count());
 }
 
+test "Screen cursorCopy hyperlink deref" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try Screen.init(alloc, 10, 10, 0);
+    defer s.deinit();
+
+    var s2 = try Screen.init(alloc, 10, 10, 0);
+    defer s2.deinit();
+    const page = &s2.cursor.page_pin.page.data;
+
+    // Create a hyperlink for the cursor.
+    try s2.startHyperlink("https://example.com/", null);
+    try testing.expectEqual(@as(usize, 1), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id != 0);
+
+    // Copy a cursor with no hyperlink, should release our hyperlink.
+    try s2.cursorCopy(s.cursor);
+    try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id == 0);
+}
+
+test "Screen cursorCopy hyperlink deref new page" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try init(alloc, 10, 10, 0);
+    defer s.deinit();
+
+    var s2 = try Screen.init(alloc, 10, 10, 2048);
+    defer s2.deinit();
+
+    // We need to get the cursor on a new page.
+    const first_page_size = s2.pages.pages.first.?.data.capacity.rows;
+
+    // Fill the scrollback with blank lines until
+    // there are only 5 rows left on the first page.
+    for (0..first_page_size - 5) |_| {
+        try s2.testWriteString("\n");
+    }
+
+    try s2.testWriteString("1\n2\n3\n4\n5\n6\n7\n8\n9\n10");
+
+    // s2.pages.diagram(...):
+    //
+    //      +----------+ = PAGE 0
+    //  ... :          :
+    //     +-------------+ ACTIVE
+    // 4300 |1         | | 0
+    // 4301 |2         | | 1
+    // 4302 |3         | | 2
+    // 4303 |4         | | 3
+    // 4304 |5         | | 4
+    //      +----------+ :
+    //      +----------+ : = PAGE 1
+    //    0 |6         | | 5
+    //    1 |7         | | 6
+    //    2 |8         | | 7
+    //    3 |9         | | 8
+    //    4 |10        | | 9
+    //      :  ^       : : = PIN 0
+    //      +----------+ :
+    //     +-------------+
+
+    // This should be PAGE 1
+    const page = &s2.cursor.page_pin.page.data;
+
+    // It should be the last page in the list.
+    try testing.expectEqual(&s2.pages.pages.last.?.data, page);
+    // It should have a previous page.
+    try testing.expect(s2.cursor.page_pin.page.prev != null);
+
+    // The cursor should be at 2, 9
+    try testing.expect(s2.cursor.x == 2);
+    try testing.expect(s2.cursor.y == 9);
+
+    // Create a hyperlink for the cursor, should be in page 1.
+    try s2.startHyperlink("https://example.com/", null);
+    try testing.expectEqual(@as(usize, 1), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id != 0);
+
+    // Copy the cursor for the first screen. This should release
+    // the hyperlink from page 1 and move the cursor back to page 0.
+    try s2.cursorCopy(s.cursor);
+    try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id == 0);
+    // The page after the page the cursor is now in should be page 1.
+    try testing.expectEqual(page, &s2.cursor.page_pin.page.next.?.data);
+    // The cursor should be at 0, 0
+    try testing.expect(s2.cursor.x == 0);
+    try testing.expect(s2.cursor.y == 0);
+}
+
+test "Screen cursorCopy hyperlink copy" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try Screen.init(alloc, 10, 10, 0);
+    defer s.deinit();
+
+    // Create a hyperlink for the cursor.
+    try s.startHyperlink("https://example.com/", null);
+    try testing.expectEqual(@as(usize, 1), s.cursor.page_pin.page.data.hyperlink_set.count());
+    try testing.expect(s.cursor.hyperlink_id != 0);
+
+    var s2 = try Screen.init(alloc, 10, 10, 0);
+    defer s2.deinit();
+    const page = &s2.cursor.page_pin.page.data;
+
+    try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id == 0);
+
+    // Copy the cursor with the hyperlink.
+    try s2.cursorCopy(s.cursor);
+    try testing.expectEqual(@as(usize, 1), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id != 0);
+}
+
 test "Screen style basics" {
     const testing = std.testing;
     const alloc = testing.allocator;

From 170f55aa8400e076c1023ed9a61d19d5b1a8f0b5 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:56:01 -0400
Subject: [PATCH 05/11] Screen: update `cursorCopy` to handle hyperlink state

---
 src/terminal/Screen.zig | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index 354e074c8..a120f8d20 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -906,6 +906,9 @@ pub fn cursorCopy(self: *Screen, other: Cursor) !void {
     assert(other.x < self.pages.cols);
     assert(other.y < self.pages.rows);
 
+    // End any currently active hyperlink on our cursor.
+    self.endHyperlink();
+
     const old = self.cursor;
     self.cursor = other;
     errdefer self.cursor = old;
@@ -913,6 +916,10 @@ pub fn cursorCopy(self: *Screen, other: Cursor) !void {
     // Keep our old style ID so it can be properly cleaned up below.
     self.cursor.style_id = old.style_id;
 
+    // Hyperlinks will be managed separately below.
+    self.cursor.hyperlink_id = 0;
+    self.cursor.hyperlink = null;
+
     // Keep our old page pin and X/Y because:
     // 1. The old style will need to be cleaned up from the page it's from.
     // 2. The new position navigated to by `cursorAbsolute` needs to be in our
@@ -927,6 +934,27 @@ pub fn cursorCopy(self: *Screen, other: Cursor) !void {
 
     // Move to the correct location to match the other cursor.
     self.cursorAbsolute(other.x, other.y);
+
+    // If the other cursor had a hyperlink, add it to ours.
+    if (other.hyperlink_id != 0) {
+        // Get the hyperlink from the other cursor's page.
+        const other_page = &other.page_pin.page.data;
+        const other_link = other_page.hyperlink_set.get(other_page.memory, other.hyperlink_id);
+
+        const uri = other_link.uri.offset.ptr(other_page.memory)[0..other_link.uri.len];
+        const id_ = switch (other_link.id) {
+            .explicit => |id| id.offset.ptr(other_page.memory)[0..id.len],
+            .implicit => null,
+        };
+
+        // And it to our cursor.
+        self.startHyperlink(uri, id_) catch |err| {
+            // This shouldn't happen because startHyperlink should handle
+            // resizing. This only happens if we're truly out of RAM. Degrade
+            // to forgetting the hyperlink.
+            log.err("failed to update hyperlink on cursor change err={}", .{err});
+        };
+    }
 }
 
 /// Always use this to write to cursor.page_pin.*.

From 38bb9b40a65648c95201db11b18ea1e7ebc6a8b5 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:57:37 -0400
Subject: [PATCH 06/11] Terminal: release hyperlink before copying cursor when
 switching screen

To avoid an unnecessary copy.
---
 src/terminal/Terminal.zig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index 667e28650..c0b37b939 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -2472,14 +2472,14 @@ pub fn alternateScreen(
     // Mark our terminal as dirty
     self.flags.dirty.clear = true;
 
+    // We always end hyperlink state
+    self.screen.endHyperlink();
+
     // Bring our pen with us
     self.screen.cursorCopy(old.cursor) catch |err| {
         log.warn("cursor copy failed entering alt screen err={}", .{err});
     };
 
-    // We always end hyperlink state
-    self.screen.endHyperlink();
-
     if (options.clear_on_enter) {
         self.eraseDisplay(.complete, false);
     }

From 5714c2feedcfe7d73a640f05dd062f31f86cacc9 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Thu, 22 Aug 2024 20:59:52 -0400
Subject: [PATCH 07/11] PageList: refactor `clone` to avoid excess work

Also avoids leaving content in out-of-bounds rows, since it doesn't copy
the content to them in the first place. Over all, just a lot cleaner.
---
 src/terminal/PageList.zig | 84 ++++-----------------------------------
 1 file changed, 8 insertions(+), 76 deletions(-)

diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig
index 5b960a8c1..c0ff5a548 100644
--- a/src/terminal/PageList.zig
+++ b/src/terminal/PageList.zig
@@ -416,93 +416,25 @@ pub fn clone(
             chunk.page.data.capacity,
             &page_size,
         );
-        assert(page.data.capacity.rows >= chunk.page.data.capacity.rows);
+        assert(page.data.capacity.rows >= chunk.end - chunk.start);
         defer page.data.assertIntegrity();
-        page.data.size.rows = chunk.page.data.size.rows;
+        page.data.size.rows = chunk.end - chunk.start;
         try page.data.cloneFrom(
             &chunk.page.data,
-            0,
-            chunk.page.data.size.rows,
+            chunk.start,
+            chunk.end,
         );
 
         page_list.append(page);
 
-        // If this is a full page then we're done.
-        if (chunk.fullPage()) {
-            total_rows += page.data.size.rows;
+        total_rows += page.data.size.rows;
 
-            // Updating tracked pins is easy, we just change the page
-            // pointer but all offsets remain the same.
-            if (opts.tracked_pins) |remap| {
-                const pin_keys = self.tracked_pins.keys();
-                for (pin_keys) |p| {
-                    if (p.page != chunk.page) continue;
-                    const new_p = try pool.pins.create();
-                    new_p.* = p.*;
-                    new_p.page = page;
-                    try remap.putNoClobber(p, new_p);
-                    try tracked_pins.putNoClobber(pool.alloc, new_p, {});
-                }
-            }
-
-            continue;
-        }
-
-        // If this is just a shortened chunk off the end we can just
-        // shorten the size. We don't worry about clearing memory here because
-        // as the page grows the memory will be reclaimable because the data
-        // is still valid.
-        if (chunk.start == 0) {
-            page.data.size.rows = @intCast(chunk.end);
-            total_rows += chunk.end;
-
-            // Updating tracked pins for the pins that are in the shortened chunk.
-            if (opts.tracked_pins) |remap| {
-                const pin_keys = self.tracked_pins.keys();
-                for (pin_keys) |p| {
-                    if (p.page != chunk.page or
-                        p.y >= chunk.end) continue;
-                    const new_p = try pool.pins.create();
-                    new_p.* = p.*;
-                    new_p.page = page;
-                    try remap.putNoClobber(p, new_p);
-                    try tracked_pins.putNoClobber(pool.alloc, new_p, {});
-                }
-            }
-
-            continue;
-        }
-
-        // We want to maintain the dirty bits from the original page so
-        // instead of setting a range we grab the dirty bit and then
-        // set it on the new page in the new location.
-        var dirty = page.data.dirtyBitSet();
-
-        // Kind of slow, we want to shift the rows up in the page up to
-        // end and then resize down.
-        const rows = page.data.rows.ptr(page.data.memory);
-        const len = chunk.end - chunk.start;
-        for (0..len) |i| {
-            const src: *Row = &rows[i + chunk.start];
-            const dst: *Row = &rows[i];
-            const old_dst = dst.*;
-            dst.* = src.*;
-            src.* = old_dst;
-            dirty.setValue(i, dirty.isSet(i + chunk.start));
-        }
-
-        // We need to clear the rows we're about to truncate.
-        for (len..page.data.size.rows) |i| {
-            page.data.clearCells(&rows[i], 0, page.data.size.cols);
-        }
-
-        page.data.size.rows = @intCast(len);
-        total_rows += len;
-
-        // Updating tracked pins
+        // Remap our tracked pins by changing the page and
+        // offsetting the Y position based on the chunk start.
         if (opts.tracked_pins) |remap| {
             const pin_keys = self.tracked_pins.keys();
             for (pin_keys) |p| {
+                // We're only interested in pins that were within the chunk.
                 if (p.page != chunk.page or
                     p.y < chunk.start or
                     p.y >= chunk.end) continue;

From ff0c1141da6bdb4402ef91907f82f675611d19a3 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Thu, 22 Aug 2024 21:00:15 -0400
Subject: [PATCH 08/11] renderer: add `updateFrame` critical region timings for
 dev benchmark

---
 src/renderer/Metal.zig | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index 89666e7bb..1629678f6 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -891,6 +891,14 @@ pub fn updateFrame(
 
     // Update all our data as tightly as possible within the mutex.
     var critical: Critical = critical: {
+        // const start = try std.time.Instant.now();
+        // const start_micro = std.time.microTimestamp();
+        // defer {
+        //     const end = std.time.Instant.now() catch unreachable;
+        //     // "[updateFrame critical time] <START us>\t<TIME_TAKEN us>"
+        //     std.log.err("[updateFrame critical time] {}\t{}", .{start_micro, end.since(start) / std.time.ns_per_us});
+        // }
+
         state.mutex.lock();
         defer state.mutex.unlock();
 

From a3fb96f543e807c107299e895c97d43e9fc87489 Mon Sep 17 00:00:00 2001
From: Qwerasd <qwerasd205@users.noreply.github.com>
Date: Sat, 24 Aug 2024 23:18:25 -0400
Subject: [PATCH 09/11] this should be a doc comment

---
 src/terminal/Terminal.zig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index c0b37b939..899718a74 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -1198,12 +1198,12 @@ pub fn reverseIndex(self: *Terminal) void {
     self.scrollDown(1);
 }
 
-// Set Cursor Position. Move cursor to the position indicated
-// by row and column (1-indexed). If column is 0, it is adjusted to 1.
-// If column is greater than the right-most column it is adjusted to
-// the right-most column. If row is 0, it is adjusted to 1. If row is
-// greater than the bottom-most row it is adjusted to the bottom-most
-// row.
+/// Set Cursor Position. Move cursor to the position indicated
+/// by row and column (1-indexed). If column is 0, it is adjusted to 1.
+/// If column is greater than the right-most column it is adjusted to
+/// the right-most column. If row is 0, it is adjusted to 1. If row is
+/// greater than the bottom-most row it is adjusted to the bottom-most
+/// row.
 pub fn setCursorPos(self: *Terminal, row_req: usize, col_req: usize) void {
     // If cursor origin mode is set the cursor row will be moved relative to
     // the top margin row and adjusted to be above or at bottom-most row in

From 6a2d57edfd5ee93f0bbd4c2bdf80d5e2c583f385 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <m@mitchellh.com>
Date: Wed, 28 Aug 2024 09:41:40 -0700
Subject: [PATCH 10/11] terminal: cursorCopy has option to not copy hyperlink

---
 src/terminal/Screen.zig   | 47 +++++++++++++++++++++++++++++++--------
 src/terminal/Terminal.zig |  7 +++---
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index a120f8d20..c6118627c 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -902,7 +902,11 @@ fn cursorDownOrScroll(self: *Screen) !void {
 
 /// Copy another cursor. The cursor can be on any screen but the x/y
 /// must be within our screen bounds.
-pub fn cursorCopy(self: *Screen, other: Cursor) !void {
+pub fn cursorCopy(self: *Screen, other: Cursor, opts: struct {
+    /// Copy the hyperlink from the other cursor. If not set, this will
+    /// clear our current hyperlink.
+    hyperlink: bool = true,
+}) !void {
     assert(other.x < self.pages.cols);
     assert(other.y < self.pages.rows);
 
@@ -936,7 +940,7 @@ pub fn cursorCopy(self: *Screen, other: Cursor) !void {
     self.cursorAbsolute(other.x, other.y);
 
     // If the other cursor had a hyperlink, add it to ours.
-    if (other.hyperlink_id != 0) {
+    if (opts.hyperlink and other.hyperlink_id != 0) {
         // Get the hyperlink from the other cursor's page.
         const other_page = &other.page_pin.page.data;
         const other_link = other_page.hyperlink_set.get(other_page.memory, other.hyperlink_id);
@@ -2905,7 +2909,7 @@ test "Screen cursorCopy x/y" {
 
     var s2 = try Screen.init(alloc, 10, 10, 0);
     defer s2.deinit();
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expect(s2.cursor.x == 2);
     try testing.expect(s2.cursor.y == 3);
     try s2.testWriteString("Hello");
@@ -2934,7 +2938,7 @@ test "Screen cursorCopy style deref" {
     try testing.expect(s2.cursor.style.flags.bold);
 
     // Copy default style, should release our style
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expect(!s2.cursor.style.flags.bold);
     try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
@@ -3000,7 +3004,7 @@ test "Screen cursorCopy style deref new page" {
 
     // Copy the cursor for the first screen. This should release
     // the style from page 1 and move the cursor back to page 0.
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expect(!s2.cursor.style.flags.bold);
     try testing.expectEqual(@as(usize, 0), page.styles.count());
     // The page after the page the cursor is now in should be page 1.
@@ -3021,7 +3025,7 @@ test "Screen cursorCopy style copy" {
     var s2 = try Screen.init(alloc, 10, 10, 0);
     defer s2.deinit();
     const page = &s2.cursor.page_pin.page.data;
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expect(s2.cursor.style.flags.bold);
     try testing.expectEqual(@as(usize, 1), page.styles.count());
 }
@@ -3043,7 +3047,7 @@ test "Screen cursorCopy hyperlink deref" {
     try testing.expect(s2.cursor.hyperlink_id != 0);
 
     // Copy a cursor with no hyperlink, should release our hyperlink.
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
     try testing.expect(s2.cursor.hyperlink_id == 0);
 }
@@ -3109,7 +3113,7 @@ test "Screen cursorCopy hyperlink deref new page" {
 
     // Copy the cursor for the first screen. This should release
     // the hyperlink from page 1 and move the cursor back to page 0.
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
     try testing.expect(s2.cursor.hyperlink_id == 0);
     // The page after the page the cursor is now in should be page 1.
@@ -3139,11 +3143,36 @@ test "Screen cursorCopy hyperlink copy" {
     try testing.expect(s2.cursor.hyperlink_id == 0);
 
     // Copy the cursor with the hyperlink.
-    try s2.cursorCopy(s.cursor);
+    try s2.cursorCopy(s.cursor, .{});
     try testing.expectEqual(@as(usize, 1), page.hyperlink_set.count());
     try testing.expect(s2.cursor.hyperlink_id != 0);
 }
 
+test "Screen cursorCopy hyperlink copy disabled" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try Screen.init(alloc, 10, 10, 0);
+    defer s.deinit();
+
+    // Create a hyperlink for the cursor.
+    try s.startHyperlink("https://example.com/", null);
+    try testing.expectEqual(@as(usize, 1), s.cursor.page_pin.page.data.hyperlink_set.count());
+    try testing.expect(s.cursor.hyperlink_id != 0);
+
+    var s2 = try Screen.init(alloc, 10, 10, 0);
+    defer s2.deinit();
+    const page = &s2.cursor.page_pin.page.data;
+
+    try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id == 0);
+
+    // Copy the cursor with the hyperlink.
+    try s2.cursorCopy(s.cursor, .{ .hyperlink = false });
+    try testing.expectEqual(@as(usize, 0), page.hyperlink_set.count());
+    try testing.expect(s2.cursor.hyperlink_id == 0);
+}
+
 test "Screen style basics" {
     const testing = std.testing;
     const alloc = testing.allocator;
diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index 899718a74..1b32eac31 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -2472,11 +2472,10 @@ pub fn alternateScreen(
     // Mark our terminal as dirty
     self.flags.dirty.clear = true;
 
-    // We always end hyperlink state
-    self.screen.endHyperlink();
-
     // Bring our pen with us
-    self.screen.cursorCopy(old.cursor) catch |err| {
+    self.screen.cursorCopy(old.cursor, .{
+        .hyperlink = false,
+    }) catch |err| {
         log.warn("cursor copy failed entering alt screen err={}", .{err});
     };
 

From df09a37597f72643c0229527a698cf77222670d7 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <m@mitchellh.com>
Date: Wed, 28 Aug 2024 09:56:52 -0700
Subject: [PATCH 11/11] terminal: tests for same page clone with hyperlinks

---
 src/terminal/page.zig | 112 ++++++++++++++++++++++++++++++++++++++++++
 src/terminal/size.zig |   4 +-
 2 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/src/terminal/page.zig b/src/terminal/page.zig
index 5f44d28a4..b1acfc2ba 100644
--- a/src/terminal/page.zig
+++ b/src/terminal/page.zig
@@ -2447,6 +2447,118 @@ test "Page cloneRowFrom partial grapheme in non-copied dest region" {
     try testing.expectEqual(@as(usize, 2), page2.graphemeCount());
 }
 
+test "Page cloneRowFrom partial hyperlink in same page copy" {
+    var page = try Page.init(.{ .cols = 10, .rows = 10 });
+    defer page.deinit();
+
+    // We need to create a hyperlink.
+    const hyperlink_id = try page.hyperlink_set.addContext(
+        page.memory,
+        .{ .id = .{ .implicit = 0 }, .uri = .{} },
+        .{ .page = &page },
+    );
+
+    // Write
+    {
+        const y = 0;
+        for (0..page.size.cols) |x| {
+            const rac = page.getRowAndCell(x, y);
+            rac.cell.* = .{
+                .content_tag = .codepoint,
+                .content = .{ .codepoint = @intCast(x + 1) },
+            };
+        }
+
+        // Hyperlink in a single cell
+        {
+            const rac = page.getRowAndCell(7, y);
+            try page.setHyperlink(rac.row, rac.cell, hyperlink_id);
+        }
+    }
+    try testing.expectEqual(@as(usize, 1), page.hyperlinkCount());
+
+    // Clone into the same page
+    try page.clonePartialRowFrom(
+        &page,
+        page.getRow(1),
+        page.getRow(0),
+        2,
+        8,
+    );
+
+    // Read it again
+    {
+        const y = 1;
+        for (0..page.size.cols) |x| {
+            const expected: u21 = if (x >= 2 and x < 8) @intCast(x + 1) else 0;
+            const rac = page.getRowAndCell(x, y);
+            try testing.expectEqual(expected, rac.cell.content.codepoint);
+        }
+        {
+            const rac = page.getRowAndCell(7, y);
+            try testing.expect(rac.row.hyperlink);
+            try testing.expect(rac.cell.hyperlink);
+        }
+    }
+    try testing.expectEqual(@as(usize, 2), page.hyperlinkCount());
+}
+
+test "Page cloneRowFrom partial hyperlink in same page omit" {
+    var page = try Page.init(.{ .cols = 10, .rows = 10 });
+    defer page.deinit();
+
+    // We need to create a hyperlink.
+    const hyperlink_id = try page.hyperlink_set.addContext(
+        page.memory,
+        .{ .id = .{ .implicit = 0 }, .uri = .{} },
+        .{ .page = &page },
+    );
+
+    // Write
+    {
+        const y = 0;
+        for (0..page.size.cols) |x| {
+            const rac = page.getRowAndCell(x, y);
+            rac.cell.* = .{
+                .content_tag = .codepoint,
+                .content = .{ .codepoint = @intCast(x + 1) },
+            };
+        }
+
+        // Hyperlink in a single cell
+        {
+            const rac = page.getRowAndCell(7, y);
+            try page.setHyperlink(rac.row, rac.cell, hyperlink_id);
+        }
+    }
+    try testing.expectEqual(@as(usize, 1), page.hyperlinkCount());
+
+    // Clone into the same page
+    try page.clonePartialRowFrom(
+        &page,
+        page.getRow(1),
+        page.getRow(0),
+        2,
+        6,
+    );
+
+    // Read it again
+    {
+        const y = 1;
+        for (0..page.size.cols) |x| {
+            const expected: u21 = if (x >= 2 and x < 6) @intCast(x + 1) else 0;
+            const rac = page.getRowAndCell(x, y);
+            try testing.expectEqual(expected, rac.cell.content.codepoint);
+        }
+        {
+            const rac = page.getRowAndCell(7, y);
+            try testing.expect(!rac.row.hyperlink);
+            try testing.expect(!rac.cell.hyperlink);
+        }
+    }
+    try testing.expectEqual(@as(usize, 1), page.hyperlinkCount());
+}
+
 test "Page moveCells text-only" {
     var page = try Page.init(.{
         .cols = 10,
diff --git a/src/terminal/size.zig b/src/terminal/size.zig
index fead2b469..ffa61e6be 100644
--- a/src/terminal/size.zig
+++ b/src/terminal/size.zig
@@ -26,8 +26,8 @@ pub fn Offset(comptime T: type) type {
 
         /// A slice of type T that stores via a base offset and len.
         pub const Slice = struct {
-            offset: Self,
-            len: usize,
+            offset: Self = .{},
+            len: usize = 0,
         };
 
         /// Returns a pointer to the start of the data, properly typed.