From 715af394358364eaef4412ae5f394b3232976415 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-screen CLIs such as Codex in --no-alt-screen mode, or Vim kept off the alternate screen, scroll through a DECSTBM region anchored at the top of the screen. The emulator scrolled such a region in place and dropped the displaced top line, so no scroll-back history was created. Like xterm and VTE, a newline at the bottom of such a region now moves the top line into the history while the lines below the region stay in place. With a full history the scroll reaches snapshots as one upward shift, so selections move with the text, and a full TerminalTextDataFastScroll rotates its ring buffer and copies back only the fixed lines instead of the whole history. Regions with a lower top margin still scroll in place. Fixes the missing scroll-back of https://github.com/eclipse-platform/eclipse.platform/issues/2680 Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../emulator/VT100EmulatorBackend.java | 28 +++- .../model/TerminalTextDataFastScroll.java | 8 + .../emulator/VT100EmulatorBackendTest.java | 144 ++++++++++++++++-- .../model/AbstractITerminalTextDataTest.java | 5 + .../model/TerminalTextDataFastScrollTest.java | 38 ++++- 5 files changed, 205 insertions(+), 18 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 fe9e0b9a2e8..e5004425a84 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 @@ -729,7 +729,11 @@ private int doLineWrap() { private void doNewline() { fWrapPending = false; // DEC STD 070: any move of the cursor forgets a pending wrap 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(); @@ -741,6 +745,28 @@ 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() { + int regionBottom = fScrollRegion.getBottomLine(); + if (regionBottom == fLines - 1) { + fTerminal.addLine(); + } else if (fTerminal.getMaxHeight() > 0 && fTerminal.getHeight() < fTerminal.getMaxHeight()) { + // the history grows, so the former top line stays where it is and becomes history; + // shift the lines below the region back down, vacating the bottom line of the region + fTerminal.addLine(); + fTerminal.scroll(toAbsoluteLine(regionBottom), fLines - regionBottom, 1); + } else { + // one negative shift that snapshots can track, so selections move with the text + fTerminal.scroll(0, toAbsoluteLine(regionBottom) + 1, -1); + } + // the absolute line of the cursor may have changed + setCursorLine(fCursorLine); + } + @Override public void processNewline() { synchronized (fTerminal) { diff --git a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScroll.java b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScroll.java index c76fab20863..2818dc25a49 100644 --- a/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScroll.java +++ b/terminal/bundles/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScroll.java @@ -186,6 +186,14 @@ public void scroll(int startLine, int size, int shift) { } else { cleanLines(startLine, Math.min(shift, getHeight() - startLine)); } + } else if (startLine == 0 && shift < 0 && fHeight - size < size + shift && fHeight == fMaxHeight) { + // rotate and copy back only the fewer lines below the region; on a ring with spare + // slots a later addLine() would expose the rotated-out lines + moveOffset(-shift); + for (int i = fHeight - 1; i >= size; i--) { + fData.copyLine(fData, getPositionOfLine(i + shift), getPositionOfLine(i)); + } + cleanLines(size + shift, -shift); } else { // we have to copy the lines. if (shift < 0) { 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 02a53b13994..ae5c4c7fde8 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 @@ -31,6 +31,7 @@ import org.eclipse.terminal.internal.model.TerminalTextTestHelper; import org.eclipse.terminal.model.ITerminalTextData; import org.eclipse.terminal.model.ITerminalTextDataReadOnly; +import org.eclipse.terminal.model.ITerminalTextDataSnapshot; import org.eclipse.terminal.model.TerminalStyle; import org.junit.jupiter.api.Test; @@ -1006,12 +1007,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. // --------------------------------------------------------------------------------------------- /** @@ -1088,14 +1084,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); @@ -1106,12 +1099,131 @@ 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(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 + } + + /** + * The same, once the scroll-back is full: the model no longer grows, so the oldest history line + * is dropped instead. This is the steady state of a long running full screen application. + */ + @Test + public void testNewlineInTopAnchoredScrollRegionWithFullHistoryDropsOldestLine() { + ITerminalTextData term = makeITerminalTextData(); + IVT100EmulatorBackend vt100 = makeBakend(term); + term.setMaxHeight(6); + vt100.setDimensions(5, 4); + fill(term, "hhhh\n" + "0000\n" + "1111\n" + "2222\n" + "3333\n" + "4444"); + assertEquals(6, term.getHeight()); // one line of history, five lines of screen + + vt100.setScrollRegion(0, 3); + vt100.setCursorLine(3); + vt100.processNewline(); + + assertEquals(6, term.getHeight()); // capped, so "hhhh" fell out of the history + assertEquals("0000", new String(term.getChars(0))); // the displaced top line took its place + 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()); + } + + /** + * With a full history the scroll must reach snapshots as one upward shift of history and region, + * otherwise selections in the region are not moved along with the text. + */ + @Test + public void testNewlineInTopAnchoredScrollRegionWithFullHistoryNotifiesScrollToSnapshots() { + ITerminalTextData term = new TerminalTextData(); + IVT100EmulatorBackend vt100 = makeBakend(term); + term.setMaxHeight(6); + vt100.setDimensions(5, 4); + fill(term, "hhhh\n" + "0000\n" + "1111\n" + "2222\n" + "3333\n" + "4444"); + ITerminalTextDataSnapshot snapshot = term.makeSnapshot(); + snapshot.updateSnapshot(true); + + vt100.setScrollRegion(0, 3); + vt100.setCursorLine(3); + vt100.processNewline(); + snapshot.updateSnapshot(true); + + assertEquals(0, snapshot.getScrollWindowStartLine()); + assertEquals(5, snapshot.getScrollWindowSize()); // history and region, not the footer + assertEquals(-1, snapshot.getScrollWindowShift()); + assertEquals("0000", new String(snapshot.getChars(0))); + assertEquals("1111", new String(snapshot.getChars(1))); + assertEquals("2222", new String(snapshot.getChars(2))); + assertEquals("3333", new String(snapshot.getChars(3))); + assertNull(snapshot.getChars(4)); + assertEquals("4444", new String(snapshot.getChars(5))); + } + + /** + * A footer of several lines stays in place, both while the history grows and once it is full. + */ + @Test + public void testNewlineInTopAnchoredScrollRegionWithTallFooter() { + ITerminalTextData term = new TerminalTextData(); + IVT100EmulatorBackend vt100 = makeBakend(term); + term.setMaxHeight(6); + vt100.setDimensions(5, 4); + fill(term, "0000\n" + "1111\n" + "2222\n" + "3333\n" + "4444"); + vt100.setScrollRegion(0, 2); + vt100.setCursorLine(2); + + vt100.processNewline(); // the history grows + assertEquals(6, term.getHeight()); + assertEquals("0000", new String(term.getChars(0))); + assertEquals("1111", new String(term.getChars(1))); + assertEquals("2222", new String(term.getChars(2))); + assertNull(term.getChars(3)); + assertEquals("3333", new String(term.getChars(4))); + assertEquals("4444", new String(term.getChars(5))); + + vt100.processNewline(); // the history is full + assertEquals(6, term.getHeight()); + assertEquals("1111", new String(term.getChars(0))); assertEquals("2222", new String(term.getChars(1))); - assertEquals("3333", new String(term.getChars(2))); + assertNull(term.getChars(2)); assertNull(term.getChars(3)); - assertEquals("4444", new String(term.getChars(4))); // footer below the region is untouched + assertEquals("3333", new String(term.getChars(4))); + assertEquals("4444", new String(term.getChars(5))); + assertEquals(2, vt100.getCursorLine()); + } + + /** + * A top-anchored scroll region that covers the whole screen has no fixed lines below it, so the + * screen just moves up by one line. + */ + @Test + public void testNewlineInTopAnchoredScrollRegionCoveringWholeScreen() { + ITerminalTextData term = makeITerminalTextData(); + IVT100EmulatorBackend vt100 = makeBakend(term); + term.setMaxHeight(10); + vt100.setDimensions(5, 4); + fill(term, "0000\n" + "1111\n" + "2222\n" + "3333\n" + "4444"); + + vt100.setScrollRegion(0, 4); // the region is the whole screen + vt100.setCursorLine(4); + vt100.processNewline(); + + assertEquals(6, term.getHeight()); + assertEquals("0000", new String(term.getChars(0))); // displaced top line + assertEquals("1111", new String(term.getChars(1))); + assertEquals("2222", new String(term.getChars(2))); + assertEquals("3333", new String(term.getChars(3))); + assertEquals("4444", new String(term.getChars(4))); + assertNull(term.getChars(5)); // new blank line at the bottom of the screen + assertEquals(4, vt100.getCursorLine()); } @Test diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/AbstractITerminalTextDataTest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/AbstractITerminalTextDataTest.java index f1d3857ba27..43b9ad2a00b 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/AbstractITerminalTextDataTest.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/AbstractITerminalTextDataTest.java @@ -573,6 +573,11 @@ public void testScrollNegative() { scrollTest(1, 2, -1, "012345", "02 345"); scrollTest(5, 1, -1, "012345", "01234 "); scrollTest(5, 1, -1, "012345", "01234 "); + scrollTest(0, 5, -1, "012345", "1234 5"); + scrollTest(0, 5, -2, "012345", "234 5"); + scrollTest(0, 4, -1, "012345", "123 45"); + scrollTest(0, 4, -3, "012345", "3 45"); + scrollTest(0, 3, -1, "012345", "12 345"); } @Test diff --git a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScrollTest.java b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScrollTest.java index 90e116b4b62..59a287db238 100644 --- a/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScrollTest.java +++ b/terminal/tests/org.eclipse.terminal.test/src/org/eclipse/terminal/internal/model/TerminalTextDataFastScrollTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2018 Wind River Systems, Inc. and others. + * Copyright (c) 2007, 2026 Wind River Systems, Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -10,7 +10,12 @@ *******************************************************************************/ package org.eclipse.terminal.internal.model; +import static org.eclipse.terminal.internal.model.TerminalTextTestHelper.fillSimple; +import static org.eclipse.terminal.internal.model.TerminalTextTestHelper.toSimple; +import static org.junit.jupiter.api.Assertions.assertEquals; + import org.eclipse.terminal.model.ITerminalTextData; +import org.junit.jupiter.api.Test; public class TerminalTextDataFastScrollTest extends AbstractITerminalTextDataTest { @Override @@ -18,4 +23,35 @@ protected ITerminalTextData makeITerminalTextData() { return new TerminalTextDataFastScroll(3); } + /** + * Scrolling a top-anchored region while the ring has unused capacity must not leave scrolled + * out text where a later {@link ITerminalTextData#addLine()} grows into. + */ + @Test + public void testScrollTopAnchoredRegionThenGrow() { + ITerminalTextData term = new TerminalTextDataFastScroll(7); + fillSimple(term, "012345"); + term.scroll(0, 5, -1); + term.addLine(); + assertEquals("1234 5 ", toSimple(term)); + } + + /** + * Repeated top-anchored scrolls on a full ring, so the rotation also runs from a non-zero + * offset. + */ + @Test + public void testScrollTopAnchoredRegionRepeatedly() { + ITerminalTextData term = new TerminalTextDataFastScroll(6); + fillSimple(term, "012345"); + term.scroll(0, 6, -2); + assertEquals("2345 ", toSimple(term)); + term.setChar(4, 0, 'a', null); + term.setChar(5, 0, 'b', null); + term.scroll(0, 5, -1); + assertEquals("345a b", toSimple(term)); + term.scroll(0, 5, -1); + assertEquals("45a b", toSimple(term)); + } + }