From 2d612ab1681bc05b8e3afb8e9f393ba3d74b6f57 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Thu, 25 Jan 2024 09:35:56 -0800
Subject: [PATCH] terminal: preserve multi-point grapheme clusters on
 scrollback deletion

This codepath was not previously tested (an accident). Upon testing
this codepath its clear to see the logic was incorrect. When we have to
remove rows from our scrollback to fit new rows in the circular buffer,
we have to delete graphemes, but we were deleting them from the wrong
row offset.

For the row offset, we previously used the _active_ screen but the
proper offset is the _full_ screen. Tests verify.
---
 src/terminal/Screen.zig   |  3 ++-
 src/terminal/Terminal.zig | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index 90762aa0c..9b5cff2b9 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -2174,7 +2174,7 @@ fn scrollDelta(self: *Screen, delta: isize, viewport_only: bool) Allocator.Error
         if (self.graphemes.count() > 0) {
             var y: usize = 0;
             while (y < rows_to_delete) : (y += 1) {
-                const row = self.getRow(.{ .active = y });
+                const row = self.getRow(.{ .screen = y });
                 if (row.storage[0].header.flags.grapheme) row.clear(.{});
             }
         }
@@ -4306,6 +4306,7 @@ test "Screen: history region with scrollback" {
         try testing.expectEqualStrings(expected, contents);
     }
 }
+
 test "Screen: row copy" {
     const testing = std.testing;
     const alloc = testing.allocator;
diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index cdbe66e01..200894eac 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -7254,3 +7254,28 @@ test "Terminal: printAttributes" {
         try testing.expectEqualStrings("0", buf);
     }
 }
+
+test "Terminal: preserve grapheme cluster on large scrollback" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 3);
+    defer t.deinit(alloc);
+
+    // This is the label emoji + the VS16 variant selector
+    const label = "\u{1F3F7}\u{FE0F}";
+
+    // This bug required a certain behavior around scrollback interacting
+    // with the circular buffer that we use at the time of writing this test.
+    // Mainly, we want to verify that in certain scroll scenarios we preserve
+    // grapheme clusters. This test is admittedly somewhat brittle but we
+    // should keep it around to prevent this regression.
+    for (0..t.screen.max_scrollback * 2) |_| {
+        try t.printString(label ++ "\n");
+    }
+
+    try t.scrollViewport(.{ .delta = -1 });
+    {
+        const str = try t.screen.testString(alloc, .viewport);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("🏷️\n🏷️\n🏷️", str);
+    }
+}