Conversation
…d right Terminals anchor an IME candidate window (Windows Terminal via ConPTY, macOS, ibus/fcitx) to their *real* cursor. This input drew its own `▍` glyph and never moved that cursor, so Chinese/Japanese/Korean popups landed wherever the last frame happened to write text, and Persian/RTL inherited the same misplacement. The caret is the terminal's now: `caretCell` (pure, in utils/ime-caret.ts) turns the caret index into a 1-based screen cell — measured in cells, so a CJK glyph counts two columns, and offset by the text renderable's own screen origin so a scrolled input still resolves. The component publishes it every frame through `renderer.setFrameCallback`, which also fixes a misplaced cursor after a scroll or resize without waiting for a re-render. `renderAfter` was the obvious hook and is not usable here: the text renderable overrides `render()` without calling it. The fake caret goes away with it. Leaving both would show two cursors, and the drawn one could only ever go stale. That retires input-cursor.tsx and the `shouldHighlight` branch with its inverted-span highlight. Vertical navigation still needs to know whether the caret sits *on* a character, so that decision survives as `caretOverCharacter`. The clipboard hook stops stripping a cursor character that no longer exists. If the caret's line is scrolled out of view, or the frame still holds the previous layout, the cursor is left alone or hidden rather than parked outside the input box. Verified: 17 new tests across two files (the helper's geometry and the component's published position), 9 obsolete ones removed. The CLI typecheck is unchanged at its 10 pre-existing errors, and the full suite matches baseline (3090 -> 3098 pass, same 20 pre-existing failures). Refs CodebuffAI#1128
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes Option A from #1128 — the minimal one — and adds the regression tests the report asked for. The real terminal cursor now sits on the caret, so a CJK IME anchors its composition string and candidate window inside the input box instead of wherever the frame last wrote text.
Why Option A and not Option B
Option B (migrate to OpenTUI's
<textarea>/EditBufferRenderable) is the cleaner end state — it is what opencode uses — but it means replacing the hand-rolled editor: word wrap, word-wise navigation, tab expansion, paste handling, the queue-edit integration and the keyboard table all live in this component, and their tests are written against it. Positioning the cursor is the actual bug. Moving the editor is a separate change with a much larger blast radius, and it can be done later on a codebase that no longer has a parked cursor.What this changes
cli/src/utils/ime-caret.tsnullwhen off-screencli/src/utils/__tests__/ime-caret.test.tscli/src/components/__tests__/multiline-input-caret.test.tsxcli/src/components/multiline-input.tsxcli/src/components/__tests__/multiline-input.test.tsxcli/src/hooks/use-clipboard.tscli/src/components/input-cursor.tsx441 insertions, 317 deletions across 7 files.
Three things that are easy to get wrong here
All three were hit while implementing, and each is why the code looks the way it does.
1.
renderAfterlooks like the right hook and is notrenderAfterfires after layout, which is exactly the timing this needs, and the React layer does forward it as a prop. It never runs on the input:TextBufferRenderableoverridesrender()without calling it, so a handler assigned there is silently dead — measured zero dispatches while the component was rendering normally. What works isrenderer.setFrameCallback, which the codebase already uses elsewhere, and it has a side benefit: the cursor also follows a scroll or a resize, without waiting for a React re-render.2. The first callback of a frame still sees the previous layout
The frame callback runs twice per frame: once with the layout of the previous frame (input box still 0 rows tall) and once with the current one. Publishing unconditionally makes the cursor blink off and back on every frame. When the geometry is not ready yet the cursor is now left alone.
3. Geometry is in cells, and one render-time value is a layout behind
A CJK glyph is two cells wide, so the column has to be measured with
string-width, never by string index. The wrap info (lineInfo.lineStartCols) read during render belongs to the previous layout — with two lines, every multiline caret would resolve onto line 0. The helper reads it at frame time instead, which is what the input's own keyboard handlers already do.Why the drawn caret goes away
The fake
▍and the new real cursor cannot coexist — that is two cursors on screen — and the drawn one could only ever go stale, which is the bug. Soinput-cursor.tsx, theshouldHighlightbranch and the inverted-span highlight all go. Vertical navigation still needs to know whether the caret sits on a character, so that decision survives ascaretOverCharacter, with the same expression as before.The cursor is hidden when the input is not focused. The report notes that OpenTUI's own
renderCursor()early-returns when unfocused, and since this fix does not hand focus to an inner textarea, that hide has to be explicit.How to try it
The report's own repro, on Windows Terminal + Microsoft Pinyin:
nihaoand watch where the composition text and candidate list land.Verified
bun test cli/src/utils/__tests__/ime-caret.test.ts cli/src/components/__tests__/multiline-input-caret.test.tsx— 17 pass. The component tests read the renderer's published cursor state after a frame, so they assert the position the terminal actually gets.bun run typecheck(incli/) — unchanged at its 10 pre-existing errors (missingtartypes, missingreact-dom/servertypes). No new ones.bun run test(200 files, 3130 tests): 3090 → 3098 pass, same 20 pre-existing failures. The 8 are the net of the 17 tests added and the 9 obsolete ones removed; the 20 failures are environmental (release wrapperneeds thetarmodule, and the env loader needs Infisical) and identical before and after.Note for anyone re-running this: the package's
bun run testscript cannot start in this mirror, because itsbunfig.tomlpreloads../test/setup-scm-loader.ts, which is not part of the public tree. Passing the file list tobun testdirectly from the repository root works.Not in this PR
<textarea>— see above.shouldBlinkCursorpreference, which is forwarded throughsetCursorStyle. The drawn caret had its own blink timing and alimefallback for terminals without truecolor; neither is needed now that the terminal draws it.