From f13c182df412e208765dcb386858e735a3552f8d Mon Sep 17 00:00:00 2001 From: Fabrizio Iannetti Date: Sat, 1 May 2021 21:18:17 +0200 Subject: [PATCH] Bug 572938 [terminal] reverse linefeed: Honour the scrolling region The reverse linefeed should scroll down when the cursor is on the first line of the scrolling region, not only on the first screen line Change-Id: I628ab135d48d868bc8e3eacd2ea57dda948873a8 Signed-off-by: Fabrizio Iannetti --- .../emulator/IVT100EmulatorBackend.java | 8 +++++ .../emulator/VT100BackendTraceDecorator.java | 6 ++++ .../terminal/emulator/VT100Emulator.java | 4 +-- .../emulator/VT100EmulatorBackend.java | 14 +++++++++ .../terminal/emulator/VT100EmulatorTest.java | 30 +++++++++++++++++++ 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/IVT100EmulatorBackend.java b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/IVT100EmulatorBackend.java index 77f6f428aea..c835bb817c4 100644 --- a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/IVT100EmulatorBackend.java +++ b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/IVT100EmulatorBackend.java @@ -219,4 +219,12 @@ public interface IVT100EmulatorBackend { * @param lines number of lines to scroll */ void scrollDown(int lines); + + /** + * Process a reverse line feed/reverse index. + * + * The content is scrolled down if the cursor is at the top of the + * scroll region. + */ + void processReverseLineFeed(); } diff --git a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100BackendTraceDecorator.java b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100BackendTraceDecorator.java index 1c589c258c3..615a7d6f618 100644 --- a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100BackendTraceDecorator.java +++ b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100BackendTraceDecorator.java @@ -205,4 +205,10 @@ public class VT100BackendTraceDecorator implements IVT100EmulatorBackend { fBackend.scrollDown(lines); } + @Override + public void processReverseLineFeed() { + fWriter.println("processReverseLineFeed()"); //$NON-NLS-1$ + fBackend.processReverseLineFeed(); + } + } diff --git a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100Emulator.java b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100Emulator.java index 95969a16712..f899dcafb60 100644 --- a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100Emulator.java +++ b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100Emulator.java @@ -398,9 +398,7 @@ public class VT100Emulator implements ControlListener { case 'M': // Reverse line feed ansiState = ANSISTATE_INITIAL; - if (text.getCursorLine() == 0) - text.scrollDown(1); - moveCursorUp(1); + text.processReverseLineFeed(); break; default: diff --git a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorBackend.java b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorBackend.java index 959e8e85a2d..514ec7c6a0f 100644 --- a/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorBackend.java +++ b/terminal/plugins/org.eclipse.tm.terminal.control/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorBackend.java @@ -367,6 +367,20 @@ public class VT100EmulatorBackend implements IVT100EmulatorBackend { } } + private void doReverseLineFeed() { + if (fCursorLine == fScrollRegion.getTopLine()) + scrollDown(1); + else + setCursorLine(fCursorLine - 1); + } + + @Override + public void processReverseLineFeed() { + synchronized (fTerminal) { + doReverseLineFeed(); + } + } + @Override public int getCursorLine() { synchronized (fTerminal) { diff --git a/terminal/plugins/org.eclipse.tm.terminal.test/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorTest.java b/terminal/plugins/org.eclipse.tm.terminal.test/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorTest.java index f2f37e4de2e..b32128bf928 100644 --- a/terminal/plugins/org.eclipse.tm.terminal.test/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorTest.java +++ b/terminal/plugins/org.eclipse.tm.terminal.test/src/org/eclipse/tm/internal/terminal/emulator/VT100EmulatorTest.java @@ -41,6 +41,10 @@ public class VT100EmulatorTest { return "\033]0;" + title + "\007"; } + private static String SCROLL_REGION(int startRow, int endRow) { + return "\033[" + startRow + ";" + endRow + "r"; + } + /** * Set the cursor position to line/column. Note that this is the logical * line and column, so 1, 1 is the top left. @@ -310,4 +314,30 @@ public class VT100EmulatorTest { assertAll(() -> assertCursorLocation(1, expected.get(1).length()), () -> assertTextEquals(expected)); } + /** + * Runs what "up arrow" would send back to terminal in less/man/etc. + * but with a scrolling region set + */ + @Test + public void testScrollReverseScrollingRegion() { + data.setMaxHeight(1000); + List expected = new ArrayList<>(); + for (int i = 0; i < WINDOW_LINES; i++) { + String line = "Hello " + i; + run(line); + expected.add(line); + run("\r\n"); + } + assertAll(() -> assertCursorLocation(WINDOW_LINES, 0), () -> assertTextEquals(expected)); + run(CURSOR_POSITION_TOP_LEFT + "\n"); + assertAll(() -> assertCursorLocation(2, 0), () -> assertTextEquals(expected)); + run(SCROLL_REGION(2, WINDOW_LINES)); + run(SCROLL_REVERSE); + expected.add(2, ""); + assertAll(() -> assertCursorLocation(2, 0), () -> assertTextEquals(expected)); + run("New text on top line following scroll reverse"); + expected.set(2, "New text on top line following scroll reverse"); + assertAll(() -> assertCursorLocation(2, expected.get(2).length()), () -> assertTextEquals(expected)); + } + }