fix(shell): approval request text input should render caret correctly#2005
fix(shell): approval request text input should render caret correctly#2005wbxl2000 merged 9 commits intoMoonshotAI:mainfrom
Conversation
Signed-off-by: _Kerman <kermanx@qq.com>
There was a problem hiding this comment.
Pull request overview
Fixes caret rendering for the shell’s approval-request inline feedback input so the “block cursor” appears at the actual cursor position (using reverse-video on the character under the cursor when mid-line), aligning the visual behavior with typical terminal cursors.
Changes:
- Render inline feedback with a cursor-aware display (reverse-video at cursor, trailing block only at EOL).
- Plumb cursor position from the prompt buffer into the approval panel rendering.
- Add tests covering cursor-in-middle vs cursor-at-end rendering; update changelog entries.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ui_and_conv/test_visualize_running_prompt.py | Updates the delegate helper to provide (text, cursor) and adds assertions for reverse-video vs trailing block cursor behavior. |
| src/kimi_cli/ui/shell/visualize/_approval_panel.py | Introduces cursor-aware rendering helper and threads cursor info through ApprovalRequestPanel.render() and ApprovalPromptDelegate. |
| src/kimi_cli/ui/shell/init.py | Provides a default (text, cursor) provider from the prompt buffer for the approval modal. |
| docs/zh/release-notes/changelog.md | Adds a Chinese changelog entry describing the cursor rendering fix. |
| docs/en/release-notes/changelog.md | Adds the English changelog entry for the cursor rendering fix. |
| CHANGELOG.md | Adds the root changelog entry for the cursor rendering fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def __init__( | ||
| self, | ||
| request: ApprovalRequest, | ||
| *, | ||
| on_response: Callable[[ApprovalRequest, ApprovalResponse.Kind, str], None], | ||
| buffer_text_provider: Callable[[], str] | None = None, | ||
| buffer_text_provider: Callable[[], tuple[str, int]] | None = None, | ||
| text_expander: Callable[[str], str] | None = None, | ||
| ) -> None: |
There was a problem hiding this comment.
ApprovalPromptDelegate is re-exported as a public name (see ui/shell/visualize/__init__.py), but this change makes buffer_text_provider a breaking API change (callers previously returning str will now cause an unpacking error in render_running_prompt_body). Consider keeping backward compatibility by accepting both Callable[[], str] and Callable[[], tuple[str, int]] (or adding a new optional cursor provider) and normalizing the return value internally.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Signed-off-by: _Kerman <kermanx@qq.com>
|
补充一个跨平台信息：这个 bug 不仅在 Linux/Windows 上存在，Mac 上同样会复现——它是 app 层渲染逻辑问题（之前 修复用的 ANSI reverse video (SGR 7) 和 Unicode Adding cross-platform information: This bug not only exists on Linux/Windows, but also on Mac - it is an app layer rendering logic problem (previously The ANSI reverse video (SGR 7) and Unicode |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 382c93f254
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| at_cursor = escape(text[cursor]) | ||
| after = escape(text[cursor + 1 :]) | ||
| return f"{before}[reverse]{at_cursor}[/reverse]{after}" |
There was a problem hiding this comment.
When the cursor sits on a backslash character (e.g. feedback like C:\temp), escape(text[cursor]) leaves \ unchanged, so the generated snippet becomes ...[reverse]\[/reverse].... In Rich markup, that \[ escapes the closing tag, which turns [/reverse] into literal text and leaves reverse styling open for the rest of the line. This regresses rendering for common inputs containing backslashes and makes the inline approval prompt display corrupted cursor/text state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 606d3236e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| from kimi_cli.ui.shell.visualize._approval_panel import _render_feedback_with_cursor | ||
|
|
||
| # Cursor at position 2 ("he|llo world" — on the first 'l'). | ||
| assert _render_feedback_with_cursor("hello world", 2) == "he[reverse]l[/reverse]lo world" |
There was a problem hiding this comment.
Text objects
_render_feedback_with_cursor now returns a rich.text.Text instance, but this test compares it to markup/plain strings, which will evaluate false (Text.__eq__ only handles Text). As a result, the new cursor tests fail even when rendering is correct, which can block CI and mask real regressions; assertions should inspect Text.plain / spans (or compare against Text(...)) instead of string literals with markup tags.
Useful? React with 👍 / 👎.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Related Issue
Description
as title.
Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.