From 1ebdd3ed61e8c173e30673b8d2b4d44c90bed858 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 11 Jun 2026 18:45:32 +0200 Subject: [PATCH] Move displaced line of top-anchored scroll region into scroll-back Full-screen CLIs such as Codex in --no-alt-screen mode render through DECSTBM scroll regions anchored at the top of the screen and scroll by emitting newlines at the region bottom. VT100EmulatorBackend scrolled such a region in place and discarded the displaced top line, so no scroll-back history was created and the output could not be scrolled backwards. Like xterm and VTE, a newline at the bottom of a scroll region whose top margin is the top of the screen now moves the displaced top line into the scroll-back history, while the lines below the region keep their position on the screen. Scroll regions with a top margin below the top of the screen still scroll in place and discard the top line. The characterization test documenting the old behavior is flipped to assert that the history is created; the remaining scrolling guards are unchanged. https://github.com/eclipse-platform/eclipse.platform/issues/2680 --- .../emulator/VT100EmulatorBackend.java | 26 +++++++++++++++- .../emulator/VT100EmulatorBackendTest.java | 31 ++++++++----------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackend.java b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackend.java index 4dfc1f1b560..b541a4f0247 100644 --- a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackend.java +++ b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackend.java @@ -354,7 +354,11 @@ private int doLineWrap() { */ private void doNewline() { if (fCursorLine == fScrollRegion.getBottomLine()) { - scrollUp(1); + if (fScrollRegion.getTopLine() == 0) { + scrollTopAnchoredRegionUpIntoHistory(); + } else { + scrollUp(1); + } } else if (fCursorLine + 1 >= fLines) { int h = fTerminal.getHeight(); fTerminal.addLine(); @@ -366,6 +370,26 @@ private void doNewline() { } } + /** + * Scrolls a scroll region whose top margin is the top of the screen: the displaced top line + * is moved into the scroll-back history (like xterm/VTE) instead of being discarded. + * MUST be called from a synchronized block! + */ + private void scrollTopAnchoredRegionUpIntoHistory() { + // adding a line moves the whole screen window up by one line: the former top line of the + // screen (and of the region) becomes the newest scroll-back line and the new bottom line of + // the screen is blank + fTerminal.addLine(); + int regionBottom = fScrollRegion.getBottomLine(); + if (regionBottom < fLines - 1) { + // lines below the region must not move: shift them back down by one line, which also + // vacates the new bottom line of the region + fTerminal.scroll(toAbsoluteLine(regionBottom), fLines - regionBottom, 1); + } + // the absolute line of the cursor may have changed + setCursorLine(fCursorLine); + } + @Override public void processNewline() { synchronized (fTerminal) { diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackendTest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackendTest.java index aecbc640616..842e3d7aa4a 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackendTest.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/emulator/VT100EmulatorBackendTest.java @@ -979,12 +979,7 @@ public void testEraseCharacters() { } // --------------------------------------------------------------------------------------------- - // Characterization tests for the scrolling behavior. - // - // These lock the current behavior before any change to how scrolling interacts with the - // scroll-back history. The regression guards below MUST keep passing through such a change; the - // "currentlyDiscards" test documents the behavior that a future fix for top-anchored scroll - // regions (eclipse.platform issue 2680) is expected to change. + // Tests for how scrolling interacts with the scroll-back history. // --------------------------------------------------------------------------------------------- /** @@ -1061,14 +1056,11 @@ public void testReverseLineFeedAtTopOfRegionScrollsDown() { } /** - * Characterization of the current behavior for a scroll region anchored at the top of - * the screen: a newline at the region bottom scrolls in place and discards the top line, creating - * no history. This is the behavior the planned fix for eclipse.platform issue 2680 will change so - * that the discarded line is added to the scroll-back instead. Until then this documents the - * status quo. + * A newline at the bottom of a top-anchored scroll region moves the displaced top line into + * the scroll-back history instead of discarding it. */ @Test - public void testNewlineInTopAnchoredScrollRegionCurrentlyDiscardsTopLine() { + public void testNewlineInTopAnchoredScrollRegionMovesTopLineToHistory() { ITerminalTextData term = makeITerminalTextData(); IVT100EmulatorBackend vt100 = makeBakend(term); term.setMaxHeight(10); @@ -1079,11 +1071,14 @@ public void testNewlineInTopAnchoredScrollRegionCurrentlyDiscardsTopLine() { vt100.setCursorLine(3); vt100.processNewline(); - assertEquals(5, term.getHeight()); // currently no history is created - assertEquals("1111", new String(term.getChars(0))); // "0000" was discarded (lost) - assertEquals("2222", new String(term.getChars(1))); - assertEquals("3333", new String(term.getChars(2))); - assertNull(term.getChars(3)); - assertEquals("4444", new String(term.getChars(4))); // footer below the region is untouched + assertEquals(6, term.getHeight()); // one line of history was created + assertEquals("0000", new String(term.getChars(0))); // ... holding the displaced top line + // the screen is now lines 1..5 of the model + assertEquals("1111", new String(term.getChars(1))); + assertEquals("2222", new String(term.getChars(2))); + assertEquals("3333", new String(term.getChars(3))); + assertNull(term.getChars(4)); // vacated line at the bottom of the region + assertEquals("4444", new String(term.getChars(5))); // footer below the region is untouched + assertEquals(3, vt100.getCursorLine()); // cursor stays at the bottom of the region } }