diff --git a/.github/workflows/next-xcframework.yml b/.github/workflows/next-xcframework.yml index 17ef97e9d..6aba9ec51 100644 --- a/.github/workflows/next-xcframework.yml +++ b/.github/workflows/next-xcframework.yml @@ -138,6 +138,9 @@ jobs: -Dtest-filter="Page vt preserved trailing blank rows" \ -Dtest-filter="Page VT preserves a fully styled blank row" \ -Dtest-filter="Page VT unstyled blank cells" \ + -Dtest-filter="Page plain wrap continuation" \ + -Dtest-filter="breaks the wrap" \ + -Dtest-filter="print wrap at the scroll region bottom" \ -Dtest-filter="render: OSC dynamic color resets" \ -Dtest-filter="get cursor visual" \ -Dtest-filter="cursor activity" \ diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 8222e4397..aaa28f725 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -1185,6 +1185,10 @@ pub fn cursorScrollAbove(self: *Screen) !void { } } + // The rows below the cursor did not move but the cursor row is a new + // blank row: the row below no longer continues it. + self.breakWrapAbove(self.cursor.page_pin.down(1).?); + if (self.cursor.style_id != style.default_id) { // The newly created line needs to be styled according to // the bg color if it is set. @@ -1297,7 +1301,9 @@ fn cursorScrollAboveRotate( /// other scroll operations such as deleteLines). /// /// The cursor stays at the same screen position (the new blank row). -/// Content outside of the region is unmodified. +/// Content outside of the region is unmodified, but a soft wrap that +/// joined a row outside the region to one inside it is broken (see +/// breakWrapAbove): those rows no longer hold one line. /// /// This is a very hot path for scroll region usage (e.g. a program /// on the alt screen using DECSTBM and scrolling via LF/IND) so this @@ -1313,7 +1319,11 @@ pub fn cursorScrollRegionUp(self: *Screen, limit: usize) !void { // If the region crosses a page boundary we take a slower path. This // is rare: it requires the active area to span multiple pages with // the split point inside the scroll region. - if (pin.y < limit) return try self.cursorScrollRegionUpSlow(limit); + if (pin.y < limit) { + try self.cursorScrollRegionUpSlow(limit); + self.breakWrapAtScrolledRegionEdges(limit); + return; + } // Fast path: the entire region is in a single page. We can clear // the top row and rotate it down to the cursor row, updating any @@ -1399,6 +1409,20 @@ pub fn cursorScrollRegionUp(self: *Screen, limit: usize) !void { const cursor_row: *Row = &rows[limit]; self.cursor.page_row = cursor_row; self.cursor.page_cell = &page.getCells(cursor_row)[self.cursor.x]; + + self.breakWrapAtScrolledRegionEdges(limit); +} + +/// After cursorScrollRegionUp: the region from `limit` rows above the +/// cursor to the cursor row moved, the rows above and below it did not, +/// so no soft wrap may join a row outside the region to one inside it. +fn breakWrapAtScrolledRegionEdges(self: *Screen, limit: usize) void { + if (self.cursor.y > limit) { + self.breakWrapAbove(self.cursor.page_pin.up(limit).?); + } + if (self.cursor.y + 1 < self.pages.rows) { + self.breakWrapAbove(self.cursor.page_pin.down(1).?); + } } /// Slow path for cursorScrollRegionUp: the scroll region spans @@ -1651,6 +1675,32 @@ pub fn cursorResetWrap(self: *Screen) void { } } +/// Break the soft wrap between the row at `pin` and the row above it, +/// if either row's flag says they are joined. A scroll of a scroll region +/// moves the rows on one side of its edge and not the other, so the two +/// rows no longer hold one line; a stale wrap or wrap_continuation would +/// make reflow and the unwrapping formatter join them. The row above may +/// be a scrollback row. +pub fn breakWrapAbove(self: *Screen, pin: Pin) void { + const row = pin.rowAndCell().row; + const prev = pin.up(1) orelse { + row.wrap_continuation = false; + return; + }; + const prev_row = prev.rowAndCell().row; + if (!row.wrap_continuation and !prev_row.wrap) return; + row.wrap_continuation = false; + prev_row.wrap = false; + + // A spacer head only exists to carry a wide character over the wrap. + const cells = prev.cells(.all); + const last = prev.node.cols() - 1; + if (cells[last].wide == .spacer_head) { + self.clearCells(prev.node.page(), prev_row, cells[last..][0..1]); + prev.markDirty(); + } +} + /// Make the cursor row start a new logical line by resetting the soft wrap /// from the row above into it. Reflow then never joins the two rows. pub fn cursorBreakWrapIntoRow(self: *Screen) void { diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index d25ca1cf4..fff7edc67 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1810,7 +1810,7 @@ fn printWrap(self: *Terminal) !void { const old_semantic_clear = cursor.semantic_content_clear_eol; // Move to the next line - try self.index(); + try self.indexOpts(true); self.screens.active.cursorHorizontalAbsolute(self.scrolling_region.left); // Our pointer should never move @@ -2384,6 +2384,12 @@ pub fn tabReset(self: *Terminal) void { /// /// This unsets the pending wrap state without wrapping. pub fn index(self: *Terminal) !void { + return self.indexOpts(false); +} + +/// index(), and for `soft_wrap` (printWrap) the cursor row keeps the wrap +/// printWrap set on it: the row it wraps into is the one the scroll makes. +fn indexOpts(self: *Terminal, comptime soft_wrap: bool) !void { const screen: *Screen = self.screens.active; // Unset pending wrap state @@ -2443,6 +2449,12 @@ pub fn index(self: *Terminal) !void { screen.kitty_images.dirty = true; } + // A linefeed at a bottom margin moves the cursor row up and away + // from the row below the region: a soft wrap from it into that row + // no longer joins one line. printWrap's own wrap goes into the new + // row the scroll makes, so it stays. + if (!soft_wrap) self.breakWrapAtBottomMargin(); + // If our scrolling region is at the top, we create scrollback, // but only if our screen retains scrollback. If our screen // doesn't retain scrollback (e.g. the alternate screen) then @@ -2508,6 +2520,16 @@ pub fn index(self: *Terminal) !void { } } +/// Before a scroll that moves the cursor row (the bottom of a full-width +/// region with rows below it) up: break the cursor row's soft wrap into the +/// row below the region, which does not move. +fn breakWrapAtBottomMargin(self: *Terminal) void { + if (self.scrolling_region.bottom + 1 >= self.rows or + self.scrolling_region.left != 0 or + self.scrolling_region.right != self.cols - 1) return; + self.screens.active.cursorResetWrap(); +} + /// The operation when we have Kitty image placements during index. /// Split out of index() so its hot paths don't carry the adjustment /// state in their stack frame, which measurably slows the scroll @@ -2795,6 +2817,7 @@ pub fn scrollUp(self: *Terminal, count: usize) !void { // Move our cursor to the bottom of the scroll region so we can // use the cursorScrollAbove function to create scrollback self.screens.active.cursorAbsolute(0, self.scrolling_region.bottom); + self.breakWrapAtBottomMargin(); for (0..adjusted_count) |_| try self.screens.active.cursorScrollAbove(); return; } @@ -2995,6 +3018,18 @@ fn invalidateFullWidthRowRange( } } +/// After insertLines or deleteLines with a full-width scroll region: the +/// rows from `top` to the region bottom moved, the rows above and below +/// them did not, so no soft wrap may join a row outside to one inside. +fn breakWrapAtLineEdges(self: *Terminal, top: size.CellCountInt) void { + const screen: *Screen = self.screens.active; + screen.breakWrapAbove(screen.pages.pin(.{ .active = .{ .y = top } }).?); + const bottom = self.scrolling_region.bottom; + if (bottom + 1 < self.rows) screen.breakWrapAbove( + screen.pages.pin(.{ .active = .{ .y = bottom + 1 } }).?, + ); +} + // TODO(qwerasd): `insertLines` and `deleteLines` are 99% identical, // the majority of their logic can (and should) be abstracted in to // a single shared helper function, probably on `Screen` not here. @@ -3174,6 +3209,8 @@ pub fn insertLines(self: *Terminal, count: usize) void { // Move our pin up to the next row. if (cur_p.up(1)) |p| cur_p.* = p; } + + if (!left_right) self.breakWrapAtLineEdges(start_y); } /// Removes amount lines from the current cursor row down. The remaining lines @@ -3342,6 +3379,8 @@ pub fn deleteLines(self: *Terminal, count: usize) void { // Move our pin down to the next row. if (cur_p.down(1)) |p| cur_p.* = p; } + + if (!left_right) self.breakWrapAtLineEdges(start_y); } /// Inserts spaces at current cursor position moving existing cell contents @@ -16908,6 +16947,171 @@ test "Terminal: scroll region linefeed recycled row has default metadata" { } } +// A scroll of a scroll region moves the rows inside it and keeps the rows +// outside it. A soft wrap between a row outside and a row inside then joins +// two rows that no longer hold one line: the outside row's wrap flag (above +// the region) or wrap_continuation flag (below it) is stale. Reflow and the +// unwrapping formatter read those flags, so each edge must be broken. +fn testRowFlags(t: *Terminal, y: u32) struct { wrap: bool, cont: bool } { + const row = t.screens.active.pages.getCell(.{ .active = .{ .y = y } }).?.row; + return .{ .wrap = row.wrap, .cont = row.wrap_continuation }; +} + +test "Terminal: index at the scroll region bottom breaks the wrap into the row below" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 2 soft-wraps into row 3. + t.setCursorPos(3, 1); + try t.printString("abcdefg"); + try testing.expect(testRowFlags(&t, 3).cont); + + // DECSTBM rows 2-3 (rows 1-2 here), then a linefeed at its bottom: + // row 2 becomes a blank row and row 3, outside the region, stays. + t.setTopAndBottomMargin(2, 3); + t.setCursorPos(3, 1); + try t.linefeed(); + + // Row 2's content moved to row 1; row 3 did not move. + try testing.expect(!testRowFlags(&t, 1).wrap); + try testing.expect(!testRowFlags(&t, 2).wrap); + try testing.expect(!testRowFlags(&t, 2).cont); + try testing.expect(!testRowFlags(&t, 3).cont); +} + +test "Terminal: print wrap at the scroll region bottom keeps its own wrap and breaks the one below" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 2 soft-wraps into row 3. + t.setCursorPos(3, 1); + try t.printString("abcdefg"); + try testing.expect(testRowFlags(&t, 3).cont); + + // DECSTBM rows 2-3 (rows 1-2 here). A wide character at row 2's last + // column does not fit, so row 2 gets a spacer head and soft-wraps; the + // wrap scrolls the region and row 2 moves to row 1. + t.setTopAndBottomMargin(2, 3); + t.setCursorPos(3, 5); + try t.print(0x6F22); // 漢 + + // Row 1 (the moved row) wraps into the new row 2, with its spacer head. + try testing.expect(testRowFlags(&t, 1).wrap); + try testing.expect(testRowFlags(&t, 2).cont); + { + const cell = t.screens.active.pages.getCell(.{ .active = .{ .x = 4, .y = 1 } }).?.cell; + try testing.expectEqual(Cell.Wide.spacer_head, cell.wide); + } + // Row 3 does not move and no longer continues anything. + try testing.expect(!testRowFlags(&t, 3).cont); +} + +test "Terminal: index in a scroll region breaks the wrap across the region top" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 0 soft-wraps into row 1. + t.setCursorPos(1, 1); + try t.printString("abcdefg"); + try testing.expect(testRowFlags(&t, 0).wrap); + + // DECSTBM rows 2-4 (rows 1-3 here): row 1 is discarded and row 0, + // outside the region, stays. + t.setTopAndBottomMargin(2, 4); + t.setCursorPos(4, 1); + try t.linefeed(); + + try testing.expect(!testRowFlags(&t, 0).wrap); + try testing.expect(!testRowFlags(&t, 1).cont); +} + +test "Terminal: index at a bottom margin with scrollback breaks the wrap into the row below" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 1 soft-wraps into row 2. + t.setCursorPos(2, 1); + try t.printString("abcdefg"); + try testing.expect(testRowFlags(&t, 2).cont); + + // DECSTBM rows 1-2 (rows 0-1 here) scrolls into the scrollback; row 1 + // becomes a blank row and row 2, outside the region, stays. + t.setTopAndBottomMargin(1, 2); + t.setCursorPos(2, 1); + try t.linefeed(); + + // Row 1's content moved to row 0; row 2 did not move. + try testing.expect(!testRowFlags(&t, 0).wrap); + try testing.expect(!testRowFlags(&t, 1).wrap); + try testing.expect(!testRowFlags(&t, 2).cont); +} + +test "Terminal: scrollUp at a bottom margin breaks the wrap into the row below" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 2 soft-wraps into row 3. + t.setCursorPos(3, 1); + try t.printString("abcdefg"); + + // DECSTBM rows 1-3 (rows 0-2 here), SU 1: row 2's content moves to + // row 1 and row 3 does not move. + t.setTopAndBottomMargin(1, 3); + try t.scrollUp(1); + + try testing.expect(!testRowFlags(&t, 1).wrap); + try testing.expect(!testRowFlags(&t, 3).cont); +} + +test "Terminal: deleteLines breaks the wrap across both region edges" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 0 soft-wraps into row 1 and row 3 soft-wraps into row 4. + t.setCursorPos(1, 1); + try t.printString("abcdefg"); + t.setCursorPos(4, 1); + try t.printString("hijklmn"); + + // DECSTBM rows 2-4 (rows 1-3 here), delete the region's top line. + t.setTopAndBottomMargin(2, 4); + t.setCursorPos(2, 1); + t.deleteLines(1); + + try testing.expect(!testRowFlags(&t, 0).wrap); + try testing.expect(!testRowFlags(&t, 1).cont); + try testing.expect(!testRowFlags(&t, 3).wrap); + try testing.expect(!testRowFlags(&t, 4).cont); +} + +test "Terminal: insertLines breaks the wrap across both region edges" { + const alloc = testing.allocator; + var t = try init(testing.io, alloc, .{ .cols = 5, .rows = 6 }); + defer t.deinit(alloc); + + // Row 0 soft-wraps into row 1 and row 3 soft-wraps into row 4. + t.setCursorPos(1, 1); + try t.printString("abcdefg"); + t.setCursorPos(4, 1); + try t.printString("hijklmn"); + + // DECSTBM rows 2-4 (rows 1-3 here), insert a line at the region's top. + t.setTopAndBottomMargin(2, 4); + t.setCursorPos(2, 1); + t.insertLines(1); + + try testing.expect(!testRowFlags(&t, 0).wrap); + try testing.expect(!testRowFlags(&t, 1).cont); + try testing.expect(!testRowFlags(&t, 3).wrap); + try testing.expect(!testRowFlags(&t, 4).cont); +} + test "Terminal: alt screen scroll up recycled row has default metadata" { const alloc = testing.allocator; const io_impl = testing.io; diff --git a/src/terminal/formatter.zig b/src/terminal/formatter.zig index a71db37f7..346717218 100644 --- a/src/terminal/formatter.zig +++ b/src/terminal/formatter.zig @@ -1368,6 +1368,8 @@ pub const PageFormatter = struct { continue; } + // Whether a line break goes out before this row (a prior row did not wrap, or blank rows came before it). + const broke_before = blank_rows > 0; if (blank_rows > 0) { // Reset style before emitting newlines to prevent background // colors from bleeding into the next line's leading cells. @@ -1432,8 +1434,13 @@ pub const PageFormatter = struct { if (!row.wrap or !self.opts.unwrap) blank_rows += 1; // If the row doesn't continue a wrap then we need to reset - // our blank cell count. - if (!row.wrap_continuation or !self.opts.unwrap) blank_cells = 0; + // our blank cell count. Also when a line break went out before + // it: blank cells carry only across a wrap we joined. A row can + // be marked as continuing a wrap while the row before it no + // longer wraps (its wrap flag cleared, the continuation flag + // kept); carrying the prior row's trailing blanks then printed + // them after the break, shifting the row right. + if (!row.wrap_continuation or !self.opts.unwrap or broke_before) blank_cells = 0; // Go through each cell and print it var cell_i: usize = 0; @@ -3123,6 +3130,40 @@ test "Page plain with prior trailing state cells with wrap continuation" { ); } +test "Page plain wrap continuation after a row that does not wrap keeps its column" { + // A row marked as continuing a wrap while the row before it does not + // wrap (cmux VT fuzz finding E2): the line break goes out, and the prior + // row's trailing blank cells must not be printed after it. + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var builder: std.Io.Writer.Allocating = .init(alloc); + defer builder.deinit(); + + var t = try Terminal.init(io, alloc, .{ + .cols = 80, + .rows = 24, + }); + defer t.deinit(alloc); + + var s = t.vtStream(); + defer s.deinit(); + + s.nextSlice("abc\r\nu"); + + const pages = &t.screens.active.pages; + const page = pages.pages.last.?.page(); + + // Row 0 does not wrap; row 1 keeps a stale continuation flag. + try testing.expect(!page.getRow(0).wrap); + page.getRow(1).wrap_continuation = true; + + var formatter: PageFormatter = .init(page, .{ .emit = .plain, .unwrap = true }); + try formatter.format(&builder.writer); + try testing.expectEqualStrings("abc\nu", builder.writer.buffered()); +} + test "Page plain soft-wrapped without unwrap" { const testing = std.testing; const alloc = testing.allocator;