diff --git a/src/features/terminal/imeCommit.test.ts b/src/features/terminal/imeCommit.test.ts index 76753353b..a6fd994c7 100644 --- a/src/features/terminal/imeCommit.test.ts +++ b/src/features/terminal/imeCommit.test.ts @@ -31,6 +31,12 @@ function gateWithRebase() { return { gate, run, pending, rebases: () => rebased }; } +/** Collects the emissions that reach the PTY, like `Session.fromTerminal`. */ +function emitter(gate: ImeCommitGate) { + const sent: string[] = []; + return { sent, emit: (data: string) => gate.accept() && sent.push(data) }; +} + describe("ImeCommitGate", () => { it("takes the first emission of a commit and drops the repeated one", () => { const { gate } = gateWithRebase(); @@ -42,10 +48,7 @@ describe("ImeCommitGate", () => { it("keeps the real text of the pi case and drops the stale suffix", () => { // xterm sends the committed diff first, then a stale `value.substring(start)`. const { gate, run } = gateWithRebase(); - const sent: string[] = []; - const emit = (data: string) => { - if (gate.accept()) sent.push(data); - }; + const { sent, emit } = emitter(gate); gate.begin(); emit("在吗"); gate.finish(); @@ -54,37 +57,101 @@ describe("ImeCommitGate", () => { expect(sent).toEqual(["在吗"]); }); - it("accepts every emission once the commit is over", () => { - const { gate, run } = gateWithRebase(); + it("lets a key the IME did not take through right after a commit", () => { + // Punctuation committed directly, then Enter within the commit's timeout: Enter must run. + const { gate, rebases } = gateWithRebase(); + const { sent, emit } = emitter(gate); + gate.begin(); + emit(","); + gate.keystroke(); + emit("\r"); + expect(sent).toEqual([",", "\r"]); + expect(rebases()).toBe(1); + // Outside a commit, or during a composition, a keystroke changes nothing. + gate.keystroke(); + gate.compositionStart(); + gate.keystroke(); + expect(rebases()).toBe(1); + }); + + it("starts a new commit on the next IME keystroke even inside the timeout", () => { + // Two commits can arrive back to back; the second must not be swallowed by the first + // commit's still-running timeout. + const { gate } = gateWithRebase(); + gate.begin(); + expect(gate.accept()).toBe(true); gate.begin(); expect(gate.accept()).toBe(true); expect(gate.accept()).toBe(false); + }); + + it("drops the stale suffix of a WebKitGTK punctuation commit without compositionstart", () => { + // fcitx5 commits `,` as insertFromComposition + compositionend without compositionstart: + // xterm sends the keydown diff `,`, then compositionend emits `好,` from the previous + // composition's position. That suffix is what showed up as a duplicate in the terminal. + const { gate, run, rebases } = gateWithRebase(); + const { sent, emit } = emitter(gate); + gate.begin(); + emit(","); + gate.finish(); + emit("好,"); + run(); + expect(sent).toEqual([","]); + expect(rebases()).toBe(1); + }); + + it("stands down for a real composition, then gates the next commit again", () => { + // compositionstart must not disable the gate for the whole session: the very next + // punctuation commit still needs deduplication. + const { gate, run, rebases } = gateWithRebase(); + gate.begin(); + gate.compositionStart(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(true); gate.finish(); run(); + expect(rebases()).toBe(1); + + gate.begin(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(false); + }); + + it("accepts every emission while composing", () => { + const { gate } = gateWithRebase(); + gate.compositionStart(); + expect(gate.accept()).toBe(true); expect(gate.accept()).toBe(true); expect(gate.accept()).toBe(true); }); - it("rebases the textarea when the commit closes, not before", () => { + it("rebases after a composition that xterm sent by itself", () => { const { gate, run, rebases } = gateWithRebase(); - gate.begin(); + gate.compositionStart(); gate.finish(); expect(rebases()).toBe(0); run(); expect(rebases()).toBe(1); }); - it("stands down once a real compositionstart arrives", () => { - const { gate, rebases } = gateWithRebase(); + it("rebases after a commit whose compositionend arrives, not before", () => { + const { gate, run, rebases } = gateWithRebase(); gate.begin(); - gate.compositionStart(); - expect(gate.accept()).toBe(true); expect(gate.accept()).toBe(true); + gate.finish(); expect(rebases()).toBe(0); - // And a later commit is not gated either. - gate.begin(); - expect(gate.accept()).toBe(true); + run(); + expect(rebases()).toBe(1); + }); + + it("opens a commit from a composition input when no keydown preceded it", () => { + const { gate } = gateWithRebase(); + gate.input(); expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(false); + // A second input event belongs to the same commit and must not reopen it. + gate.input(); + expect(gate.accept()).toBe(false); }); it("closes a commit whose compositionend never arrives", () => { @@ -96,4 +163,13 @@ describe("ImeCommitGate", () => { expect(rebases()).toBe(1); expect(gate.accept()).toBe(true); }); + + it("does not rebase into a composition that started before the close ran", () => { + const { gate, run, rebases } = gateWithRebase(); + gate.begin(); + gate.finish(); + gate.compositionStart(); + run(); + expect(rebases()).toBe(0); + }); }); diff --git a/src/features/terminal/imeCommit.ts b/src/features/terminal/imeCommit.ts index baeca349f..1f2c1ca79 100644 --- a/src/features/terminal/imeCommit.ts +++ b/src/features/terminal/imeCommit.ts @@ -1,14 +1,31 @@ /** * Collapses the repeated data emissions of one IME commit. * - * WebKitGTK + fcitx5 commit text without firing `compositionstart`, so xterm's `CompositionHelper` - * never advances `_compositionPosition.start` and keeps appending the committed text to its hidden - * textarea. A commit then reaches `onData` twice — once with the committed text, once with a stale - * `value.substring(start)` — and the same text shows up in the terminal twice. + * WebKitGTK commits IME text with an inconsistent mix of events, and xterm's + * `CompositionHelper` only copes with part of it: * - * One commit must send one chunk: take the first emission and drop the rest, then rebase the - * textarea so the next commit starts from an empty value. IMEs that do report `compositionstart` - * (macOS, IBus) keep xterm's own bookkeeping, so the gate stands down for them. + * - A composed word (`你好`) reports `compositionstart`, so xterm records where the + * composition starts in its hidden textarea and sends the committed text once on + * `compositionend`. + * - Text that the IME commits directly (punctuation such as `;` or `,`) reports + * `insertFromComposition` + `compositionend` **without** `compositionstart`. xterm then + * finalizes the *previous* composition's position: it sends the keydown's textarea diff + * first and, once `compositionend` runs, a stale `value.substring(start)` on top. That + * second emission is the duplicated suffix users see (typing `;你好,` sends `,` and then + * `好,`). Newer WebKitGTK delivers the same commits as `insertText` without any + * composition events, which xterm sends through its keydown path; the stale emission there + * carries the previous commit's text instead. + * + * One commit must send one chunk, so the gate lets the first emission through and drops the + * rest until the commit closes. A commit is delimited by the IME's own keystroke (code 229) + * outside a composition, or by a composition input when the platform sends no keydown. + * Commits that really compose are left to xterm: `compositionstart` stands the gate down for + * that commit only — it must not disable the gate for the session, because the very next + * punctuation commit still needs deduplication. + * + * xterm never clears its hidden textarea by itself, so every commit ends with a rebase: the + * textarea is emptied after xterm's own senders have run, keeping the next commit from + * carrying old text. */ /** How long a commit may stay open when its `compositionend` never arrives. */ @@ -27,8 +44,8 @@ const WINDOW_SCHEDULER: ImeScheduler = { export class ImeCommitGate { private open = false; private sent = false; + private composing = false; private timer: number | null = null; - private native = false; private readonly rebase: () => void; private readonly scheduler: ImeScheduler; @@ -37,21 +54,61 @@ export class ImeCommitGate { this.scheduler = scheduler; } - /** A commit starts: the IME's own keystroke (code 229) or its composition input event. */ + /** + * A commit starts: the IME's own keystroke (code 229) outside a composition. + * + * Always starts a fresh commit, even when the previous one is still inside its timeout: + * two commits can arrive back to back, and the second must not be swallowed. + */ begin(): void { - if (this.native || this.open) return; + if (this.composing) return; + this.cancelTimer(); this.open = true; this.sent = false; this.timer = this.scheduler.after(() => this.close(), IME_COMMIT_TIMEOUT_MS); } - /** The commit ended: let the emissions both of xterm's senders queued go out, then rebase. */ + /** + * A composition input arrived. Opens a commit only when no commit is open: resetting + * `sent` here would let the emissions already queued for this commit go out again. + */ + input(): void { + if (this.composing || this.open) return; + this.open = true; + this.sent = false; + this.timer = this.scheduler.after(() => this.close(), IME_COMMIT_TIMEOUT_MS); + } + + /** + * An IME composition begins. xterm's own composition handling owns this commit, so the + * gate stays out of its way — but only until the composition ends. + */ + compositionStart(): void { + this.composing = true; + this.cancelTimer(); + this.open = false; + this.sent = false; + } + + /** + * The composition — or, on WebKitGTK, a directly committed character — ended. + * Bubble phase: xterm's own `compositionend` listener ran first and queued its send, so a + * 0 ms close lands behind that emission and cannot cut the committed text off. + */ finish(): void { - if (!this.open) return; + this.composing = false; this.cancelTimer(); this.timer = this.scheduler.after(() => this.close(), 0); } + /** + * A keystroke the IME did not take (Enter, an arrow, a letter typed outside it): the commit + * is over, and what that key sends must reach the PTY. + */ + keystroke(): void { + if (this.open && !this.composing) this.close(); + } + /** Whether this emission should reach the PTY. */ accept(): boolean { if (!this.open) return true; @@ -60,19 +117,13 @@ export class ImeCommitGate { return true; } - /** The IME reports composition events, so xterm needs no help — stay out of its way. */ - compositionStart(): void { - this.native = true; - this.cancelTimer(); - this.open = false; - this.sent = false; - } - /** Ends the commit and rebases the textarea to an empty value. */ close(): void { this.cancelTimer(); - if (!this.open) return; + // A new composition started before this close ran; its own finish will rebase. + if (this.composing) return; this.open = false; + this.sent = false; this.rebase(); } diff --git a/src/features/terminal/session.ts b/src/features/terminal/session.ts index dce99c6cb..70e90bfd1 100644 --- a/src/features/terminal/session.ts +++ b/src/features/terminal/session.ts @@ -413,17 +413,18 @@ export class TerminalSession { /** * Routes a WebKitGTK / fcitx5 IME commit through `ImeCommitGate`: the platform reports the - * composition's own keystroke (code 229) and its input event but never `compositionstart`, - * which is what xterm's own bookkeeping is built on. + * commit's own keystroke (code 229) and its input event, but reports `compositionstart` only + * for composed words — never for the punctuation an IME commits directly. */ private watchImeCommit() { const textarea = this.term.textarea; if (!textarea) return; const onKeyDown = (e: KeyboardEvent) => { if (e.keyCode === 229) this.ime.begin(); + else this.ime.keystroke(); }; const onInput = (e: InputEvent) => { - if (e.inputType === "insertFromComposition" || e.inputType === "insertCompositionText") this.ime.begin(); + if (e.inputType === "insertFromComposition" || e.inputType === "insertCompositionText") this.ime.input(); }; const onCompositionStart = () => this.ime.compositionStart(); // Bubble phase, i.e. after xterm's own listener: the rebase must land behind the emissions