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 632d6626..5680b90a 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 f1d8558d..fa8e29b8 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; }