Fix Chinese IME cursor rendering in terminal preedit text#9919
Fix Chinese IME cursor rendering in terminal preedit text#9919kill74 wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR stores terminal marked text with the IME selected range and uses that range to place the preedit cursor for CJK/double-width text.
Concerns
- The selected-range normalization assumes UTF-8 byte offsets, but the shared terminal path also receives macOS
NSRangeUTF-16 offsets; this can move CJK IME cursors back to the start of marked text on macOS.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| pub fn set_marked_text(&mut self, marked_text: &str, selected_range: &Range<usize>) { | ||
| self.marked_text = Some(MarkedText { | ||
| text: marked_text.to_string(), | ||
| selected_range: clamp_byte_range_to_char_boundaries( |
There was a problem hiding this comment.
selected_range as UTF-8 byte offsets, but the macOS path passes NSRange UTF-16 offsets directly into terminal marked text. For "中文", a macOS caret after the first character arrives as 1..1, gets clamped to 0..0, and renders the cursor at the start; normalize platform ranges to a common unit before this clamp.
kill74
force-pushed
the
fix-chinese-ime-marked-text-cursor
branch
from
de5a65a to
13a2274
Compare
kill74
force-pushed
the
fix-chinese-ime-marked-text-cursor
branch
from
13a2274 to
4264f6f
Compare
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @warpdotdev/oss-maintainers. Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
Reviewed the terminal grid changes for IME marked-text cursor positioning, selected-range normalization, wide-character cursor detection, and the related tests. The implementation is scoped to rendering state for preedit text and I did not identify correctness or security concerns in the changed lines.
Concerns
- None identified.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
oz-for-oss
Bot
requested review from
a team and
vorporeal
and removed request for
a team
|
reassigning to @abhishekp106, who has done the most IME work |
2 participants
Description
Fixes Chinese IME/preedit cursor rendering in terminal input, including when using Claude Code inside Warp.
Previously, the terminal grid stored only the marked text string and ignored the IME
selected_range. As a result, the cursor render point was calculated from the full marked-text cell length instead of the IME cursor position. For CJK text, this could cause the character under the cursor to appear visually split, clipped, or partially overwritten.This change stores marked text together with its selected range, clamps byte-indexed IME ranges to UTF-8 character boundaries, and calculates the cursor render point from the marked-text prefix before the IME cursor. It also makes wide-character cursor detection aware of marked text so the cursor is rendered correctly over CJK/double-width characters.
Linked Issue
Fixes #9906
ready-to-specorready-to-implement.Screenshots / Videos
Please attach the issue screenshots or a short before/after recording showing Chinese IME composition in Claude Code.
Testing
Automated tests added:
cursor_render_point_respects_marked_text_selected_range_with_cjkmarked_text_selected_range_is_clamped_to_char_boundaryis_cursor_on_wide_char_checks_marked_textAutomated checks run:
cargo fmtcargo test cursor_render_point_respects_marked_text_selected_range_with_cjkcargo test marked_text_selected_range_is_clamped_to_char_boundarycargo test is_cursor_on_wide_char_checks_marked_textAlso ran:
cargo nextest runcargo nextest rundid not complete successfully due to unrelated GCP cloud-provider tests failing on Windows:ai::agent_sdk::driver::cloud_provider::tests::collect_provider_env_vars_merges_all_providersai::agent_sdk::driver::cloud_provider::tests::extract_cloud_providers_creates_gcp_providerai::agent_sdk::driver::cloud_provider::tests::gcp_provider_env_varsThe failure appears unrelated to this IME/grid rendering change.
Manual test plan:
Agent Mode
CHANGELOG-BUG-FIX: Fixed Chinese IME preedit cursor rendering in terminal input on Windows.