Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ A real Electron app with file IO, live preview, and every plugin enabled — the
| `@floatboat/nexus-react` | React binding — `useEditor` hook and `<Editor />` component |
| `@floatboat/nexus-vue` | Vue 3 binding — `useEditor` composable |
| `@floatboat/nexus-preset-gfm` | GitHub Flavored Markdown preset (tables, strikethrough, task lists) |
| `@floatboat/nexus-plugin-history` | Undo/redo with `Ctrl+Z` / `Ctrl+Shift+Z` |
| `@floatboat/nexus-plugin-history` | Undo/redo with `Ctrl+Z` / `Ctrl+Shift+Z`, plus document-load history boundaries |
| `@floatboat/nexus-plugin-search` | Search and replace helpers |
| `@floatboat/nexus-plugin-slash` | Slash command detection, ranking, and a vanilla-DOM floating menu UI |
| `@floatboat/nexus-plugin-toolbar` | Toolbar primitives for formatting commands |
Expand Down Expand Up @@ -209,9 +209,10 @@ Plugin platform documentation (Chinese): [native plugin API](./docs/plugins/nati
```ts
editor.getDocument() // current Markdown string
editor.getAst() // current mdast Root
editor.setDocument(md) // replace entire document
editor.setDocument(md) // replace entire document (undoable)
editor.setDocument(md, { silent: true, preserveSelection: true })
editor.setDocument(md, { selection: { anchor: 0 } })
editor.setDocument(md, { silent: true, history: "reset" }) // open a file: not undoable, clears undo/redo
editor.setSelection(pos) // move cursor
editor.focus() / editor.blur()
editor.destroy()
Expand All @@ -226,6 +227,20 @@ editor.off("change", handler)
editor.getCoordsAtPos(pos) // { left, right, top, bottom } | null
```

**Document loading and undo history.** `setDocument` accepts `history`, which
controls how the replacement interacts with undo/redo:

| `history` | Replacement becomes an undo entry | Clears undo/redo stacks | Use for |
|---|---|---|---|
| `"record"` *(default)* | yes | no | programmatic user edits — toolbar formatting, replace-all |
| `"skip"` | no | no | incoming text that should not itself be undoable |
| `"reset"` | no | yes | opening or switching files |

Loading a file with the default records the load as an edit, so `Ctrl+Z` before
typing reverts the buffer and a redo from the previous file leaks into the new
one. Pass `history: "reset"` at a document boundary instead. It is a no-op when
no history plugin is installed.

</details>

<details>
Expand Down
5 changes: 4 additions & 1 deletion apps/electron-demo/src/renderer/editor-shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,11 @@ export function createEditorShell(options: EditorShellOptions): EditorShell {
// onChange pipeline (avoids redundant parse + link-index rebuild on
// every file open). Caller owns state.content / dirty below and the
// link index is re-seeded as part of vault open.
// `history: "reset"` makes the open a document boundary: Ctrl+Z must
// not revert the file that was just opened, and a redo left over from
// the previous document must not restore it into this buffer.
const t0 = performance.now();
editor.setDocument(content, { silent: true });
editor.setDocument(content, { silent: true, history: "reset" });
const t1 = performance.now();
state.content = content;
state.dirty = false;
Expand Down
74 changes: 74 additions & 0 deletions apps/electron-demo/test/editor-shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,80 @@ describe("createEditorShell", () => {
shell.destroy();
});

it("loads a document as a fresh undo/redo history boundary", () => {
const container = document.createElement("div");
const state = createState();
const shell = createEditorShell({
container,
state,
settings: defaultSettings(),
onStateChange: vi.fn(),
});

// Opening a file must not become an undo step — otherwise Ctrl+Z before
// typing anything reverts the loaded buffer back to the empty document.
shell.loadDocument("opened file");
expect(shell.editor.undo()).toBe(false);
expect(shell.editor.getDocument()).toBe("opened file");

// An edit after the load is undoable, and undo stops at the loaded content.
shell.editor.setDocument("opened file edited", { history: "record" });
expect(shell.editor.undo()).toBe(true);
expect(shell.editor.getDocument()).toBe("opened file");
expect(shell.editor.redo()).toBe(true);
expect(shell.editor.getDocument()).toBe("opened file edited");

// Switching documents must not let the previous file leak back in, in
// either direction.
shell.loadDocument("second file");
expect(shell.editor.undo()).toBe(false);
expect(shell.editor.redo()).toBe(false);
expect(shell.editor.getDocument()).toBe("second file");

shell.destroy();
});

it("keeps each shell's undo history isolated from other live editors", () => {
const firstContainer = document.createElement("div");
document.body.append(firstContainer);
const secondContainer = document.createElement("div");
document.body.append(secondContainer);

const first = createEditorShell({
container: firstContainer,
state: createState(),
settings: defaultSettings(),
onStateChange: vi.fn(),
});
const second = createEditorShell({
container: secondContainer,
state: createState(),
settings: defaultSettings(),
onStateChange: vi.fn(),
});

first.loadDocument("first opened file");
first.editor.setDocument("first opened file edited", { history: "record" });

// A reset in one shell must not disturb another live shell's stacks. This
// holds because each editor owns its history compartment; the assertions
// lock the contract rather than reproducing a known defect.
second.loadDocument("second opened file");
second.editor.setDocument("second opened file edited", { history: "record" });
expect(second.editor.undo()).toBe(true);
expect(second.editor.getDocument()).toBe("second opened file");
expect(second.editor.undo()).toBe(false);

expect(first.editor.undo()).toBe(true);
expect(first.editor.getDocument()).toBe("first opened file");
expect(first.editor.undo()).toBe(false);

first.destroy();
second.destroy();
firstContainer.remove();
secondContainer.remove();
});

it("leaves migrated UI to the runtime owner in runtime mode", () => {
const container = document.createElement("div");
document.body.append(container);
Expand Down
88 changes: 88 additions & 0 deletions openspec/changes/fix-document-load-undo-boundary/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
## Context

`setDocument` is the single whole-document replacement primitive on `EditorAPI`. It has two callers with opposite requirements:

| Caller | Example | Requirement |
|---|---|---|
| A programmatic user edit | `plugin-toolbar` formatting, `plugin-search` replace-all, `apps/electron-demo` image drag-resize | The edit must be undoable — Ctrl+Z has to take it back |
| A document load | `EditorShell.loadDocument`, controlled `value` sync in `packages/react` / `packages/vue` | The load must not be undoable — Ctrl+Z must never erase an opened file |

Today both collapse into one behavior: any non-empty change set is recorded by the CM6 history field unless the transaction carries `Transaction.addToHistory.of(false)`. `silent` does not help — it only suppresses the `onChange` / `change` event and the AST resync (`packages/core/src/editor.ts`, `silentDocChange` annotation), and it is also used by the framework bindings for controlled-value sync, which must **not** silently drop user history.

There is also no existing spec for this: `openspec/specs/` contains only plugin-* capabilities, and the archived `plugin-editor-extensions` spec only requires that reconfiguration preserve "compatible undo/redo history". The load-boundary behavior was never specified, which is why the bug shipped with all 7 existing history/editor-demo tests green.

## Goals / Non-Goals

- Goals
- Make the document-load boundary explicit and opt-in, without changing any existing call site's behavior.
- Guarantee that undo/redo cannot cross a document load in either direction.
- Keep the fix inside `packages/core` + the demo caller: no new dependency, no new package, no plugin API surface.
- Non-Goals
- Undo/redo *grouping* (Roadmap item 8) — merging adjacent edits into fewer entries is a separate change.
- Multi-document history stacks or per-file history restoration. `"reset"` discards history; restoring a previous document's history is not attempted.
- Changing the history behavior of `replaceRange` or `replaceSelection`. Range edits stay `"record"`-only in this change so the surface stays minimal.

## Decisions

### Decision: a three-valued `history` option rather than a boolean

`"record" | "skip" | "reset"` names three genuinely different intents, and a boolean (`resetHistory: true`) cannot express the middle one. The middle case is not hypothetical: a host that renders external content (sync pull, formatter, test harness) wants the incoming text to be visible to subsequent undo mapping without itself becoming an undo step.

- Alternative considered: `resetHistory?: boolean` — rejected, cannot express `"skip"`, and `false` would be indistinguishable from the default.
- Alternative considered: a separate `editor.resetHistory()` method plus `setDocument(..., { skipHistory: true })` — rejected as two coordinated calls for one intent. Two dispatches also reintroduce the ordering hazard that `add-selection-api` already fixed for edits: a caller that forgets the second call silently keeps the bug.

### Decision: default stays `"record"`

`setDocument` is used for user-initiated edits in `packages/plugin-toolbar` (6 sites) and `packages/plugin-search` (2 sites). Those are covered by regression tests that assert a single Ctrl+Z reverts a toolbar toggle (`packages/plugin-toolbar/test/plugin-toolbar.test.ts`, "toggleUnorderedList — atomic undo"). Flipping the default to `"skip"` would break undo for formatting commands, so the load caller opts in instead.

### Decision: reset is a two-transaction compartment toggle, not a field write

Clearing the stacks turned out to be the hard part, and three plausible approaches were measured against a real CodeMirror state before choosing. Only one works:

1. `Transaction.addToHistory.of(false)` stops the load from *becoming* an entry, but explicitly does not clear what is already on the stack — CM6 maps the change through `state.addMapping(tr.changes.desc)` and keeps every earlier entry. Undo still reverts to the pre-load document. **Rejected on its own.**
2. `isolateHistory.of("full")` bounds the *redo* stack only. Measured: after loading B over an edited A, `done` still held the A edit and undo produced `"BA"`. **Rejected.**
3. `historyField.init(factory)` looks like the way to supply a replacement value, but it returns an *extension array* (`[field, initField.of(...)]`), not a dispatchable `StateEffect` — `initField` is a facet consulted on the field's create/reconfigure path. Passing it as an effect throws `effect.is is not a function`. **Rejected; the API for writing a live field value does not exist.**
4. **Chosen:** rebuild the extension itself. Reconfiguring the history compartment to `[]` drops the history field (and both stacks); reconfiguring it back to the plugin's `history()` re-creates it empty.

A CodeMirror compartment keeps only the **last** `reconfigure` in a transaction — measured directly — so the toggle cannot be a single dispatch. `performSetDocument` therefore issues two dispatches for `"reset"`: an effects-only dispatch that drops the extension, then the load transaction that installs a fresh instance. Both run synchronously in the same task with no input handling between them, so nothing can slip into the gap. The load transaction still carries `addToHistory(false)`, which keeps the replacement itself out of the rebuilt stack.

- Alternative considered: `view.setState(EditorState.create({ doc, extensions: state.config.extensions }))` — this does clear the stacks, but rebuilds every view plugin, losing scroll position, focus, and widget DOM. Rejected as disproportionate for a file open.
- Alternative considered: drain the stack with `while (editor.undo()) {}` — rejected. It is O(history depth) dispatches, each rebuilding decorations and firing `selectionChange`, and it reads as a workaround rather than a boundary.

### Decision: history extensions go through the compartment *exclusively*

The first working draft kept the plugin's `cmExtensions` in the plain extension list **and** installed a copy inside the compartment. Resetting then cleared only the compartment copy while the duplicate kept the history field — and its stacks — alive, so `"reset"` silently did nothing even though the disable dispatch ran. Measured on the real editor: `historyExtensions: 2, hasDisable: true` and yet `undone=1` after the load.

Declaring `historyCompartment: true` therefore *moves* a plugin's extensions into the compartment rather than copying them: the editor filters those plugins out of the plain list. This is the load-bearing part of the design and is pinned by the `"reset"` regression tests.

### Decision: the compartment is per-editor

Each `createEditor` call creates its own `Compartment`. CodeMirror computes compartment values per state, so a shared instance was measured **not** to leak across editors — resetting one editor left another's `done`/`undone` stacks intact. A per-editor instance is still the right contract: it makes "this editor owns its history slot" structural instead of incidental, and it keeps a plugin-level singleton from becoming the thing that breaks if a future CodeMirror version resolves compartments differently.

Plugins opt in with `historyCompartment?: true`, and `createHistoryPlugin()` sets it. Plugins that install `history()` themselves keep working unchanged and simply keep their history across document loads.

- Alternative considered: exporting a factory so plugins could create the compartment and hand it back — rejected as a wider public API with an ordering hazard for no additional capability.
- Alternative considered: an internal marker that does not appear on `NexusPlugin` — rejected as hidden coupling; a documented optional field is honest about the contract.

### Decision: no new dependency

Everything used is already exported by current dependencies: `Compartment` and `Transaction.addToHistory` from `@codemirror/state`, which `packages/core` already depends on. `@codemirror/commands` is not even needed for the reset path. This change adds no `dependencies` entry, so `GOVERNANCE.md` §6.3 does not apply.

## Risks / Trade-offs

- **Toggling a third-party extension off and back on** → Uses only documented `Compartment` behaviour, and the resulting semantics are locked by regression tests covering first load, load-after-undo, load-after-edit, and no-history-plugin. If `@codemirror/commands` ever exposes a supported reset, this can be simplified.
- **A plugin declaring `historyCompartment` moves its extensions into the compartment** → The contract is documented on `NexusPlugin.historyCompartment` and covered by the `"reset"` tests; a plugin that both declares it and expects its extensions on the plain path would notice immediately, because its history would reset on load.
- **`"reset"` degrades to a no-op when no history plugin is installed** → `historyExtensions` is empty, so the editor skips both dispatches instead of reconfiguring. A host without history has no stacks to clear, and the option is documented as a no-op there.
- **A host that expects file-switch undo** → This is the bug being fixed, and the option is opt-in, so only `EditorShell.loadDocument` changes behavior. The framework bindings keep their current controlled-sync semantics.
- **Two existing `plugin-history` tests assert the buggy assumption** → They are re-pointed to `history: "record"` so the test keeps covering "a recorded replacement is undoable" while a new case covers the boundary. Re-pointing is deliberate, not test-fitting, and it is called out in the PR description.
- **Undo across `"skip"` loads** → A `"skip"` load is still mapped, so position mapping is the only expectation; the scenario pins that a later `"record"` edit undoes to the skipped content, not past it.
- **Two dispatches instead of one for `"reset"`** → Visible to any transaction filter that counts dispatches per `setDocument` call. Accepted: it is the only way to express the toggle, and both dispatches happen inside one synchronous call.

## Migration Plan

Additive and opt-in. `EditorShell.loadDocument` is the only call site that switches to `"reset"`; every other caller keeps `"record"` by default and needs no change. No data migration, no serialized-state format change, no public API removal. Rollback is reverting the option plumbing — the default path is untouched.

## Open Questions

- Should `replaceRange` grow the same option? Deferred deliberately: no caller needs it today, and adding it later is a compatible change. Recorded here so the omission is a decision rather than an oversight.
- Should a `"reset"` load also reset scroll position or folding state? Out of scope; the load path already owns viewport behavior in the demo shell.
24 changes: 24 additions & 0 deletions openspec/changes/fix-document-load-undo-boundary/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Change: Isolate undo history across document loads

## Why
Opening a file is recorded as an undoable edit. `loadDocument` loads disk content through `editor.setDocument(content, { silent: true })`, and `silent` only suppresses the `onChange` pipeline — the whole-document replacement still lands in the CM6 undo stack. Consequences a user hits immediately:

- Open a file, press Ctrl+Z before typing anything, and the buffer is reverted to the empty document that preceded the load (content appears deleted).
- Edit A, open B, press Ctrl+Z in B, and A's text is restored into B's buffer.
- Undo in B can also redo A's edits, because the redo stack survives the switch.

This is a data-presentation bug in the load path, not a request for new undo behavior.

## What Changes
- **ADDED** `SetDocumentOptions.history?: "record" | "skip" | "reset"` on `setDocument`, with `"record"` as the default so every existing programmatic-replacement call site keeps its current undo semantics (toolbar formatting, search replace-all).
- `"skip"` — the replacement participates in history mapping but is not recorded as an undo entry.
- `"reset"` — the replacement is not recorded **and** the undo and redo stacks are cleared, making the load an explicit document boundary. Implemented by routing the history extension through a per-editor compartment and rebuilding it.
- **ADDED** optional `NexusPlugin.historyCompartment` so the editor installs a plugin's history extension inside that compartment instead of the plain extension list. `createHistoryPlugin()` declares it.
- **MODIFIED** `apps/electron-demo` `EditorShell.loadDocument` to pass `history: "reset"`, so file open and file switch start a fresh history.
- **No breaking changes**: the option is additive and defaults to today's behavior; plugins that install `history()` themselves are unaffected.

## Impact
- Affected specs: editor-core
- Affected code: `packages/core/src/types.ts`, `packages/core/src/editor.ts`, `packages/plugin-history/src/index.ts`, `apps/electron-demo/src/renderer/editor-shell.ts`, `README.md`
- No new runtime dependencies; `Compartment` and `Transaction.addToHistory` come from `@codemirror/state`, a current `packages/core` dependency.
- Hosts that do not load `plugin-history` are unaffected: `history: "reset"` degrades to a no-op instead of throwing.
Loading