From e4f9923dbe1e20e986d1ed91e6705ad860045c9e Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 5 Nov 2026 19:07:04 +0100 Subject: [PATCH] Fix #341: Switch chat input history only from the first or last visual line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chat input guessed from the logical line number whether an arrow key moved the caret, so moving within a wrapped line, or from a line reached by mouse, replaced the prompt with an older one. It now checks before the key is handled whether the caret is on the first or last visual line and only then switches the history. Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../ui/chat/ChatInputTextViewerTests.java | 114 +++++++++++++++++- .../eclipse/ui/chat/ChatInputTextViewer.java | 40 +++--- 2 files changed, 128 insertions(+), 26 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewerTests.java index 632d66267..5680b90a9 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewerTests.java @@ -4,10 +4,21 @@ package com.microsoft.copilot.eclipse.ui.chat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.jface.text.contentassist.ContentAssistant; import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.StyledText; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Event; import org.eclipse.swt.widgets.Shell; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -16,7 +27,10 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import com.microsoft.copilot.eclipse.ui.chat.services.ChatCompletionService; +import com.microsoft.copilot.eclipse.ui.chat.services.ChatFontService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; +import com.microsoft.copilot.eclipse.ui.chat.services.UserPreferenceService; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; @ExtendWith(MockitoExtension.class) @@ -25,6 +39,15 @@ class ChatInputTextViewerTests { @Mock private ChatServiceManager mockChatServiceManager; + @Mock + private UserPreferenceService mockUserPreferenceService; + + @Mock + private ChatCompletionService mockChatCompletionService; + + @Mock + private ChatFontService mockChatFontService; + private Display display; private Shell shell; private Composite parent; @@ -32,14 +55,19 @@ class ChatInputTextViewerTests { @BeforeEach void setUp() { + lenient().when(mockChatServiceManager.getUserPreferenceService()).thenReturn(mockUserPreferenceService); + lenient().when(mockChatServiceManager.getChatCompletionService()).thenReturn(mockChatCompletionService); + lenient().when(mockChatServiceManager.getChatFontService()).thenReturn(mockChatFontService); + // Set up SWT components - SwtUtils.invokeOnDisplayThread(() -> { + runOnDisplayThread(() -> { display = Display.getDefault(); shell = new Shell(display); parent = new Composite(shell, SWT.NONE); // Create the viewer chatInputTextViewer = new ChatInputTextViewer(parent, mockChatServiceManager); + chatInputTextViewer.setContentAssistProcessor(new ContentAssistant()); }); } @@ -99,4 +127,88 @@ void testRedoFunctionality() { assertEquals("Modified text", chatInputTextViewer.getContent()); }); } + + @Test + void testArrowUpMovesWithinWrappedLineInsteadOfSwitchingHistory() { + runOnDisplayThread(() -> { + StyledText textWidget = chatInputTextViewer.getTextWidget(); + textWidget.setSize(150, 300); + String longLine = "word ".repeat(100); + chatInputTextViewer.setContent(longLine); + int caretOffset = textWidget.getCaretOffset(); + + pressKey(SWT.ARROW_UP); + + assertEquals(longLine, chatInputTextViewer.getContent()); + assertNotEquals(caretOffset, textWidget.getCaretOffset()); + verify(mockUserPreferenceService, never()).getPreviousInput(anyString()); + }); + } + + @Test + void testArrowDownMovesWithinWrappedLineInsteadOfSwitchingHistory() { + runOnDisplayThread(() -> { + StyledText textWidget = chatInputTextViewer.getTextWidget(); + textWidget.setSize(150, 300); + String longLine = "word ".repeat(100); + chatInputTextViewer.setContent(longLine); + textWidget.setSelection(0); + + pressKey(SWT.ARROW_DOWN); + + assertEquals(longLine, chatInputTextViewer.getContent()); + assertNotEquals(0, textWidget.getCaretOffset()); + verify(mockUserPreferenceService, never()).getNextInput(); + }); + } + + @Test + void testArrowUpOnFirstLineSwitchesToPreviousInput() { + runOnDisplayThread(() -> { + when(mockUserPreferenceService.getPreviousInput("current")).thenReturn("older"); + chatInputTextViewer.getTextWidget().setSize(150, 300); + chatInputTextViewer.setContent("current"); + + pressKey(SWT.ARROW_UP); + + assertEquals("older", chatInputTextViewer.getContent()); + }); + } + + @Test + void testArrowDownOnLastLineSwitchesToNextInput() { + runOnDisplayThread(() -> { + when(mockUserPreferenceService.getNextInput()).thenReturn("newer"); + chatInputTextViewer.getTextWidget().setSize(150, 300); + chatInputTextViewer.setContent("current"); + + pressKey(SWT.ARROW_DOWN); + + assertEquals("newer", chatInputTextViewer.getContent()); + }); + } + + // The workbench's syncExec swallows exceptions, so rethrow them on the test thread + private void runOnDisplayThread(Runnable runnable) { + AtomicReference failure = new AtomicReference<>(); + SwtUtils.invokeOnDisplayThread(() -> { + try { + runnable.run(); + } catch (Throwable t) { + failure.set(t); + } + }); + if (failure.get() instanceof Error error) { + throw error; + } + if (failure.get() instanceof RuntimeException exception) { + throw exception; + } + } + + private void pressKey(int keyCode) { + Event event = new Event(); + event.keyCode = keyCode; + chatInputTextViewer.getTextWidget().notifyListeners(SWT.KeyDown, event); + } } \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewer.java index f1d8558d1..fa8e29b82 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewer.java @@ -53,8 +53,8 @@ public class ChatInputTextViewer extends UndoableTextViewer implements PaintList private ChatServiceManager chatServiceManager; private ContentAssistant contentAssistant; - private boolean caretLineOffsetChanged = false; - private int lastCursorLineOffset = 0; + private boolean caretOnFirstVisualLine = true; + private boolean caretOnLastVisualLine = true; private Color placeholderColor; @@ -139,6 +139,13 @@ public void modifyText(org.eclipse.swt.events.ModifyEvent e) { } }); + // Runs before StyledText handles the key, which it does before notifying the key listeners. + tvw.addVerifyKeyListener(e -> { + int caretY = tvw.getLocationAtOffset(tvw.getCaretOffset()).y; + caretOnFirstVisualLine = caretY == tvw.getLocationAtOffset(0).y; + caretOnLastVisualLine = caretY == tvw.getLocationAtOffset(tvw.getCharCount()).y; + }); + tvw.addKeyListener(new KeyAdapter() { @Override public void keyPressed(KeyEvent e) { @@ -235,11 +242,7 @@ private void onKeyPressed(KeyEvent e) { } String text = this.getContent(); - // check the caret status so that we know if this is moving caret through multiple lines, or it's a switching - this.updateCaretLineOffsetStatus(); if (handleArrowKeyEvent(e, text)) { - // caret status need update since arrow key event may change the position via switching input history - this.updateCaretLineOffsetStatus(); return; } // If current char is not line break, it means assistant pop up is visible and assistant listener handle it @@ -290,7 +293,12 @@ private void onKeyPressed(KeyEvent e) { * Return true if the event is handled, false otherwise. */ private boolean handleArrowKeyEvent(KeyEvent e, String text) { - if (this.caretLineOffsetChanged || this.isProposalPopupActive()) { + if (this.isProposalPopupActive()) { + return false; + } + // Switch the input history only from the first or last visual line, otherwise the key navigates the text. + if ((e.keyCode == SWT.ARROW_UP && !caretOnFirstVisualLine) + || (e.keyCode == SWT.ARROW_DOWN && !caretOnLastVisualLine)) { return false; } if (e.keyCode == SWT.ARROW_UP) { @@ -350,7 +358,6 @@ private void removeLineBreak() { } private void handleSendMessage() { - resetCaretLineOffsetStatus(); Optional.ofNullable(this.sendMessageHandler).ifPresent(handler -> handler.accept(this.getContent())); } @@ -384,23 +391,6 @@ private String getPlaceholderText() { } } - private void updateCaretLineOffsetStatus() { - StyledText textWidget = this.getTextWidget(); - int caretOffset = textWidget.getCaretOffset(); - int offset = textWidget.getLineAtOffset(caretOffset); - if (lastCursorLineOffset != offset) { - lastCursorLineOffset = offset; - caretLineOffsetChanged = true; - } else { - caretLineOffsetChanged = false; - } - } - - private void resetCaretLineOffsetStatus() { - lastCursorLineOffset = 0; - caretLineOffsetChanged = false; - } - public void setContentAssistProcessor(ContentAssistant ca) { contentAssistant = ca; }