Skip to content

Point the real terminal cursor at the caret so IME candidate windows land in the input (#1128) - #1350

Open
KazenDev wants to merge 1 commit into
CodebuffAI:mainfrom
KazenDev:fix/cli-ime-caret
Open

KazenDev wants to merge 1 commit into
CodebuffAI:mainfrom
KazenDev:fix/cli-ime-caret

Conversation

@KazenDev

Copy link
Copy Markdown

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

File
cli/src/utils/ime-caret.ts new, 72 — pure: caret index + wrap info + screen origin → 1-based cell, or null when off-screen
cli/src/utils/__tests__/ime-caret.test.ts new, 111 — 9 tests of that geometry
cli/src/components/__tests__/multiline-input-caret.test.tsx new, 145 — 8 tests of what the component publishes
cli/src/components/multiline-input.tsx +97/-89 — publishes the caret each frame; the drawn caret is gone
cli/src/components/__tests__/multiline-input.test.tsx -162 — the tests that asserted the drawn caret
cli/src/hooks/use-clipboard.ts -2 — stopped stripping a cursor character that no longer exists
cli/src/components/input-cursor.tsx deleted

441 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. renderAfter looks like the right hook and is not

renderAfter fires 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: TextBufferRenderable overrides render() without calling it, so a handler assigned there is silently dead — measured zero dispatches while the component was rendering normally. What works is renderer.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. So input-cursor.tsx, the shouldHighlight branch and the inverted-span highlight all go. Vertical navigation still needs to know whether the caret sits on a character, so that decision survives as caretOverCharacter, 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:

  1. Run the CLI.
  2. Switch to a Chinese IME.
  3. Type nihao and watch where the composition text and candidate list land.
  4. Keep typing after enough output to scroll the transcript, and type on a second line — the cursor should track the caret in both cases.

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 (in cli/) — unchanged at its 10 pre-existing errors (missing tar types, missing react-dom/server types). No new ones.
  • Full CLI suite, same file list as 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 wrapper needs the tar module, and the env loader needs Infisical) and identical before and after.

Note for anyone re-running this: the package's bun run test script cannot start in this mirror, because its bunfig.toml preloads ../test/setup-scm-loader.ts, which is not part of the public tree. Passing the file list to bun test directly from the repository root works.

Not in this PR

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant