Reverse a confirmed autocomplete with a single undo (#34635)

Confirming a completion produces two model updates for one keystroke: the
completion replaces the range and closes the autocomplete, and then the wrapper
closes it again. The model reported that second close like any other change, and
since it carries no input type and no diff the history manager treats it as bulk
input and always pushes, leaving two identical states on the undo stack.

The model now ignores a close which has nothing to replace and arrives once the
autocomplete is already gone. Dismissing with escape, and the fallback close when
there was no selection to confirm, both happen while it is still open and are
unaffected.

Tests: a single undo restores the typed text after a completion, and the
redundant close causes no further render.
This commit is contained in:
hayyaksi
2026-08-10 14:01:38 +00:00
committed by GitHub
parent 613ee3c324
commit 0b264a482e
2 changed files with 44 additions and 0 deletions
+38
View File
@@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
import { describe, it, expect } from "vitest";
import EditorModel from "./model";
import HistoryManager from "./history";
import { createPartCreator, createRenderer, type MockAutoComplete } from "./__mocks__";
import DocumentOffset from "./offset";
import { type PillPart } from "./parts";
@@ -192,6 +193,43 @@ describe("editor/model", function () {
expect(model.parts[1].text).toBe("Alice");
});
it("reverses a confirmed completion with a single undo", function () {
const history = new HistoryManager();
const pc = createPartCreator([{ resourceId: "@alice", text: "Alice" } as PillPart]);
// Stands in for the composer, which pushes every update it is told about onto the
// undo stack.
const model: EditorModel = new EditorModel([pc.plain("hello ")], pc, (caret) =>
history.tryPush(model, caret),
);
model.update("hello @a", "insertText", new DocumentOffset(8, true));
history.ensureLastChangesPushed(model);
const autoComplete = model.autoComplete as unknown as MockAutoComplete;
autoComplete.tryComplete();
// Confirming a completion closes the autocomplete a second time, which is what used to
// put an identical second state on the undo stack.
autoComplete.close();
const undone = history.undo(model);
expect(undone).toBeTruthy();
expect(undone!.parts.map((p) => p.text)).toEqual(["hello ", "@a"]);
});
it("does not report the redundant close which follows a completion", function () {
const renderer = createRenderer();
const pc = createPartCreator([{ resourceId: "@alice", text: "Alice" } as PillPart]);
const model = new EditorModel([pc.plain("hello ")], pc, renderer);
model.update("hello @a", "insertText", new DocumentOffset(8, true));
const autoComplete = model.autoComplete as unknown as MockAutoComplete;
autoComplete.tryComplete();
expect(renderer.count).toBe(2);
autoComplete.close();
expect(renderer.count).toBe(2);
});
it("insert room pill", function () {
const renderer = createRenderer();
const pc = createPartCreator([{ resourceId: "#riot-dev" } as PillPart]);
+6
View File
@@ -262,6 +262,12 @@ export default class EditorModel {
}
private onAutoComplete = ({ replaceParts, close, range }: ICallback): void => {
// Confirming a completion closes the autocomplete twice: once as part of the completion and
// once more by the wrapper afterwards. Reporting the second one would have the history
// manager record a second, identical state, costing the user an extra undo to reverse one
// completion.
if (!replaceParts && close && !this._autoComplete) return;
let pos: DocumentPosition | undefined;
if (replaceParts) {
const autoCompletePartIdx = this.autoCompletePartIdx || 0;