From 5197adcc0298537ff73945c32b99869a769daf02 Mon Sep 17 00:00:00 2001 From: JavaZeroo <71128095+JavaZeroo@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:42:01 +0800 Subject: [PATCH 1/3] feat(core): keep the scroll margins per buffer refs #41 FrameState now carries its own DECSTBM margins, so a 1049 switch no longer inherits the other buffer region; resize resets both. --- experiments/core/edit_csi_test.cpp | 20 +++++++++++++-- experiments/core/screen.hpp | 40 ++++++++++++++++++------------ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/experiments/core/edit_csi_test.cpp b/experiments/core/edit_csi_test.cpp index 030b5b84..7f965c96 100644 --- a/experiments/core/edit_csi_test.cpp +++ b/experiments/core/edit_csi_test.cpp @@ -91,14 +91,30 @@ int main() { row_text(margin.screen, 2).empty() && row_text(margin.screen, 3) == "d", "DL ignored the scroll margins"); + // Margins belong to the buffer: a 1049 switch keeps each buffer's region. + { + Parser buffers(4, 10, 10, termforge::ParseMode::recovery); + buffers.feed("\x1b[2;3r"); + require(buffers.screen.region_top() == 1 && buffers.screen.region_bottom() == 2, + "primary region was not set"); + buffers.feed("\x1b[?1049h"); + require(buffers.screen.region_top() == 0 && buffers.screen.region_bottom() == 3, + "alternate buffer inherited the primary region"); + buffers.feed("\x1b[1;2r"); + require(buffers.screen.region_top() == 0 && buffers.screen.region_bottom() == 1, + "alternate region was not set"); + buffers.feed("\x1b[?1049l"); + require(buffers.screen.region_top() == 1 && buffers.screen.region_bottom() == 2, + "primary region was lost across the switch"); + } // A resize drops the margins: scrolling afterwards reaches history again. { Parser resized(4, 10, 10, termforge::ParseMode::recovery); resized.feed("a\r\nb\r\nc\r\nd"); resized.feed("\x1b[2;3r"); - require(resized.screen.scroll_bottom() == 2, "test setup lost the region"); + require(resized.screen.region_bottom() == 2, "test setup lost the region"); resized.screen.resize(4, 12); - require(resized.screen.scroll_bottom() == 3, "resize kept the stale region bottom"); + require(resized.screen.region_bottom() == 3, "resize kept the stale region bottom"); resized.feed("\x1b[4;1H"); resized.feed("\n"); require(resized.screen.scrollback_lines() > 0, diff --git a/experiments/core/screen.hpp b/experiments/core/screen.hpp index fa29fabe..e7196877 100644 --- a/experiments/core/screen.hpp +++ b/experiments/core/screen.hpp @@ -64,8 +64,13 @@ class Screen { // True only while the retained sequence is known to be ordered by id. // False is the safe fallback: lookups scan instead of bisecting. bool ordered_ = true; + // DECSTBM margins belong to the frame: the primary and alternate buffers + // keep their own scrolling region across a 1049 switch. + int scroll_top = 0; + int scroll_bottom = -1; template void swap(State& other) noexcept { std::swap(row, other.row); std::swap(column, other.column); + std::swap(scroll_top, other.scroll_top); std::swap(scroll_bottom, other.scroll_bottom); std::swap(last_row, other.last_row); std::swap(last_column, other.last_column); std::swap(wrap_pending, other.wrap_pending); std::swap(discarded, other.discarded); std::swap(style, other.style); @@ -81,10 +86,10 @@ class Screen { std::deque history; int columns_; uint64_t next_id_ = 1; - int scroll_top_ = 0; - // Negative means "the bottom row", which keeps full-screen scrolling on the - // history path after a resize changes the height. - int scroll_bottom_ = -1; + // DECSTBM margins travel with the buffer through active_state_copy/swap. + // Negative bottom means "the last row", which survives a height change. + int scroll_top = 0; + int scroll_bottom = -1; uint64_t buffer_epoch_ = 0; uint64_t order_revision_ = 0; bool ordered_ = true; @@ -92,7 +97,8 @@ class Screen { ScreenBuffer active_buffer_ = ScreenBuffer::primary; std::optional inactive_; FrameState active_state_copy() const { - return {row, column, last_row, last_column, wrap_pending, discarded, style, lines, history, ordered_}; + return {row, column, last_row, last_column, wrap_pending, discarded, style, lines, history, ordered_, + scroll_top, scroll_bottom}; } void swap_active(FrameState& state) noexcept { state.swap(*this); @@ -382,8 +388,9 @@ class Screen { if (hidden) inactive_->swap(*hidden); columns_ = columns; next_id_ = identity; // The reference terminal drops the scrolling margins on resize, so a - // stale bottom could point past the new height. - scroll_top_ = 0; scroll_bottom_ = -1; + // stale bottom could point past the new height. Both frames reset. + scroll_top = 0; scroll_bottom = -1; + if (inactive_) { inactive_->scroll_top = 0; inactive_->scroll_bottom = -1; } advance_order(ordered_); } void invalidate_attachment() { last_row = last_column = -1; } @@ -391,7 +398,8 @@ class Screen { row = std::clamp(r, 0, height() - 1); column = std::clamp(c, 0, width() - 1); wrap_pending = false; invalidate_attachment(); } - int scroll_bottom() const { return scroll_bottom_ < 0 ? height() - 1 : scroll_bottom_; } + int region_bottom() const { return scroll_bottom < 0 ? height() - 1 : scroll_bottom; } + int region_top() const { return scroll_top; } // DECSTBM: rows inside [top, bottom] scroll together. The list itself keeps // whole rows only, so a partial region scroll shifts entries in place; rows // that leave a full-screen region still reach history. @@ -401,7 +409,7 @@ class Screen { // An invalid region is ignored rather than rejected: the reference // terminal logs and returns, leaving the previous margins in place. if (bottom - top < 1) return; - scroll_top_ = top; scroll_bottom_ = bottom; + scroll_top = top; scroll_bottom = bottom; // DECSTBM homes the cursor to the screen origin, matching the reference // terminal (vendor/alacritty_terminal/src/term/mod.rs: goto(0, 0)). move(0, 0); @@ -414,13 +422,13 @@ class Screen { // A newly allocated bottom row is known before moving its predecessor. previous.next_id = automatic ? (row + 1 < height() ? lines[row + 1].id : next_id_) : 0; wrap_pending = false; invalidate_attachment(); - if (row + 1 < height() && row < scroll_bottom()) { ++row; return; } - if (row < scroll_bottom() || scroll_top_ > 0 || scroll_bottom() < height() - 1) { + if (row + 1 < height() && row < region_bottom()) { ++row; return; } + if (row < region_bottom() || region_top() > 0 || region_bottom() < height() - 1) { // Scrolling inside a region (or inside a movement that stops early) // moves rows up within the margins and blanks the last region row. - for (int r = scroll_top_; r < scroll_bottom(); ++r) lines[r] = std::move(lines[r + 1]); - lines[scroll_bottom()] = make_line(width(), Cell{}, next_id_); - row = scroll_bottom(); + for (int r = region_top(); r < region_bottom(); ++r) lines[r] = std::move(lines[r + 1]); + lines[region_bottom()] = make_line(width(), Cell{}, next_id_); + row = region_bottom(); advance_order(ordered_); return; } @@ -472,7 +480,7 @@ class Screen { } void insert_lines(int count) { if (count < 1) throw InputError("invalid insert line count"); - const int bottom = scroll_bottom(); + const int bottom = region_bottom(); const int insert = (std::min)(count, bottom + 1 - row); if (insert <= 0) { wrap_pending = false; return; } // Rows pushed past the bottom margin leave the screen; their identities @@ -484,7 +492,7 @@ class Screen { } void delete_lines(int count) { if (count < 1) throw InputError("invalid delete line count"); - const int bottom = scroll_bottom(); + const int bottom = region_bottom(); const int remove = (std::min)(count, bottom + 1 - row); if (remove <= 0) { wrap_pending = false; return; } for (int r = row; r <= bottom - remove; ++r) lines[r] = std::move(lines[r + remove]); From 832b167bc7fb62e5b7e70f12d51c0df40d3ce36a Mon Sep 17 00:00:00 2001 From: JavaZeroo <71128095+JavaZeroo@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:44:07 +0800 Subject: [PATCH 2/3] fix(core): only the bottom margin scrolls on a line feed refs #41 Rows above the region now move down normally and rows below it no longer scroll the region, which the 175 byte margin source exposed as a lost row. --- experiments/core/screen.hpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/experiments/core/screen.hpp b/experiments/core/screen.hpp index e7196877..3dbe157c 100644 --- a/experiments/core/screen.hpp +++ b/experiments/core/screen.hpp @@ -422,17 +422,19 @@ class Screen { // A newly allocated bottom row is known before moving its predecessor. previous.next_id = automatic ? (row + 1 < height() ? lines[row + 1].id : next_id_) : 0; wrap_pending = false; invalidate_attachment(); - if (row + 1 < height() && row < region_bottom()) { ++row; return; } - if (row < region_bottom() || region_top() > 0 || region_bottom() < height() - 1) { - // Scrolling inside a region (or inside a movement that stops early) - // moves rows up within the margins and blanks the last region row. - for (int r = region_top(); r < region_bottom(); ++r) lines[r] = std::move(lines[r + 1]); - lines[region_bottom()] = make_line(width(), Cell{}, next_id_); - row = region_bottom(); + const int bottom = region_bottom(); + // Only the bottom margin scrolls; rows above it move down normally and + // rows below it must not disturb the region at all. + if (row < bottom) { ++row; return; } + if (row > bottom) { if (row + 1 < height()) ++row; return; } + if (region_top() > 0 || bottom < height() - 1) { + // Scrolling inside the margins moves rows up and blanks the last one. + for (int r = region_top(); r < bottom; ++r) lines[r] = std::move(lines[r + 1]); + lines[bottom] = make_line(width(), Cell{}, next_id_); + row = bottom; advance_order(ordered_); return; } - if (row < height() - 1) { ++row; return; } const auto limit = active_history_limit(); if (limit) { history.push_back(std::move(lines.front())); From 6e41200e35b5e7e7c905dd5d5e013344e2293310 Mon Sep 17 00:00:00 2001 From: JavaZeroo <71128095+JavaZeroo@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:45:28 +0800 Subject: [PATCH 3/3] test(core): pin the 175 byte margin source result refs #41 The translated margin fixture must land on the required screen and cursor, so a regression back to the ConPTY observed rows fails here. --- experiments/core/edit_csi_test.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/experiments/core/edit_csi_test.cpp b/experiments/core/edit_csi_test.cpp index 7f965c96..ec91dd7d 100644 --- a/experiments/core/edit_csi_test.cpp +++ b/experiments/core/edit_csi_test.cpp @@ -164,6 +164,23 @@ int main() { require(row_text(insert.screen, 0) == "abcdQ" && row_text(insert.screen, 1).empty(), "text after a cancelled ICH wrap did not stay on the row"); } + // The 175-byte margin source from experiments/shell/margin_checks.py, with + // its lone line feeds translated to CRLF the way a console does on output. + // The core must land on the "required" screen, not the ConPTY "observed" one. + { + Parser margin(24, 80, 100, termforge::ParseMode::recovery); + margin.feed("\x1b[2J\x1b[HTOP_FIXED\r\nREGION_A\r\nREGION_B\r\nBOTTOM_FIXED\r\n\x1b[2;3r"); + margin.feed("\x1b[?1049hALT_OUTSIDE\r\n\x1b[4;1HALT_A\r\nALT_B\x1b[4;5r\x1b[5;1H\r\nALT_C\r\nALT_D"); + margin.feed("\x1b[?1049l"); + margin.feed("\x1b[3;1H\r\nREGION_C\r\nREGION_D\x1b[6;1HTF_MARGIN_DONE\r\n"); + const std::string expected[] = {"TOP_FIXED", "REGION_C", "REGION_D", "BOTTOM_FIXED", "", + "TF_MARGIN_DONE"}; + for (int index = 0; index < 6; ++index) + require(row_text(margin.screen, index) == expected[index], + "margin source produced the ConPTY-observed screen"); + require(margin.screen.row == 6 && margin.screen.column == 0, + "margin source cursor mismatch"); + } std::puts("edit csi contracts passed"); return 0; } catch (const std::exception& error) {