Conversation
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary / 摘要
Loading a file is recorded as an undoable edit, so
Ctrl+Zbefore typing anything reverts the freshly opened buffer — and a redo left over from the previous document restores that document into the new one.setDocumentnow takes ahistoryoption, and the demo's file-open path uses it to make a load a real history boundary.Motivation / 背景与动机
Undo / redo grouping(本 PR 不含 grouping,只修边界)openspec/changes/fix-document-load-undo-boundary/loadDocumentloads disk content withsetDocument(content, { silent: true }).silentonly suppresses theonChange/changeevent and the AST resync — it never touched the CM6 undo stack, so the whole-document replacement was recorded like a user edit. Result:silentcould not simply be reinterpreted as "document load":packages/reactandpackages/vuealso use it for controlled-valuesync, where dropping history would be wrong.Changes / 变更内容
packages/core:SetDocumentOptions.history?: "record" | "skip" | "reset", default"record"so every existing call site keeps today's semantics."skip"→Transaction.addToHistory.of(false); the load is not an undo entry but earlier entries stay mapped."reset"→ also rebuilds the history extension: the editor owns a per-editorCompartmentfor history, and reset toggles the plugin'shistory()out and back in.NexusPlugin.historyCompartment?: true— the opt-in marker that routes a plugin'scmExtensionsthrough that compartment.packages/plugin-history:createHistoryPlugin()declareshistoryCompartment: true. Plugins that installhistory()themselves keep working unchanged and simply retain history across loads.apps/electron-demo:EditorShell.loadDocumentpasses{ silent: true, history: "reset" }. All fourshell.loadDocument(...)call sites inapp.tsinherit the boundary, including close-file (loadDocument("")), which runs behindconfirmDiscardIfDirty().search-bar.tsreplace-all andplugin-toolbarformatting still rely on the"record"default so their edits stay undoable.openspec/:fix-document-load-undo-boundary(proposal / design / tasks /specs/editor-coredelta with 7 scenarios).README.md:historyoption and the load-boundary semantics; updates theplugin-historyrow.Why
"reset"needs two dispatches (the part worth reviewing)Parallel attempts were measured against real CodeMirror state before choosing; three of them silently do not work:
Transaction.addToHistory.of(false)alone does not clear what is already on the stack — CM6 keeps every earlier entry and maps the change through them.isolateHistory.of("full")bounds the redo stack only; measured,donestill held the previous document's edit and undo produced"BA".historyField.init(factory)looks like a value replacement, but it returns an extension array ([field, initField.of(...)]), not a dispatchable effect — passing it as an effect throwseffect.is is not a function. There is no public API to write a live field value.What works is reconfiguring the history extension itself. A compartment keeps only the last
reconfigurein a transaction, soperformSetDocumentissues an effects-only dispatch that drops the extension (and both stacks), then the load transaction that installs a fresh instance. Both run synchronously in one call, so no input can slip into the gap.Two details that a first draft got wrong and the tests now pin down:
cmExtensionsinto the plain extension list. Clearing the compartment then left the duplicate copy alive, so"reset"silently did nothing even though the disable dispatch ran (historyExtensions: 2, hasDisable: true, yetundonestayed at 1). DeclaringhistoryCompartment: truenow moves those extensions into the compartment.createEditorShelltest locks that isolation.Testing / 测试
pnpm testpasses / 全绿 —Test Files 1 failed | 67 passed,Tests 10 failed | 900 passed (910)pnpm build) / 受影响包构建通过pnpm typecheckclean,pnpm check:apipasses,pnpm build:electron-demosucceedspackages/core/test/editor.test.ts— 8 new cases insetDocument undo history boundary(default records;skip;reset; undo-after-edit stops at the loaded content; redo stack cleared; no history plugin installed —resetandskip; reset load deferred by IME composition)packages/plugin-history/test/plugin-history.test.ts— 2 existing cases re-pointed tohistory: "record"(they asserted the buggy assumption that a baresetDocumentis undoable) + 2 new isolation casesapps/electron-demo/test/editor-shell.test.ts— 2 integration cases over the realcreateEditorShell+loadDocumentpath, including that resetting one shell leaves another live shell's stacks intactPre-existing failures, not caused by this change
apps/electron-demo/test/plugin-host-broker.test.tsfails 10 cases on this Windows machine both before and after the change (888 passed / 10 failedon a cleanmaincheckout,900 passed / 10 failedwith this change). They are environmental: symlink creation needs elevation (EPERM) andO_NONBLOCKhas different semantics on Windows. None of them touch undo history.The first red run of the new tests failed for the right reason (option not implemented yet); the IME case initially failed on my own test harness (fake timers prevented CodeMirror from initializing), which I fixed before implementing.
Compliance / 合规自检
AI-assisted notes / AI 使用说明:bug发现为本人实际使用demo后发现,交给Codex,使用GPT6Astra 验证和提出解决方案,使用DSH搭配DSv4.1Flash实现.
Compartment,StateEffect, andTransaction.addToHistorycome from@codemirror/state, already apackages/coredependency.@codemirror/commandsis not used by the reset path at all.dist/,dist-electron/, compiled.jsfrom.ts) —git statusshows only source, tests, README, and the OpenSpec change.env/ personal vault data committedChecklist / 自检清单
README.mddocumentshistory; both new options carry JSDoc. No exports were added or removed, so thecheck:apisnapshot is unchanged.live-preview-table.ts→ walked through the 12 Table Widget rules in CLAUDE.md — not touchedpackages/corebug fix + demo callerScreenshots / Recordings · 截图或录屏 (UI changes)
修复前:


修复后: