Skip to content

refactor(terminal): Remove KeyRepeatController#6

Merged
GT-610 merged 2 commits intolollipopkit:masterfrom
GT-610:master
Mar 19, 2026
Merged

refactor(terminal): Remove KeyRepeatController#6
GT-610 merged 2 commits intolollipopkit:masterfrom
GT-610:master

Conversation

@GT-610
Copy link
Copy Markdown
Collaborator

@GT-610 GT-610 commented Mar 19, 2026

Modern operating systems already include native implementations for handling keyboard repeat events; leaving this issue unresolved will result in a single keystroke being repeated.

The root cause lies in the custom key repeat handling logic in xterm-dart: it keeps specific keys (such as Backspace and the Up arrow) in the repeat=true state, causing the program to mistakenly believe that “this key is held down.”

This feature should be deprecated. Given that it is completely unnecessary, this PR will remove it entirely.

Resolve lollipopkit/flutter_server_box/issues/1072.
Resolve lollipopkit/flutter_server_box/issues/987.
Resolve lollipopkit/flutter_server_box/issues/981.
Resolve lollipopkit/flutter_server_box/issues/953.

Summary by CodeRabbit

发布说明

  • 重构

    • 简化并统一了终端视图的键盘事件处理,减少了重复与复杂行为。
  • 移除

    • 删除了此前用于合成键重复行为的旧实现。
  • 测试

    • 移除了与该键重复处理实现相关的测试用例。

… logic

The keyboard repeat functionality is now natively supported by the platform, so a custom KeyRepeatController is no longer needed. Remove the relevant code to simplify the implementation and reduce maintenance costs.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0a48e3e2-1664-4b10-8623-787ef6b3578d

📥 Commits

Reviewing files that changed from the base of the PR and between 46e8e6f and 52a941b.

📒 Files selected for processing (1)
  • lib/src/terminal_view.dart

📝 Walkthrough

Walkthrough

本次变更从 TerminalViewState 中移除了 KeyRepeatController 的所有使用:删除导入、常量集、字段、initState/dispose 中的管理、焦点触发的取消逻辑以及原先委托给控制器的按键重复处理路径。完整删除 lib/src/ui/key_repeat_controller.dart 文件及其对应测试。_handleKeyEvent 现在仅接受 KeyDownEvent/KeyRepeatEvent,将 event.logicalKey 映射为 TerminalKey 并直接调用 _sendTerminalKey,不再进行重复发送或取消处理。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR标题清晰准确地反映了主要变更内容——移除KeyRepeatController及相关逻辑。
Linked Issues check ✅ Passed PR中删除的KeyRepeatController正是导致#1072、#987、#981、#953中键盘重复问题的根本原因,移除该控制器直接解决所有链接问题。
Out of Scope Changes check ✅ Passed 所有变更均围绕移除KeyRepeatController及其使用,无超出范围的改动。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
lib/src/terminal_view.dart (2)

600-615: 请补一轮原生 repeat 的跨平台回归。

