From 3704933d1e981706749998714f5a7db6b99919e9 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sun, 6 Aug 2023 07:22:09 +0100 Subject: [PATCH] Better unicode handling - Pass raw input into KeyEvent if it comes as unicode from jline - Fixes in InputView to handle wide chars and cursor position - Fixes #845 --- .../shell/component/view/TerminalUI.java | 4 +- .../component/view/control/InputView.java | 51 ++++--- .../shell/component/view/event/KeyEvent.java | 7 +- .../view/control/AbstractViewTests.java | 4 + .../view/control/InputViewTests.java | 137 +++++++++++++++--- 5 files changed, 161 insertions(+), 42 deletions(-) diff --git a/spring-shell-core/src/main/java/org/springframework/shell/component/view/TerminalUI.java b/spring-shell-core/src/main/java/org/springframework/shell/component/view/TerminalUI.java index 1593a498..77e03d7b 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/component/view/TerminalUI.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/component/view/TerminalUI.java @@ -40,8 +40,8 @@ import org.springframework.shell.component.view.event.EventLoop; import org.springframework.shell.component.view.event.KeyBinder; import org.springframework.shell.component.view.event.KeyEvent; import org.springframework.shell.component.view.event.KeyHandler; -import org.springframework.shell.component.view.event.MouseEvent; import org.springframework.shell.component.view.event.KeyHandler.KeyHandlerResult; +import org.springframework.shell.component.view.event.MouseEvent; import org.springframework.shell.component.view.event.MouseHandler; import org.springframework.shell.component.view.event.MouseHandler.MouseHandlerResult; import org.springframework.shell.component.view.geom.Rectangle; @@ -333,7 +333,7 @@ public class TerminalUI { else if (operation == KeyEvent.Key.Unicode) { String lastBinding = bindingReader.getLastBinding(); if (StringUtils.hasLength(lastBinding)) { - dispatchKeyEvent(KeyEvent.of(lastBinding.charAt(0))); + dispatchKeyEvent(KeyEvent.of(lastBinding)); } } else if (operation == KeyEvent.Key.Mouse) { diff --git a/spring-shell-core/src/main/java/org/springframework/shell/component/view/control/InputView.java b/spring-shell-core/src/main/java/org/springframework/shell/component/view/control/InputView.java index 1d349bea..ebdf2ead 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/component/view/control/InputView.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/component/view/control/InputView.java @@ -15,6 +15,9 @@ */ package org.springframework.shell.component.view.control; +import java.util.ArrayList; +import java.util.stream.Collectors; + import org.springframework.shell.component.view.event.KeyEvent; import org.springframework.shell.component.view.event.KeyEvent.Key; import org.springframework.shell.component.view.event.KeyHandler; @@ -29,8 +32,8 @@ import org.springframework.shell.component.view.screen.Screen; */ public class InputView extends BoxView { - private final StringBuilder text = new StringBuilder(); - private int cursorPosition = 0; + private final ArrayList text = new ArrayList<>(); + private int cursorIndex = 0; @Override protected void initInternal() { @@ -50,6 +53,9 @@ public class InputView extends BoxView { int plainKey = event.getPlainKey(); add(new String(new char[]{(char)plainKey})); } + else if (event.isKey(KeyEvent.Key.Unicode)) { + add(event.data()); + } return KeyHandler.resultOf(event, consumed, null); }; return handler.thenIfNotConsumed(super.getKeyHandler()); @@ -58,46 +64,51 @@ public class InputView extends BoxView { @Override protected void drawInternal(Screen screen) { Rectangle rect = getInnerRect(); - String s = text.toString(); + String s = getInputText(); screen.writerBuilder().build().text(s, rect.x(), rect.y()); screen.setShowCursor(hasFocus()); - screen.setCursorPosition(new Position(rect.x() + cursorPosition, rect.y())); + int sum = text.stream().limit(cursorIndex).mapToInt(text -> text.length()).sum(); + screen.setCursorPosition(new Position(rect.x() + sum, rect.y())); super.drawInternal(screen); } + /** + * Get a current known input text. + * + * @return current input text + */ public String getInputText() { - return text.toString(); + return text.stream().collect(Collectors.joining()); } - // private void enter(KeyEvent event) { - // // getShellMessageListener().onMessage(ShellMessageBuilder.ofViewFocus("enter", this)); - // } - - // private void leave(KeyEvent event) { - // // getShellMessageListener().onMessage(ShellMessageBuilder.ofViewFocus("leave", this)); - // } - private void add(String data) { - text.append(data); - right(); + text.add(data); + moveCursor(1); } private void backspace() { - if (cursorPosition > 0) { - text.deleteCharAt(cursorPosition - 1); + if (cursorIndex > 0) { + text.remove(cursorIndex - 1); } left(); } private void delete() { - text.deleteCharAt(cursorPosition); + text.remove(cursorIndex); + } + + private void moveCursor(int index) { + int toIndex = cursorIndex + index; + if (toIndex > -1 && toIndex <= text.size()) { + cursorIndex = toIndex; + } } private void left() { - cursorPosition--; + moveCursor(-1); } private void right() { - cursorPosition++; + moveCursor(1); } } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/component/view/event/KeyEvent.java b/spring-shell-core/src/main/java/org/springframework/shell/component/view/event/KeyEvent.java index c00a9141..772021cc 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/component/view/event/KeyEvent.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/component/view/event/KeyEvent.java @@ -23,12 +23,15 @@ package org.springframework.shell.component.view.event; * * */ -public record KeyEvent(int key) { +public record KeyEvent(int key, String data) { public static KeyEvent of(int key) { - return new KeyEvent(key); + return new KeyEvent(key, null); } + public static KeyEvent of(String raw) { + return new KeyEvent(Key.Unicode, raw); + } public boolean hasCtrl() { return ((key >> 30) & 1) == 1; diff --git a/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/AbstractViewTests.java b/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/AbstractViewTests.java index 8a44c618..1f261c40 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/AbstractViewTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/AbstractViewTests.java @@ -88,6 +88,10 @@ public class AbstractViewTests { return handleKeyEvent(view, KeyEvent.of(key)); } + protected KeyHandlerResult handleKey(View view, String key) { + return handleKeyEvent(view, KeyEvent.of(key)); + } + protected KeyHandlerResult handleKeyEvent(View view, KeyEvent key) { return view.getKeyHandler().handle(KeyHandler.argsOf(key)); } diff --git a/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/InputViewTests.java b/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/InputViewTests.java index 058d1454..4ad96695 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/InputViewTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/component/view/control/InputViewTests.java @@ -15,37 +15,138 @@ */ package org.springframework.shell.component.view.control; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.shell.component.view.event.KeyEvent; +import org.springframework.shell.component.view.event.KeyEvent.Key; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; class InputViewTests extends AbstractViewTests { - @Test - void shouldShowInput() { - InputView view = new InputView(); - view.setShowBorder(true); - view.setRect(0, 0, 80, 24); + private static final String CURSOR_INDEX_FIELD = "cursorIndex"; - dispatchEvent(view, KeyEvent.of('1')); - view.draw(screen24x80); + @Nested + class Input { + + InputView view; + + @BeforeEach + void setup() { + view = new InputView(); + configure(view); + } + + @Test + void shouldShowPlainText() { + view.setShowBorder(true); + view.setRect(0, 0, 80, 24); + + dispatchEvent(view, KeyEvent.of('1')); + view.draw(screen24x80); + + assertThat(forScreen(screen24x80)).hasHorizontalText("1", 1, 1, 1); + assertThat(forScreen(screen24x80)).hasCursorInPosition(2, 1); + } + + @Test + void shouldShowUnicode() { + view.setShowBorder(true); + view.setRect(0, 0, 80, 24); + + dispatchEvent(view, KeyEvent.of('★')); + view.draw(screen24x80); + + assertThat(forScreen(screen24x80)).hasHorizontalText("★", 1, 1, 1); + assertThat(forScreen(screen24x80)).hasCursorInPosition(2, 1); + } + + @Test + void shouldShowUnicodeEmoji() { + view.setShowBorder(true); + view.setRect(0, 0, 80, 24); + + dispatchEvent(view, KeyEvent.of("😂")); + view.draw(screen24x80); + + assertThat(forScreen(screen24x80)).hasHorizontalText("😂", 1, 1, 2); + assertThat(forScreen(screen24x80)).hasCursorInPosition(3, 1); + } - assertThat(forScreen(screen24x80)).hasHorizontalText("1", 1, 1, 1); - assertThat(forScreen(screen24x80)).hasCursorInPosition(2, 1); } - @Test - void shouldShowUnicode() { - InputView view = new InputView(); - view.setShowBorder(true); - view.setRect(0, 0, 80, 24); + @Nested + class CursorPositions { - dispatchEvent(view, KeyEvent.of('★')); - view.draw(screen24x80); + InputView view; + + @BeforeEach + void setup() { + view = new InputView(); + configure(view); + } + + int cursorIndex() { + return (Integer) ReflectionTestUtils.getField(view, CURSOR_INDEX_FIELD); + } + + @Test + void initialCursorPosition() { + assertThat(cursorIndex()).isEqualTo(0); + } + + @Test + void shouldNotMoveOutOfBoundsIfMovingRight() { + handleKey(view, Key.CursorRight); + assertThat(cursorIndex()).isEqualTo(0); + } + + @Test + void shouldNotMoveOutOfBoundsIfMovingLeft() { + handleKey(view, Key.CursorLeft); + assertThat(cursorIndex()).isEqualTo(0); + } + + @Test + void shouldMoveWithInputKeysNarrow() { + handleKey(view, Key.a); + assertThat(cursorIndex()).isEqualTo(1); + } + + @Test + void shouldMoveWithInputKeysWide() { + handleKey(view, "😂"); + assertThat(cursorIndex()).isEqualTo(1); + } - assertThat(forScreen(screen24x80)).hasHorizontalText("★", 1, 1, 1); - assertThat(forScreen(screen24x80)).hasCursorInPosition(2, 1); } + + @Nested + class MoveAndDeletions { + + InputView view; + + @BeforeEach + void setup() { + view = new InputView(); + configure(view); + } + + int cursorIndex() { + return (Integer) ReflectionTestUtils.getField(view, CURSOR_INDEX_FIELD); + } + + @Test + void addEmojiAndBackspace() { + handleKey(view, "😂"); + assertThat(cursorIndex()).isEqualTo(1); + handleKey(view, Key.Backspace); + assertThat(cursorIndex()).isEqualTo(0); + } + + } + }