terminal: restoreCursor is now longer fallible

We need to have sane behavior in error handling because the running
program that sends the restore cursor command has no way to realize it
failed. So if our style fails to add (our only fail case) then we revert
to no style.

https://ampcode.com/threads/T-019bd7dc-cf0b-7439-ad2f-218b3406277a
This commit is contained in:
Mitchell Hashimoto
2026-01-19 12:04:34 -08:00
parent c412b30cb5
commit a8b31ceb84
4 changed files with 103 additions and 36 deletions

View File

@@ -9350,17 +9350,21 @@ test "Screen setAttribute increases capacity when style map is full" {
const page = &s.cursor.page_pin.node.data;
const original_styles_capacity = page.capacity.styles;
// Fill the style map to capacity
// Fill the style map to capacity using the StyleSet's layout capacity
// which accounts for the load factor
{
page.pauseIntegrityChecks(true);
defer page.pauseIntegrityChecks(false);
defer page.assertIntegrity();
var n: u24 = 1;
while (page.styles.add(
page.memory,
.{ .bg_color = .{ .rgb = @bitCast(n) } },
)) |_| n += 1 else |_| {}
const max_items = page.styles.layout.cap;
var n: usize = 1;
while (n < max_items) : (n += 1) {
_ = page.styles.add(
page.memory,
.{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } },
) catch break;
}
}
// Now try to set a new unique attribute that would require a new style slot
@@ -9411,17 +9415,21 @@ test "Screen setAttribute splits page on OutOfSpace at max styles" {
var page = &s.cursor.page_pin.node.data;
try testing.expectEqual(max_styles, page.capacity.styles);
// Fill the style map to capacity
// Fill the style map to capacity using the StyleSet's layout capacity
// which accounts for the load factor
{
page.pauseIntegrityChecks(true);
defer page.pauseIntegrityChecks(false);
defer page.assertIntegrity();
var n: u24 = 1;
while (page.styles.add(
page.memory,
.{ .bg_color = .{ .rgb = @bitCast(n) } },
)) |_| n += 1 else |_| {}
const max_items = page.styles.layout.cap;
var n: usize = 1;
while (n < max_items) : (n += 1) {
_ = page.styles.add(
page.memory,
.{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } },
) catch break;
}
}
// Track the node before setAttribute

View File

