mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-10 09:19:42 +02:00
fix(terminal): let a composition-only overlay follow the prompt, repaint it on removeChar, document the API (#499 review)
Merge-time fixes for the three findings of the third review round of #499. - minor: a composition on an empty prompt did not follow the prompt after output or a resize. The post-write re-place in flushPendingWrites and the resize observer both ran rerender() only when hasPending was true, and hasPending deliberately excludes the composition, so the first word of a prompt (an overlay holding only a composition) stayed on the old row over whatever output moved there. Both sites now call rerender() unconditionally; it already returns early when there is nothing to draw, so nothing changes without a composition. New browser case drives the real batchTerminalWrite/flushPendingWrites path against real xterm 6 and the overlay built from source, moves the prompt from row 0 to row 3 and checks the overlay follows (it fails on the old guard, overlay left on row 0), with a parity case for pending text. The structure test pins the post-write site through vm and the resize site, which is a closure inside initTerminal(), by source. - nit: removeChar() dropped the composition but did not repaint on its false path, leaving a composition-only overlay on screen showing text the addon no longer held. It now hides the overlay there when a composition was dropped. Package tests cover that path and the flushed path repainting without the tail. - nit: the package README did not document setComposition() or the composition getter and described hasPending as "any content". Added both to the API tables plus a short IME composition section, reworded hasPending (pending or flushed text, excludes the composition), and made the quick start re-render unconditionally instead of teaching the hasPending guard. The hasPending JSDoc says the same. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -106,10 +106,10 @@ terminal.onData((data) => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// 3. Re-render after terminal output (for full-screen TUI frameworks like Ink)
|
// 3. Re-render after terminal output (for full-screen TUI frameworks like Ink).
|
||||||
terminal.onWriteParsed(() => {
|
// Unconditional: rerender() is a no-op when there is nothing to draw, and
|
||||||
if (zerolag.hasPending) zerolag.rerender();
|
// hasPending would miss an overlay that shows only an IME composition.
|
||||||
});
|
terminal.onWriteParsed(() => zerolag.rerender());
|
||||||
```
|
```
|
||||||
|
|
||||||
That is the whole integration. Everything below is for tuning it.
|
That is the whole integration. Everything below is for tuning it.
|
||||||
@@ -186,7 +186,7 @@ If one terminal hosts several CLIs with different prompts, swap the strategy in
|
|||||||
zerolag.setPrompt({ type: 'character', char: '❯', offset: 2 });
|
zerolag.setPrompt({ type: 'character', char: '❯', offset: 2 });
|
||||||
```
|
```
|
||||||
|
|
||||||
`setPrompt()` clears the cached prompt position and re-renders if anything is pending, so a mode switch cannot leave the overlay pinned to the old column.
|
`setPrompt()` clears the cached prompt position and re-renders if the overlay has anything to draw, so a mode switch cannot leave the overlay pinned to the old column.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -202,8 +202,9 @@ Implements the xterm.js `ITerminalAddon` interface. It deliberately does **not**
|
|||||||
|--------|---------|-------------|
|
|--------|---------|-------------|
|
||||||
| `addChar(char)` | `void` | Add a single printable character. Auto-detects existing buffer text on the first keystroke. |
|
| `addChar(char)` | `void` | Add a single printable character. Auto-detects existing buffer text on the first keystroke. |
|
||||||
| `appendText(text)` | `void` | Append multiple characters (paste). |
|
| `appendText(text)` | `void` | Append multiple characters (paste). |
|
||||||
| `removeChar()` | `'pending'` \| `'flushed'` \| `false` | Remove the last character. See [backspace handling](#backspace-handling). |
|
| `removeChar()` | `'pending'` \| `'flushed'` \| `false` | Remove the last character and drop any IME composition. See [backspace handling](#backspace-handling). |
|
||||||
| `clear()` | `void` | Clear all state and hide the overlay. Call on Enter, Ctrl+C, Escape. |
|
| `clear()` | `void` | Clear all state, the composition included, and hide the overlay. Call on Enter, Ctrl+C, Escape. |
|
||||||
|
| `setComposition(text)` | `void` | Show text an IME is still composing as an underlined tail after the typed text. Pass `''` to remove it. See [IME composition](#ime-composition). |
|
||||||
|
|
||||||
### Backspace handling
|
### Backspace handling
|
||||||
|
|
||||||
@@ -217,6 +218,19 @@ Implements the xterm.js `ITerminalAddon` interface. It deliberately does **not**
|
|||||||
|
|
||||||
The cascade order is pending text, then flushed text, then auto-detected buffer text (which is what makes backspace work after tab completion). Backspace "just works" across any combination of typed, in-flight and completed text.
|
The cascade order is pending text, then flushed text, then auto-detected buffer text (which is what makes backspace work after tab completion). Backspace "just works" across any combination of typed, in-flight and completed text.
|
||||||
|
|
||||||
|
### IME composition
|
||||||
|
|
||||||
|
While an input method (Japanese kana, Chinese pinyin, Korean) is still composing, the text is not committed yet, so it is not in `pendingText` either. `setComposition(text)` draws it as an underlined, `aria-hidden` tail right after the pending and flushed text, using the same wrapping and on-screen layout as the rest of the overlay.
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
const textarea = terminal.textarea!;
|
||||||
|
textarea.addEventListener('compositionupdate', (e) => zerolag.setComposition(e.data));
|
||||||
|
textarea.addEventListener('compositionend', () => zerolag.setComposition(''));
|
||||||
|
// xterm then emits the committed text through onData: add it with addChar()/appendText() as usual.
|
||||||
|
```
|
||||||
|
|
||||||
|
The composition is visual only: it is never part of `pendingText`, `hasPending` or `state`, so it can never be sent. Control characters and line breaks are stripped from it. `clear()` and `removeChar()` drop it. Because `hasPending` excludes it, re-place the overlay after output or a resize with an unconditional `rerender()`, not one gated on `hasPending`.
|
||||||
|
|
||||||
### Flushed text
|
### Flushed text
|
||||||
|
|
||||||
"Flushed" means sent to the PTY but the echo has not arrived yet. This happens during tab switches and tab completion.
|
"Flushed" means sent to the PTY but the echo has not arrived yet. This happens during tab switches and tab completion.
|
||||||
@@ -242,7 +256,7 @@ Finds text that exists after the prompt but was never typed through the overlay.
|
|||||||
|
|
||||||
| Method | Description |
|
| Method | Description |
|
||||||
|--------|-------------|
|
|--------|-------------|
|
||||||
| `rerender()` | Force a re-render. Call after buffer reloads, screen redraws, resizes and reconnects. |
|
| `rerender()` | Force a re-render. Call after buffer reloads, screen redraws, resizes and reconnects. A no-op when there is nothing to draw, so it needs no guard. |
|
||||||
| `refreshFont()` | Re-cache font and color properties from the terminal. Call after a font size or theme change. |
|
| `refreshFont()` | Re-cache font and color properties from the terminal. Call after a font size or theme change. |
|
||||||
|
|
||||||
### Prompt
|
### Prompt
|
||||||
@@ -258,7 +272,8 @@ Finds text that exists after the prompt but was never typed through the overlay.
|
|||||||
| Property | Type | Description |
|
| Property | Type | Description |
|
||||||
|----------|------|-------------|
|
|----------|------|-------------|
|
||||||
| `pendingText` | `string` | Unacknowledged text (read-only) |
|
| `pendingText` | `string` | Unacknowledged text (read-only) |
|
||||||
| `hasPending` | `boolean` | `true` if the overlay has any content |
|
| `hasPending` | `boolean` | `true` if there is pending or flushed text. Excludes the IME composition, so it can be `false` while the overlay still shows one |
|
||||||
|
| `composition` | `string` | The text set by `setComposition()`, `''` when none (read-only) |
|
||||||
| `state` | `ZerolagInputState` | Full snapshot: `pendingText`, `flushedLength`, `flushedText`, `visible`, `promptPosition` |
|
| `state` | `ZerolagInputState` | Full snapshot: `pendingText`, `flushedLength`, `flushedText`, `visible`, `promptPosition` |
|
||||||
|
|
||||||
### Options
|
### Options
|
||||||
|
|||||||
@@ -208,9 +208,13 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
* - `'flushed'`: A character was removed from text already sent to the PTY.
|
* - `'flushed'`: A character was removed from text already sent to the PTY.
|
||||||
* The consumer SHOULD send backspace to the PTY.
|
* The consumer SHOULD send backspace to the PTY.
|
||||||
* - `false`: Nothing to remove. The consumer should NOT send backspace.
|
* - `false`: Nothing to remove. The consumer should NOT send backspace.
|
||||||
|
*
|
||||||
|
* Any IME composition is dropped in every case, and the overlay is repainted
|
||||||
|
* without it (hidden when nothing else is left).
|
||||||
*/
|
*/
|
||||||
removeChar(): 'pending' | 'flushed' | false {
|
removeChar(): 'pending' | 'flushed' | false {
|
||||||
// A backspace that reaches the overlay means no composition is open.
|
// A backspace that reaches the overlay means no composition is open.
|
||||||
|
const droppedComposition = this._composition.length > 0;
|
||||||
this._composition = '';
|
this._composition = '';
|
||||||
if (this._pendingText.length > 0) {
|
if (this._pendingText.length > 0) {
|
||||||
this._pendingText = this._pendingText.slice(0, -1);
|
this._pendingText = this._pendingText.slice(0, -1);
|
||||||
@@ -247,6 +251,9 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
return 'flushed';
|
return 'flushed';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Nothing to remove, but a composition-only overlay is still on screen
|
||||||
|
// drawing the text dropped above.
|
||||||
|
if (droppedComposition) this._hide();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -460,7 +467,13 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
return this._pendingText;
|
return this._pendingText;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Whether there is any overlay content (pending or flushed). */
|
/**
|
||||||
|
* Whether there is pending or flushed text. Excludes the IME composition,
|
||||||
|
* which is never sent, so an overlay showing only a composition reports
|
||||||
|
* `false` while still on screen. To re-place the overlay after output or a
|
||||||
|
* resize, call `rerender()` unconditionally: it is a no-op when there is
|
||||||
|
* nothing to draw.
|
||||||
|
*/
|
||||||
get hasPending(): boolean {
|
get hasPending(): boolean {
|
||||||
return this._pendingText.length > 0 || this._flushedOffset > 0;
|
return this._pendingText.length > 0 || this._flushedOffset > 0;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -169,6 +169,26 @@ describe('setComposition', () => {
|
|||||||
expect(lineText(lineDivs(overlay)[0])).toBe('ab');
|
expect(lineText(lineDivs(overlay)[0])).toBe('ab');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('removeChar() with nothing to remove still takes a composition-only overlay off screen', () => {
|
||||||
|
const { addon, overlay } = setup();
|
||||||
|
addon.setComposition('ka');
|
||||||
|
expect(compositionText(overlay)).toBe('ka');
|
||||||
|
expect(addon.removeChar()).toBe(false);
|
||||||
|
expect(addon.composition).toBe('');
|
||||||
|
expect(compositionText(overlay)).toBe('');
|
||||||
|
expect(overlay.style.display).toBe('none');
|
||||||
|
expect(addon.state.visible).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('removeChar() repaints flushed text without the dropped composition', () => {
|
||||||
|
const { addon, overlay } = setup();
|
||||||
|
addon.setFlushed(3, 'abc');
|
||||||
|
addon.setComposition('xy');
|
||||||
|
expect(addon.removeChar()).toBe('flushed');
|
||||||
|
expect(compositionText(overlay)).toBe('');
|
||||||
|
expect(lineText(lineDivs(overlay)[0])).toBe('ab');
|
||||||
|
});
|
||||||
|
|
||||||
it('text appended while composing lands before the tail', () => {
|
it('text appended while composing lands before the tail', () => {
|
||||||
const { addon, overlay } = setup();
|
const { addon, overlay } = setup();
|
||||||
addon.appendText('ab');
|
addon.appendText('ab');
|
||||||
|
|||||||
@@ -1407,9 +1407,10 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
// has to re-resolve their gate before the redraw, not just move them.
|
// has to re-resolve their gate before the redraw, not just move them.
|
||||||
this.applyLineageLineSettings?.();
|
this.applyLineageLineSettings?.();
|
||||||
this.updateConnectionLines();
|
this.updateConnectionLines();
|
||||||
if (this._localEchoOverlay?.hasPending) {
|
// Unguarded on purpose: hasPending excludes an IME composition, so a
|
||||||
this._localEchoOverlay.rerender();
|
// composition-only overlay would stay on the old prompt row. rerender()
|
||||||
}
|
// is a no-op when the overlay has nothing to draw.
|
||||||
|
this._localEchoOverlay?.rerender();
|
||||||
// Pane B (split view) has its own container and its own fit()/resize
|
// Pane B (split view) has its own container and its own fit()/resize
|
||||||
// frame — this observer only ever measured Pane A's container, so
|
// frame — this observer only ever measured Pane A's container, so
|
||||||
// without this call Pane B never learned about a window resize, an
|
// without this call Pane B never learned about a window resize, an
|
||||||
@@ -4161,9 +4162,10 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
|
|
||||||
// Re-position local echo overlay after terminal writes — Ink redraws can
|
// Re-position local echo overlay after terminal writes — Ink redraws can
|
||||||
// move the ❯ prompt to a different row, making the overlay invisible.
|
// move the ❯ prompt to a different row, making the overlay invisible.
|
||||||
if (this._localEchoOverlay?.hasPending) {
|
// Unguarded on purpose: hasPending excludes an IME composition, so a
|
||||||
this._localEchoOverlay.rerender();
|
// composition-only overlay (the first word of a prompt) would otherwise
|
||||||
}
|
// stay on the old row. rerender() is a no-op when there is nothing to draw.
|
||||||
|
this._localEchoOverlay?.rerender();
|
||||||
|
|
||||||
// After Tab completion: detect the completed text in the overlay.
|
// After Tab completion: detect the completed text in the overlay.
|
||||||
// Use terminal.write('', callback) to defer detection until xterm.js
|
// Use terminal.write('', callback) to defer detection until xterm.js
|
||||||
|
|||||||
@@ -690,6 +690,25 @@ describe('mobile IME commit and authoritative terminal output', () => {
|
|||||||
expect(controller.noteAuthoritativeOutput).not.toHaveBeenCalled();
|
expect(controller.noteAuthoritativeOutput).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('re-places an overlay that holds only a composition after output (hasPending is false there)', () => {
|
||||||
|
const { app, flush } = outputHarness();
|
||||||
|
const overlay = { hasPending: false, pendingText: '', composition: '今日', rerender: vi.fn() };
|
||||||
|
app._localEchoOverlay = overlay;
|
||||||
|
app.batchTerminalWrite('output that moves the prompt');
|
||||||
|
flush();
|
||||||
|
expect(overlay.rerender).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('re-places it after a resize too: no rerender() site is gated on hasPending', () => {
|
||||||
|
// The resize observer is a closure inside initTerminal(), so it is pinned
|
||||||
|
// by source; the post-write follow runs against real xterm in
|
||||||
|
// test/mobile-ime-preview.browser.test.ts.
|
||||||
|
expect(terminalSource).toMatch(
|
||||||
|
/this\.updateConnectionLines\(\);\s*(?:\/\/[^\n]*\n\s*)*this\._localEchoOverlay\?\.rerender\(\);/
|
||||||
|
);
|
||||||
|
expect(terminalSource).not.toMatch(/hasPending\)\s*\{?\s*this\._localEchoOverlay\.rerender\(\)/);
|
||||||
|
});
|
||||||
|
|
||||||
it('notifies once per commit, never for later output', () => {
|
it('notifies once per commit, never for later output', () => {
|
||||||
const { app, controller, flush, parseNext } = outputHarness();
|
const { app, controller, flush, parseNext } = outputHarness();
|
||||||
app._consumeMobileImeTerminalData('你好');
|
app._consumeMobileImeTerminalData('你好');
|
||||||
|
|||||||
@@ -289,4 +289,104 @@ describe('mobile IME preview over the local echo overlay', () => {
|
|||||||
const result = await composeAfter('今日は', '天気', true);
|
const result = await composeAfter('今日は', '天気', true);
|
||||||
expect(result.afterCommit).toEqual({ pendingText: '今日は天気', compositionSpans: 0, overlayText: '今日は天気' });
|
expect(result.afterCommit).toEqual({ pendingText: '今日は天気', compositionSpans: 0, overlayText: '今日は天気' });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Composes `composing` after `pending`, then streams output through the REAL
|
||||||
|
* write path (batchTerminalWrite, the scheduled flushPendingWrites, xterm's
|
||||||
|
* async parse) that moves the ❯ row from 0 to 3, then one more frame that
|
||||||
|
* leaves the prompt where it is (a status-line repaint). Reports the overlay
|
||||||
|
* row after each frame.
|
||||||
|
*
|
||||||
|
* The post-write re-place runs right after terminal.write() returns, before
|
||||||
|
* xterm parses that chunk, so it sees the buffer as of the previous frame: the
|
||||||
|
* overlay reaches the new row on the frame after the move. That timing is the
|
||||||
|
* same for pending text; the composition-only overlay used to never get there
|
||||||
|
* because the re-place was gated on hasPending, which excludes it.
|
||||||
|
*/
|
||||||
|
async function composeThenMovePrompt(pending: string, composing: string) {
|
||||||
|
return page.evaluate(
|
||||||
|
async ({ pending, composing }) => {
|
||||||
|
const w = window as any;
|
||||||
|
const host = document.getElementById('t') as HTMLElement;
|
||||||
|
host.innerHTML = '';
|
||||||
|
const term = new w.Terminal({
|
||||||
|
cols: 40,
|
||||||
|
rows: 8,
|
||||||
|
fontSize: 14,
|
||||||
|
fontFamily: 'monospace',
|
||||||
|
allowProposedApi: true,
|
||||||
|
});
|
||||||
|
term.open(host);
|
||||||
|
await new Promise<void>((r) => term.write('❯ ', () => r()));
|
||||||
|
const app = new w.CodemanApp();
|
||||||
|
Object.assign(app, {
|
||||||
|
terminal: term,
|
||||||
|
_localEchoEnabled: true,
|
||||||
|
_localEchoOverlay: new w.LocalEchoOverlay(term),
|
||||||
|
pendingWrites: [],
|
||||||
|
activeSessionId: 'session-a',
|
||||||
|
sessions: new Map([['session-a', { mode: 'claude' }]]),
|
||||||
|
});
|
||||||
|
w.MobileImePreview.isIosWebKitTouch = () => true;
|
||||||
|
app._initMobileImePreview();
|
||||||
|
|
||||||
|
if (pending) app._localEchoOverlay.appendText(pending);
|
||||||
|
const textarea = term.textarea as HTMLTextAreaElement;
|
||||||
|
textarea.focus();
|
||||||
|
textarea.dispatchEvent(new CompositionEvent('compositionstart', { data: '' }));
|
||||||
|
textarea.value = composing;
|
||||||
|
textarea.dispatchEvent(new CompositionEvent('compositionupdate', { data: composing }));
|
||||||
|
await new Promise((r) => requestAnimationFrame(() => setTimeout(r, 20)));
|
||||||
|
|
||||||
|
const cellH = term._core._renderService.dimensions.css.cell.height;
|
||||||
|
const overlayEl = app._localEchoOverlay._overlay as HTMLElement;
|
||||||
|
const overlayRow = () =>
|
||||||
|
overlayEl.style.display === 'none' ? null : Math.round(parseFloat(overlayEl.style.top) / cellH);
|
||||||
|
// Output goes through the app's own scheduler; wait until it has been
|
||||||
|
// flushed and parsed.
|
||||||
|
const stream = async (data: string) => {
|
||||||
|
app.batchTerminalWrite(data);
|
||||||
|
for (let i = 0; i < 200; i++) {
|
||||||
|
if (!app.writeFrameScheduled && !app._terminalWriteInFlight && app.pendingWrites.length === 0) break;
|
||||||
|
await new Promise((r) => setTimeout(r, 10));
|
||||||
|
}
|
||||||
|
await new Promise<void>((r) => term.write('', () => r()));
|
||||||
|
};
|
||||||
|
|
||||||
|
const before = overlayRow();
|
||||||
|
await stream('\r\x1b[2Kline 1\r\nline 2\r\nline 3\r\n❯ ');
|
||||||
|
const promptRow = app._localEchoOverlay.findPrompt()?.row ?? null;
|
||||||
|
await stream('\x1b7\x1b[8;1Hworking\x1b8');
|
||||||
|
const result = {
|
||||||
|
before,
|
||||||
|
promptRow,
|
||||||
|
after: overlayRow(),
|
||||||
|
composition: Array.from(term.element.querySelectorAll('[data-zerolag-composition]'))
|
||||||
|
.map((el) => (el as HTMLElement).textContent)
|
||||||
|
.join(''),
|
||||||
|
hasPending: app._localEchoOverlay.hasPending,
|
||||||
|
};
|
||||||
|
app._destroyMobileImePreview();
|
||||||
|
app._localEchoOverlay.dispose();
|
||||||
|
term.dispose();
|
||||||
|
return result;
|
||||||
|
},
|
||||||
|
{ pending, composing }
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
it('a composition on an empty prompt follows the prompt when output moves it', async () => {
|
||||||
|
const result = await composeThenMovePrompt('', '今日');
|
||||||
|
expect(result.before).toBe(0);
|
||||||
|
expect(result.promptRow).toBe(3);
|
||||||
|
// Nothing is pending: before the fix this stayed on row 0, over "line 1".
|
||||||
|
expect(result.hasPending).toBe(false);
|
||||||
|
expect(result.after).toBe(3);
|
||||||
|
expect(result.composition).toBe('今日');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a composition after pending text follows it the same way', async () => {
|
||||||
|
const result = await composeThenMovePrompt('abc', '今日');
|
||||||
|
expect(result).toEqual({ before: 0, promptRow: 3, after: 3, composition: '今日', hasPending: true });
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user