Skip to content

Commit c1b7ea8

Browse files
ibleajacob314
andauthored
Fix incorrect IME input window positioning when "multiline input + specific conditions" occur (#45)
* Code review fixes. More IME fixes Checkpoint. Cursor simplifications Remove enableImeCursor and move the cursor to the appropriate location without making the cursor be visible. Gemini CLI will continue to render its own cursor so there is no need to show the cursor as that introduces visual artifacts. Gemini CLI does however need the cursor position to be at the correct location so IME input works as expected. Fixes for incremental updates in non-alternate buffer mode and tests. * fix: improve cursor position calculation in wrapped text with dropped spaces - Rename currentLineStartOffset to previousLineEndOffset for clearer semantics - Add early exit to avoid unnecessary iteration - Keep cursor at end of previous line instead of jumping to next line in gap area - Add comments for code clarity * fix: handle cursor position when first line is empty * merge: master - 3aef965 --------- Co-authored-by: jacob314 <jacob314@gmail.com>
1 parent 3aef965 commit c1b7ea8

File tree

2 files changed

+119
-24
lines changed

2 files changed

+119
-24
lines changed

src/render-node-to-output.ts

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -491,33 +491,54 @@ const calculateWrappedCursorPosition = (
491491

492492
let cursorLineIndex = lines.length - 1;
493493
let relativeCursorPosition = targetOffset;
494-
let currentLineStartOffset = 0;
494+
// -1 represents "before document start" so first character (offset 0) is handled correctly
495+
let previousLineEndOffset = -1;
495496

496497
for (const [i, line] of lines.entries()) {
497498
if (line.length > 0) {
498499
const firstChar = line.find(char => styledCharToOffset.has(char));
500+
const lastChar = line.findLast(char => styledCharToOffset.has(char));
499501

500-
if (firstChar) {
501-
const firstCharOffset = styledCharToOffset.get(firstChar)!;
502-
503-
if (targetOffset >= currentLineStartOffset) {
502+
if (!firstChar || !lastChar) {
503+
// Padding-only line (originally empty), treat as empty line
504+
if (targetOffset > previousLineEndOffset) {
504505
cursorLineIndex = i;
505-
relativeCursorPosition = Math.max(0, targetOffset - firstCharOffset);
506+
relativeCursorPosition = targetOffset - previousLineEndOffset - 1;
507+
previousLineEndOffset++;
506508
}
507509

508-
const lastChar = line.findLast(char => styledCharToOffset.has(char));
510+
continue;
511+
}
512+
513+
const lineStartOffset = styledCharToOffset.get(firstChar)!;
514+
const lineEndOffset =
515+
styledCharToOffset.get(lastChar)! + lastChar.value.length;
509516

510-
if (lastChar) {
511-
currentLineStartOffset =
512-
styledCharToOffset.get(lastChar)! + lastChar.value.length;
513-
}
517+
// Set as candidate if targetOffset is at or after line start
518+
if (targetOffset >= lineStartOffset) {
519+
cursorLineIndex = i;
520+
relativeCursorPosition = Math.max(0, targetOffset - lineStartOffset);
514521
}
515-
} else if (i > 0 && targetOffset >= currentLineStartOffset) {
522+
523+
// Finalize and exit if targetOffset is within or before this line's range.
524+
// If targetOffset is in a gap (between previousLineEndOffset and lineStartOffset),
525+
// the cursor stays at the previous line's end (already set in previous iteration).
526+
if (targetOffset <= lineEndOffset) {
527+
break;
528+
}
529+
530+
previousLineEndOffset = lineEndOffset;
531+
} else if (i === 0 && targetOffset === 0) {
532+
// Edge case: First line is empty and cursor is at position 0
533+
cursorLineIndex = 0;
534+
relativeCursorPosition = 0;
535+
break;
536+
} else if (i > 0 && targetOffset > previousLineEndOffset) {
516537
// Handle empty lines (usually caused by \n)
517538
cursorLineIndex = i;
518-
relativeCursorPosition = targetOffset - currentLineStartOffset;
519-
// Advance currentLineStartOffset past the \n
520-
currentLineStartOffset++;
539+
relativeCursorPosition = targetOffset - previousLineEndOffset - 1;
540+
// Advance past the \n character
541+
previousLineEndOffset++;
521542
}
522543
}
523544

test/terminal-cursor.tsx

Lines changed: 83 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -370,16 +370,13 @@ test('renderer returns correct cursorPosition for wrapped Text with dropped spac
370370
root.yogaNode?.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
371371

372372
const result = renderer(root, false);
373-
// Currently, if it's in the "gap" created by dropping spaces, it will fall through
374-
// and default to the last line's beginning if not caught.
373+
// When cursor is in the "gap" created by dropping spaces during wrapping,
374+
// it should stay at the end of the previous line rather than jumping to next line.
375375
// In "Hello World" with width 5, lines are ["Hello", "World"].
376-
// TargetOffset = 6.
377-
// Line 0: offset 0, len 5. 6 >= 0 but 6 < 5 is false.
378-
// Synchronization: next line starts with "W" which is at original index 8.
379-
// CurrentOriginalOffset becomes 8.
380-
// Line 1: offset 8, len 5. 6 >= 8 is false.
381-
// Not found. Defaults to last line (1), relative pos 0.
382-
t.deepEqual(result.cursorPosition, {row: 1, col: 0});
376+
// TargetOffset = 6 (in dropped spaces area).
377+
// Line 0: [0, 4], Line 1: [8, 12]
378+
// Since 6 > 4 (line 0 end) but 6 < 8 (line 1 start), cursor stays at line 0 end.
379+
t.deepEqual(result.cursorPosition, {row: 0, col: 5});
383380
});
384381

385382
test('renderer returns correct cursorPosition for wrapped Text with dropped spaces - after gap', t => {
@@ -406,6 +403,83 @@ test('renderer returns correct cursorPosition for wrapped Text with dropped spac
406403
t.deepEqual(result.cursorPosition, {row: 1, col: 1});
407404
});
408405

406+
test('renderer returns correct cursorPosition when first line is empty', t => {
407+
const root = dom.createNode('ink-root');
408+
root.yogaNode?.setWidth(100);
409+
410+
const textNode = dom.createNode('ink-text');
411+
textNode.internal_terminalCursorFocus = true;
412+
textNode.internal_terminalCursorPosition = 0; // Cursor at the very beginning
413+
414+
// First line is empty (text starts with \n)
415+
const text = dom.createTextNode('\n한글\n문제가');
416+
dom.appendChildNode(textNode, text as unknown as dom.DOMElement);
417+
dom.appendChildNode(root, textNode);
418+
419+
root.yogaNode?.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
420+
421+
const result = renderer(root, false);
422+
// Cursor should be on the first (empty) line, not the last line
423+
t.deepEqual(result.cursorPosition, {row: 0, col: 0});
424+
});
425+
426+
test('renderer returns correct cursorPosition when first line is empty - cursor on second line', t => {
427+
const root = dom.createNode('ink-root');
428+
root.yogaNode?.setWidth(100);
429+
430+
const textNode = dom.createNode('ink-text');
431+
textNode.internal_terminalCursorFocus = true;
432+
textNode.internal_terminalCursorPosition = 1; // After the first \n, start of "한글"
433+
434+
const text = dom.createTextNode('\n한글\n문제가');
435+
dom.appendChildNode(textNode, text as unknown as dom.DOMElement);
436+
dom.appendChildNode(root, textNode);
437+
438+
root.yogaNode?.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
439+
440+
const result = renderer(root, false);
441+
// Cursor should be at the start of the second line
442+
t.deepEqual(result.cursorPosition, {row: 1, col: 0});
443+
});
444+
445+
test('renderer handles multiple leading newlines', t => {
446+
const root = dom.createNode('ink-root');
447+
root.yogaNode?.setWidth(100);
448+
449+
const textNode = dom.createNode('ink-text');
450+
textNode.internal_terminalCursorFocus = true;
451+
textNode.internal_terminalCursorPosition = 0;
452+
453+
// Multiple empty lines at start
454+
const text = dom.createTextNode('\n\n한글');
455+
dom.appendChildNode(textNode, text as unknown as dom.DOMElement);
456+
dom.appendChildNode(root, textNode);
457+
458+
root.yogaNode?.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
459+
460+
const result = renderer(root, false);
461+
t.deepEqual(result.cursorPosition, {row: 0, col: 0});
462+
});
463+
464+
test('renderer handles cursor in middle of second line after empty first', t => {
465+
const root = dom.createNode('ink-root');
466+
root.yogaNode?.setWidth(100);
467+
468+
const textNode = dom.createNode('ink-text');
469+
textNode.internal_terminalCursorFocus = true;
470+
textNode.internal_terminalCursorPosition = 2; // After '\n' and '한'
471+
472+
const text = dom.createTextNode('\n한글');
473+
dom.appendChildNode(textNode, text as unknown as dom.DOMElement);
474+
dom.appendChildNode(root, textNode);
475+
476+
root.yogaNode?.calculateLayout(undefined, undefined, Yoga.DIRECTION_LTR);
477+
478+
const result = renderer(root, false);
479+
// '한' is a wide character (width 2), so cursor after '한' is at col 2
480+
t.deepEqual(result.cursorPosition, {row: 1, col: 2});
481+
});
482+
409483
// =============================================================================
410484
// Integration tests with Text component
411485
// =============================================================================

0 commit comments

Comments
 (0)