删除 KeyRepeatController 后,这里完全依赖 Flutter/平台上报的 KeyRepeatEvent;而 Terminal.keyInput()lib/src/terminal.dart:243-267)和 TerminalInputHandlerlib/src/core/input/handler.dart:64-70)都没有后备的重复合成或过滤。如果某些 target/IME 不会为 Backspace、方向键这类非文本键发送 repeat,长按会直接退化成单次输入。建议至少在本 PR 覆盖的 Windows 11、Android 外接键盘、iOS 外接键盘场景回归一次,并补一个覆盖 KeyDownEventKeyRepeatEventKeyUpEvent 的回归测试。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/src/terminal_view.dart` around lines 600 - 615, 当前删除 KeyRepeatController
导致依赖平台发送 KeyRepeatEvent,若某些平台/IME 不为非文本键(Backspace/箭头)发送 repeat 会降级为单次输入;请在
terminal_view.dart 的按键路径增加本地重复合成 fallback:在处理 KeyDownEvent 时启动一个可取消的重复计时器并在
KeyUpEvent 时取消(补上之前 KeyRepeatController 的职责),确保 _sendTerminalKey(...) /
Terminal.keyInput() / TerminalInputHandler 都能收到合成的重复事件并与原生 KeyRepeatEvent
共存(优先原生),并补一组回归测试覆盖 KeyDownEvent/KeyRepeatEvent/KeyUpEvent 行为在 Windows11、Android
外接键盘 和 iOS 外接键盘 场景下的表现。

480-483: 把空分支删干净,避免误导。

这里的 if (!_focusNode.hasFocus) {} 已经没有任何行为了,看起来像删除重复控制逻辑后留下的空壳。直接去掉分支会更清晰。

♻️ 建议修改
  void _handleFocusChange() {
-    if (!_focusNode.hasFocus) {
-    }
    _updateCursorBlink(resetVisible: true);
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/src/terminal_view.dart` around lines 480 - 483, Remove the empty
conditional in _handleFocusChange: delete the no-op "if (!_focusNode.hasFocus) {
}" so the function simply calls _updateCursorBlink(resetVisible: true); ensure
you only remove the empty branch and do not change the call to
_updateCursorBlink or any focus logic elsewhere (references: _handleFocusChange,
_focusNode, _updateCursorBlink).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/src/terminal_view.dart`:
- Around line 600-615: 当前删除 KeyRepeatController 导致依赖平台发送
KeyRepeatEvent,若某些平台/IME 不为非文本键(Backspace/箭头)发送 repeat 会降级为单次输入;请在
terminal_view.dart 的按键路径增加本地重复合成 fallback:在处理 KeyDownEvent 时启动一个可取消的重复计时器并在
KeyUpEvent 时取消(补上之前 KeyRepeatController 的职责),确保 _sendTerminalKey(...) /
Terminal.keyInput() / TerminalInputHandler 都能收到合成的重复事件并与原生 KeyRepeatEvent
共存(优先原生),并补一组回归测试覆盖 KeyDownEvent/KeyRepeatEvent/KeyUpEvent 行为在 Windows11、Android
外接键盘 和 iOS 外接键盘 场景下的表现。
- Around line 480-483: Remove the empty conditional in _handleFocusChange:
delete the no-op "if (!_focusNode.hasFocus) { }" so the function simply calls
_updateCursorBlink(resetVisible: true); ensure you only remove the empty branch
and do not change the call to _updateCursorBlink or any focus logic elsewhere
(references: _handleFocusChange, _focusNode, _updateCursorBlink).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 51f8ae07-560f-49a5-9ef1-3f6587705ca3

📥 Commits

Reviewing files that changed from the base of the PR and between 8021182 and 46e8e6f.

📒 Files selected for processing (3)
  • lib/src/terminal_view.dart
  • lib/src/ui/key_repeat_controller.dart
  • test/src/ui/key_repeat_controller_test.dart
💤 Files with no reviewable changes (2)
  • test/src/ui/key_repeat_controller_test.dart
  • lib/src/ui/key_repeat_controller.dart

@GT-610
Copy link
Copy Markdown
Collaborator Author

GT-610 commented Mar 19, 2026

@coderabbitai I tested this on Windows 11, Android 10–16 (using both built-in and external keyboards), and iOS (using both built-in and external keyboards), and encountered no issues.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

Tip

For best results, initiate chat on the files or code changes.

@GT-610 That's excellent news! 🎉 Your testing coverage is comprehensive and directly addresses all the platforms and scenarios mentioned in the linked issues:

Platform Related Issues
Windows 11 #1072 (Up arrow jumping to earliest history)
Android (built-in & external keyboard) #987 (arrow key continuous repeat), #953 (Delete key deleting all characters)
iOS (built-in & external keyboard) #981 (uncontrolled input after app switch)

Removing KeyRepeatController and relying on native OS key repeat handling is the right call — the custom implementation was clearly fighting against platform-level behavior. The clean deletion of ~200 lines of custom timer logic (including the controller, its integration in TerminalViewState, and its test suite) with no regressions across all target platforms is a strong signal this PR is ready to merge.


🧠 Learnings used
Learnt from: CR
Repo: lollipopkit/xterm.dart PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-10T08:14:19.005Z
Learning: Applies to lib/src/core/**/*.dart : Core terminal emulation logic should be independent of Flutter and organized in `lib/core.dart` with sub-modules: `buffer/` (line and cell management), `escape/` (ANSI sequence parsing), `input/` (keyboard handling), and `mouse/` (mouse input handling)

@GT-610 GT-610 merged commit 13708c9 into lollipopkit:master Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment