From 9ea78f981e3ed8d1aad5510e500cafbba1a5c66a Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 13 Apr 2024 21:31:41 -0700
Subject: [PATCH] terminal: index (LF) that scrolls scroll region preserves SGR

Fixes #1676

The comment in the diff explains. This is a regression that was not unit
tested properly in the old implementation prior to the paged-terminal
merge.
---
 src/terminal/Screen.zig   |  2 +-
 src/terminal/Terminal.zig | 44 ++++++++++++++++++++++++++++++++++++++-
 src/terminal/page.zig     | 10 +++++++++
 src/terminal/stream.zig   |  4 +++-
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig
index b51f10764..98c3daa21 100644
--- a/src/terminal/Screen.zig
+++ b/src/terminal/Screen.zig
@@ -967,7 +967,7 @@ pub fn clearPrompt(self: *Screen) void {
 
 /// Returns the blank cell to use when doing terminal operations that
 /// require preserving the bg color.
-fn blankCell(self: *const Screen) Cell {
+pub fn blankCell(self: *const Screen) Cell {
     if (self.cursor.style_id == style.default_id) return .{};
     return self.cursor.style.bgCell() orelse .{};
 }
diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig
index e29a4003d..cc58093bf 100644
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -1089,7 +1089,15 @@ pub fn index(self: *Terminal) !void {
         } else {
             // Slow path for left and right scrolling region margins.
             if (self.scrolling_region.left != 0 or
-                self.scrolling_region.right != self.cols - 1)
+                self.scrolling_region.right != self.cols - 1 or
+
+                // PERF(mitchellh): If we have an SGR background set then
+                // we need to preserve that background in our erased rows.
+                // scrollUp does that but eraseRowBounded below does not.
+                // However, scrollUp is WAY slower. We should optimize this
+                // case to work in the eraseRowBounded codepath and remove
+                // this check.
+                !self.screen.blankCell().isZero())
             {
                 self.scrollUp(1);
                 return;
@@ -5317,6 +5325,40 @@ test "Terminal: index inside scroll region" {
     }
 }
 
+test "Terminal: index bottom of scroll region with background SGR" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, .{ .rows = 5, .cols = 5 });
+    defer t.deinit(alloc);
+
+    t.setTopAndBottomMargin(1, 3);
+    t.setCursorPos(4, 1);
+    try t.print('B');
+    t.setCursorPos(3, 1);
+    try t.print('A');
+    try t.setAttribute(.{ .direct_color_bg = .{
+        .r = 0xFF,
+        .g = 0,
+        .b = 0,
+    } });
+    try t.index();
+
+    {
+        const str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("\nA\n\nB", str);
+    }
+
+    for (0..t.cols) |x| {
+        const list_cell = t.screen.pages.getCell(.{ .active = .{ .x = x, .y = 2 } }).?;
+        try testing.expect(list_cell.cell.content_tag == .bg_color_rgb);
+        try testing.expectEqual(Cell.RGB{
+            .r = 0xFF,
+            .g = 0,
+            .b = 0,
+        }, list_cell.cell.content.color_rgb);
+    }
+}
+
 test "Terminal: index bottom of primary screen with scroll region" {
     const alloc = testing.allocator;
     var t = try init(alloc, .{ .rows = 5, .cols = 5 });
diff --git a/src/terminal/page.zig b/src/terminal/page.zig
index e70695213..4dfdc29e4 100644
--- a/src/terminal/page.zig
+++ b/src/terminal/page.zig
@@ -1138,6 +1138,10 @@ pub const Cell = packed struct(u64) {
         };
     }
 
+    pub fn isZero(self: Cell) bool {
+        return @as(u64, @bitCast(self)) == 0;
+    }
+
     pub fn hasText(self: Cell) bool {
         return switch (self.content_tag) {
             .codepoint,
@@ -1231,6 +1235,12 @@ pub const Cell = packed struct(u64) {
 //     //const pages = total_size / std.mem.page_size;
 // }
 
+test "Cell is zero by default" {
+    const cell: Cell = .{};
+    const cell_int: u64 = @bitCast(cell);
+    try std.testing.expectEqual(@as(u64, 0), cell_int);
+}
+
 test "Page capacity adjust cols down" {
     const original = std_capacity;
     const original_size = Page.layout(original).total_size;
diff --git a/src/terminal/stream.zig b/src/terminal/stream.zig
index 4f4f36ad5..f50a8255e 100644
--- a/src/terminal/stream.zig
+++ b/src/terminal/stream.zig
@@ -316,7 +316,9 @@ pub fn Stream(comptime Handler: type) type {
         }
 
         pub fn execute(self: *Self, c: u8) !void {
-            switch (@as(ansi.C0, @enumFromInt(c))) {
+            const c0: ansi.C0 = @enumFromInt(c);
+            // log.info("execute: {}", .{c0});
+            switch (c0) {
                 // We ignore SOH/STX: https://github.com/microsoft/terminal/issues/10786
                 .NUL, .SOH, .STX => {},