@@ -996,7 +996,7 @@ pub fn saveCursor(self: *Terminal) void {
///
/// The primary and alternate screen have distinct save state.
/// If no save was done before values are reset to their initial values.
pub fn restoreCursor(self: *Terminal) !void {
pub fn restoreCursor(self: *Terminal) void {
const saved: Screen.SavedCursor = self.screens.active.saved_cursor orelse .{
.x = 0,
.y = 0,
@@ -1008,10 +1008,17 @@ pub fn restoreCursor(self: *Terminal) !void {
};
// Set the style first because it can fail
const old_style = self.screens.active.cursor.style;
self.screens.active.cursor.style = saved.style;
errdefer self.screens.active.cursor.style = old_style;
try self.screens.active.manualStyleUpdate();
self.screens.active.manualStyleUpdate() catch |err| {
// Regardless of the error here, we revert back to an unstyled
// cursor. It is more important that the restore succeeds in
// other attributes because terminals have no way to communicate
// failure back.
log.warn("restoreCursor error updating style err={}", .{err});
const screen: *Screen = self.screens.active;
screen.cursor.style = .{};
screen.cursor.style_id = style.default_id;
};
self.screens.active.charset = saved.charset;
self.modes.set(.origin, saved.origin);
@@ -2747,12 +2754,7 @@ pub fn switchScreenMode(
}
} else {
assert(self.screens.active_key == .primary);
self.restoreCursor() catch |err| {
log.warn(
"restore cursor on switch screen failed to={} err={}",
.{ to, err },
);
};
self.restoreCursor();
},
}
}
@@ -4807,7 +4809,7 @@ test "Terminal: horizontal tab back with cursor before left margin" {
t.saveCursor();
t.modes.set(.enable_left_and_right_margin, true);
t.setLeftAndRightMargin(5, 0);
try t.restoreCursor();
t.restoreCursor();
try t.horizontalTabBack();
try t.print('X');
@@ -9873,7 +9875,7 @@ test "Terminal: saveCursor" {
t.screens.active.charset.gr = .G0;
try t.setAttribute(.{ .unset = {} });
t.modes.set(.origin, false);
try t.restoreCursor();
t.restoreCursor();
try testing.expect(t.screens.active.cursor.style.flags.bold);
try testing.expect(t.screens.active.charset.gr == .G3);
try testing.expect(t.modes.get(.origin));
@@ -9889,7 +9891,7 @@ test "Terminal: saveCursor position" {
t.saveCursor();
t.setCursorPos(1, 1);
try t.print('B');
try t.restoreCursor();
t.restoreCursor();
try t.print('X');
{
@@ -9909,7 +9911,7 @@ test "Terminal: saveCursor pending wrap state" {
t.saveCursor();
t.setCursorPos(1, 1);
try t.print('B');
try t.restoreCursor();
t.restoreCursor();
try t.print('X');
{
@@ -9929,7 +9931,7 @@ test "Terminal: saveCursor origin mode" {
t.modes.set(.enable_left_and_right_margin, true);
t.setLeftAndRightMargin(3, 5);
t.setTopAndBottomMargin(2, 4);
try t.restoreCursor();
t.restoreCursor();
try t.print('X');
{
@@ -9947,7 +9949,7 @@ test "Terminal: saveCursor resize" {
t.setCursorPos(1, 10);
t.saveCursor();
try t.resize(alloc, 5, 5);
try t.restoreCursor();
t.restoreCursor();
try t.print('X');
{
@@ -9968,7 +9970,7 @@ test "Terminal: saveCursor protected pen" {
t.saveCursor();
t.setProtectedMode(.off);
try testing.expect(!t.screens.active.cursor.protected);
try t.restoreCursor();
t.restoreCursor();
try testing.expect(t.screens.active.cursor.protected);
}
@@ -9981,10 +9983,67 @@ test "Terminal: saveCursor doesn't modify hyperlink state" {
const id = t.screens.active.cursor.hyperlink_id;
t.saveCursor();
try testing.expectEqual(id, t.screens.active.cursor.hyperlink_id);
try t.restoreCursor();
t.restoreCursor();
try testing.expectEqual(id, t.screens.active.cursor.hyperlink_id);
}
test "Terminal: restoreCursor uses default style on OutOfSpace" {
// Tests that restoreCursor falls back to default style when
// manualStyleUpdate fails with OutOfSpace (can't split a 1-row page
// and styles are at max capacity).
const alloc = testing.allocator;
// Use a single row so the page can't be split
var t = try init(alloc, .{ .cols = 10, .rows = 1 });
defer t.deinit(alloc);
// Set a style and save the cursor
try t.setAttribute(.{ .bold = {} });
t.saveCursor();
// Clear the style
try t.setAttribute(.{ .unset = {} });
try testing.expect(!t.screens.active.cursor.style.flags.bold);
// Fill the style map to max capacity
const max_styles = std.math.maxInt(size.CellCountInt);
while (t.screens.active.cursor.page_pin.node.data.capacity.styles < max_styles) {
_ = t.screens.active.increaseCapacity(
t.screens.active.cursor.page_pin.node,
.styles,
) catch break;
}
const page = &t.screens.active.cursor.page_pin.node.data;
try testing.expectEqual(max_styles, page.capacity.styles);
// Fill all style slots using the StyleSet's layout capacity which accounts
// for the load factor. The capacity in the layout is the actual max number
// of items that can be stored.
{
page.pauseIntegrityChecks(true);
defer page.pauseIntegrityChecks(false);
defer page.assertIntegrity();
const max_items = page.styles.layout.cap;
var n: usize = 1;
while (n < max_items) : (n += 1) {
_ = page.styles.add(
page.memory,
.{ .bg_color = .{ .rgb = @bitCast(@as(u24, @intCast(n))) } },
) catch break;
}
}
// Restore cursor - should fall back to default style since page
// can't be split (1 row) and styles are at max capacity
t.restoreCursor();
// The style should be reset to default because OutOfSpace occurred
try testing.expect(!t.screens.active.cursor.style.flags.bold);
try testing.expectEqual(style.default_id, t.screens.active.cursor.style_id);
}
test "Terminal: setProtectedMode" {
const alloc = testing.allocator;
var t = try init(alloc, .{ .cols = 3, .rows = 3 });
@@ -11376,7 +11435,7 @@ test "Terminal: resize with reflow and saved cursor" {
t.saveCursor();
try t.resize(alloc, 5, 3);
try t.restoreCursor();
t.restoreCursor();
{
const str = try t.plainString(testing.allocator);
@@ -11417,7 +11476,7 @@ test "Terminal: resize with reflow and saved cursor pending wrap" {
t.saveCursor();
try t.resize(alloc, 5, 3);
try t.restoreCursor();
t.restoreCursor();
{
const str = try t.plainString(testing.allocator);

View File

@@ -125,7 +125,7 @@ pub const Handler = struct {
}
},
.save_cursor => self.terminal.saveCursor(),
.restore_cursor => try self.terminal.restoreCursor(),
.restore_cursor => self.terminal.restoreCursor(),
.invoke_charset => self.terminal.invokeCharset(value.bank, value.charset, value.locking),
.configure_charset => self.terminal.configureCharset(value.slot, value.charset),
.set_attribute => switch (value) {
@@ -240,7 +240,7 @@ pub const Handler = struct {
.save_cursor => if (enabled) {
self.terminal.saveCursor();
} else {
try self.terminal.restoreCursor();
self.terminal.restoreCursor();
},
.enable_mode_3 => {},

View File

@@ -721,7 +721,7 @@ pub const StreamHandler = struct {
if (enabled) {
self.terminal.saveCursor();
} else {
try self.terminal.restoreCursor();
self.terminal.restoreCursor();
}
},
@@ -933,7 +933,7 @@ pub const StreamHandler = struct {
}
pub inline fn restoreCursor(self: *StreamHandler) !void {
try self.terminal.restoreCursor();
self.terminal.restoreCursor();
}
pub fn enquiry(self: *StreamHandler) !void {