refactor(terminal): Remove KeyRepeatController#6
Conversation
… 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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough本次变更从 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/src/terminal_view.dart (2)
600-615: 请补一轮原生 repeat 的跨平台回归。删除
KeyRepeatController后,这里完全依赖 Flutter/平台上报的KeyRepeatEvent;而Terminal.keyInput()(lib/src/terminal.dart:243-267)和TerminalInputHandler(lib/src/core/input/handler.dart:64-70)都没有后备的重复合成或过滤。如果某些 target/IME 不会为 Backspace、方向键这类非文本键发送 repeat,长按会直接退化成单次输入。建议至少在本 PR 覆盖的 Windows 11、Android 外接键盘、iOS 外接键盘场景回归一次,并补一个覆盖KeyDownEvent、KeyRepeatEvent和KeyUpEvent的回归测试。🤖 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
📒 Files selected for processing (3)
lib/src/terminal_view.dartlib/src/ui/key_repeat_controller.darttest/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
|
@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. |
|
Tip For best results, initiate chat on the files or code changes.
Removing 🧠 Learnings used |
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=truestate, 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
发布说明
重构
移除
测试