terminal: ECH handles protection attributes properly

src/terminal/Screen.zig

@@ -56,6 +56,7 @@ const Allocator = std.mem.Allocator;
 
 const utf8proc = @import("utf8proc");
 const trace = @import("tracy").trace;
+const ansi = @import("ansi.zig");
 const sgr = @import("sgr.zig");
 const color = @import("color.zig");
 const kitty = @import("kitty.zig");
@@ -932,6 +933,13 @@ saved_charset: CharsetState = .{},
 /// independent to each screen (primary and alternate)
 saved_origin_mode: bool = false,
 
+/// The current or most recent protected mode. Once a protection mode is
+/// set, this will never become "off" again until the screen is reset.
+/// The current state of whether protection attributes should be set is
+/// set on the Cell pen; this is only used to determine the most recent
+/// protection mode since some sequences such as ECH depend on this.
+protected_mode: ansi.ProtectedMode = .off,
+
 /// Initialize a new screen.
 pub fn init(
     alloc: Allocator,

src/terminal/Terminal.zig

@@ -1343,11 +1343,25 @@ pub fn eraseChars(self: *Terminal, count: usize) void {
         break :end end;
     };
 
-    // Shift
+    const pen: Screen.Cell = .{
-    row.fillSlice(.{
         .bg = self.screen.cursor.pen.bg,
         .attrs = .{ .has_bg = self.screen.cursor.pen.attrs.has_bg },
-    }, self.screen.cursor.x, end);
+    };
+
+    // If we never had a protection mode, then we can assume no cells
+    // are protected and go with the fast path. If the last protection
+    // mode was not ISO we also always ignore protection attributes.
+    if (self.screen.protected_mode != .iso) {
+        row.fillSlice(pen, self.screen.cursor.x, end);
+    }
+
+    // We had a protection mode at some point. We must go through each
+    // cell and check its protection attribute.
+    for (self.screen.cursor.x..end) |x| {
+        const cell = row.getCellPtr(x);
+        if (cell.attrs.protected) continue;
+        cell.* = pen;
+    }
 }
 
 /// Move the cursor to the left amount cells. If amount is 0, adjust it to 1.
@@ -1885,15 +1899,20 @@ pub fn setProtectedMode(self: *Terminal, mode: ansi.ProtectedMode) void {
     switch (mode) {
         .off => {
             self.screen.cursor.pen.attrs.protected = false;
+
+            // screen.protected_mode is NEVER reset to ".off" because
+            // logic such as eraseChars depends on knowing what the
+            // _most recent_ mode was.
         },
 
-        // TODO: ISO/DEC have very subtle differences, so we should track that.
         .iso => {
             self.screen.cursor.pen.attrs.protected = true;
+            self.screen.protected_mode = .iso;
         },
 
         .dec => {
             self.screen.cursor.pen.attrs.protected = true;
+            self.screen.protected_mode = .dec;
         },
     }
 }
@@ -1909,6 +1928,7 @@ pub fn fullReset(self: *Terminal, alloc: Allocator) void {
     self.screen.saved_cursor = .{};
     self.screen.selection = null;
     self.screen.kitty_keyboard = .{};
+    self.screen.protected_mode = .off;
     self.scrolling_region = .{
         .top = 0,
         .bottom = self.rows - 1,
@@ -3669,6 +3689,58 @@ test "Terminal: eraseChars wide character" {
     }
 }
 
+test "Terminal: eraseChars protected attributes respected with iso" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    t.setProtectedMode(.iso);
+    for ("ABC") |c| try t.print(c);
+    t.setCursorPos(1, 1);
+    t.eraseChars(2);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("ABC", str);
+    }
+}
+
+test "Terminal: eraseChars protected attributes ignored with dec most recent" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    t.setProtectedMode(.iso);
+    for ("ABC") |c| try t.print(c);
+    t.setProtectedMode(.dec);
+    t.setProtectedMode(.off);
+    t.setCursorPos(1, 1);
+    t.eraseChars(2);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("  C", str);
+    }
+}
+test "Terminal: eraseChars protected attributes ignored with dec set" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    t.setProtectedMode(.dec);
+    for ("ABC") |c| try t.print(c);
+    t.setCursorPos(1, 1);
+    t.eraseChars(2);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("  C", str);
+    }
+}
+
 // https://github.com/mitchellh/ghostty/issues/272
 // This is also tested in depth in screen resize tests but I want to keep
 // this test around to ensure we don't regress at multiple layers.

website/app/vt/ech/page.mdx

@@ -25,16 +25,16 @@ Both erased cells are colored with the current background color according
 to the current SGR state.
 
 If [Select Character Selection Attribute (DECSCA)](#TODO) is enabled
-or was the most recently enabled protection mode,
+or was the most recently enabled protection mode on the currently active screen,
 protected attributes are ignored as if they were never set and the cells
 with them are erased. It does not matter if DECSCA is currently disabled,
 protected attributes are still ignored so long as DECSCA was the
 _most recently enabled_ protection mode.
 
 If DECSCA is not currently enabled and was not the most recently enabled protection
-mode, cells with the protected attribute set are respected and not erased but
+mode on the currently active screen, cells with the protected attribute set are
-still count towards `n`. It does not matter if the protection attribute for a
+respected and not erased but still count towards `n`. It does not matter if the
-cell was originally set from DECSCA.
+protection attribute for a cell was originally set from DECSCA.
 
 ## Validation