Repository navigation
Conversation
…rlay A mid-line composition visually swallows the character after the cursor: the preedit overlay (.composition-view) is an opaque box anchored to the cursor cell, and nothing reaches the PTY while composing, so the covered cells still hold their characters — the box just hides them until the syllable commits. Composing 가 with the cursor before 하 in 안녕하세요 blanks 하 for the whole composition (stablyai#12545). Long-standing overlay behavior in the vendored terminal library, not a regression; Terminal.app's native marked-text rendering handles the same case correctly. The fix renders the rest of the row's committed text after the preedit inside the overlay, so the composition reads as inserted text pushing the tail right. The preedit is underlined the way native marked text is, the view is start-anchored (ltr) when a tail is present so the preedit stays visible while the tail clips at the terminal edge, and the overlay is themed from options.theme instead of the stock #000/#FFF so the tail reads as ordinary terminal text. The end-of-row composition keeps the previous single-text rtl behavior byte-for-byte, and the textarea now syncs to the preedit's own bounds so IME candidate dialogs keep anchoring to the composing text rather than past the rendered tail. A TUI can repaint the row while the composition is open (spinners, streamed output), so updateCompositionElements — which already runs on every render — re-reads the row remainder and re-renders the view when it changed; a string compare keeps the per-frame cost trivial and adds no layout read. One subtlety: the tail is read with an explicit end column because the cacheable form of translateToString arms the line string cache's self-renewing idle-clear timer, and the composition path must not own stray timers — the adversarial timer-bounding test runs every captured callback synchronously and turns that self-renewal into an unbounded scheduling loop. The bundle hunks are the same edits applied to the shipped minified output; the sourcemaps are carried through unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a happy-dom/Vitest test suite for xterm IME composition rendering. The tests cover mid-line committed row tails, changing composition text, end-of-row behavior, terminal theme colors, row repaint refreshes, streamed output refreshes, and deactivation after 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/src/components/terminal-pane/terminal-ime-xterm-midline-preedit-tail.test.ts (1)
2-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the file header comment.
Keep only the non-obvious test constraint. The test names and assertions already describe the IME behavior.
As per coding guidelines, “Comments must be concise, limited to non-obvious information, and preferably one line.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 48bb66b2-63c8-4eaf-8bc7-1fcc11b68cf3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
config/patches/@xterm__xterm@6.1.0-beta.287.patchsrc/renderer/src/components/terminal-pane/terminal-ime-xterm-midline-preedit-tail.test.ts
Review found two shortcuts in the test rig: the growth test restarted a composition per preedit value instead of streaming compositionupdate events through one active composition, and the repaint tests called the private _compositionHelper.updateCompositionElements directly instead of exercising the production terminal.onRender hook. Split the compose helper into start/update, and replace the private calls with a write helper that awaits the repaint the write schedules — arming the render listener only after the parse callback, because a repaint scheduled by an earlier write can fire first and still show the old row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Closing the branch — your fix is merged, as #15014 ( It needed a port rather than a rebase: the base was 383 commits behind, main had since changed the same Two things added on top:
Your Thanks — this was the right design and it's now what ships. |
Closes #12545.
Summary
A mid-line IME composition visually swallows the character after the cursor: the preedit overlay (
.composition-view) is an opaque box anchored to the cursor cell, and since nothing reaches the PTY while composing, the covered cells still hold their characters — the box just hides them until the syllable commits. Composing가with the cursor before하in안녕하세요blanks하for the whole composition. Long-standing overlay behavior in the vendored terminal library, not a regression.The implementation follows the fix direction proposed in the issue: while a composition is open and the cursor sits mid-line, the rest of the row's committed text is rendered after the preedit inside the overlay, so the composition reads as inserted text pushing the tail right — what a native marked-text implementation (Terminal.app) shows.
What changed
All in the vendored
config/patches/@xterm__xterm@6.1.0-beta.287.patch(CompositionHelper), plus a new test suite:_renderCompositionView: when a row tail exists, the view holds an underlined preedit span (the way native marked text renders) followed by a tail span; with no tail, the previous single-textrtlform is preserved byte-for-byte.ltr) so the preedit stays visible and the pushed tail clips at the terminal edge; alone,rtlstill keeps a long preedit's end in view.options.themeinstead of the stock#000/#FFF, so the tail reads as ordinary terminal text and light themes keep contrast.updateCompositionElements— which already runs on every render — re-reads the row remainder and re-renders the view when it changed. A string compare keeps the per-frame cost trivial, and no layout read is added (the Korean/CJK IME input lag in terminal panes — installTerminalImeCandidateAnchor forces layout on every compositionupdate #12211 concern).translateToStringarms the line string cache's self-renewing idle-clear timer, and the composition path must not own stray timers — the adversarial timer-bounding suite runs every captured callback synchronously and turns that self-renewal into an unbounded scheduling loop.lib/xterm.js/lib/xterm.mjs; the sourcemaps are carried through unchanged (same convention as Show a preedit the IME resumes without a compositionstart #13284).Screenshots
Before: the character right of the cursor disappears for the whole composition — reproduced on a pre-patch build with macOS Korean 2-Set exactly as the issue describes.
After (real macOS Korean 2-Set typing on the patched dev build —
가composing, underlined, with the tail visible behind it):Plain zsh prompt:
Claude Code composer:
Testing
terminal-ime-xterm-midline-preedit-tail.test.ts(7 cases): the issue's exact repro (안녕하세요+ CUB 6 + composing가shows하세요after the preedit), tail tracking across preedit growth, end-of-row single-text behavior preserved, theming, tail refresh under a TUI repaint, tail appearing when output lands mid-composition, and overlay hide oncompositionend.pnpm vitest runover everyterminal-ime-*suite: 21 files / 284 tests pass — including the ESM+CJS adversarial composition-ownership suite and the timer-bounding case that catches the string-cache trap above.pnpm typecheckandpnpm lintpass.AI review report
onDataoutput is byte-identical, so Windows/WSL/Linux IME flows see only the overlay difference.Notes
_isComposing, transaction events, pending finalization) is deliberately untouched: this changes what the overlay shows, staying clear of the composition-ownership area recently reverted in Revert the terminal IME composition-ownership change #13282.