Skip to content

Fix #341: Switch chat input history only from the first or last visual line #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lars Vogel (vogella) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix #341: Switch chat input history only from the first or last visual line #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 113 additions & 1 deletion ...t.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -25,21 +39,35 @@ 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;
private ChatInputTextViewer chatInputTextViewer;

@BeforeEach
void setUp() {
lenient().when(mockChatServiceManager.getUserPreferenceService()).thenReturn(mockUserPreferenceService);
lenient().when(mockChatServiceManager.getChatCompletionService()).thenReturn(mockChatCompletionService);
Comment on lines +58 to +59
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());
});

}
Expand Down Expand Up @@ -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<Throwable> 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);
}
}
40 changes: 15 additions & 25 deletions ...oft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatInputTextViewer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -350,7 +358,6 @@ private void removeLineBreak() {
}

private void handleSendMessage() {
resetCaretLineOffsetStatus();
Optional.ofNullable(this.sendMessageHandler).ifPresent(handler -> handler.accept(this.getContent()));
}

Expand Down Expand Up @@ -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;
}
Expand Down