Skip to content

fix(session-ui): add CM6 editing shortcuts (undo/redo/copy/cut/paste/select-all) to Files Changed editor #302

Description

@jeonghun-jj-lee

Important

Problem: Standard editing shortcuts (Cmd+Z, Cmd+Shift+Z, Cmd+C, Cmd+X, Cmd+V, Cmd+A)
don't work in the CodeMirror 6 diff editor in the Files Changed tab. Two root causes:
(1) the CM6 editor has no history(), defaultKeymap, or historyKeymap extensions, so
there is nothing to handle undo/redo or standard editing bindings; (2) inside the VS Code
webview, global-clipboard.ts intercepts these chords in capture phase and calls
event.preventDefault() + document.execCommand(), which CM6 ignores because it manages
its own document model outside the DOM undo stack.

Approach: Add the missing CM6 editing extensions (history, defaultKeymap,
historyKeymap, bracketMatching). Extend the existing CLIPBOARD_SELF_SELECTOR
contract in global-clipboard.ts with a "codemirror" value that tells the global handler
which chords to delegate and which to continue bridging.

Scope: Four files across two packages (session-ui and app). No new endpoints, no
component restructuring. One doc fix: update the revert() comment to match the now-visible
undo-after-revert behavior.

Assumptions: execCommand("insertText") fires a beforeinput event that CM6 catches
(standard browser behavior for contentEditable). execCommand("delete") similarly fires
beforeinput for CM6-compatible selection deletion. window.getSelection().toString()
correctly reads CM6's rendered selection.

Acceptance Criteria

  • Cmd+Z / Cmd+Shift+Z (undo/redo) works in the editable diff editor, both inside the VS Code webview and in a plain browser
  • Undo history tracks user edits but NOT external content updates (updateOriginal, updateModified)
  • Cmd+C / Cmd+X (copy/cut) works inside the VS Code webview — text reaches the OS clipboard via the bridge
  • Cut removes the selected text AND the deletion is tracked in CM6's undo history (undoable)
  • Cmd+V (paste) works inside the VS Code webview — text is inserted from the clipboard bridge and tracked in CM6's undo history
  • Cmd+A selects all text within the focused editor pane, not the entire review panel
  • Standard editing keybindings from defaultKeymap work (backspace-by-word, move-by-group, etc.)
  • Bracket matching highlights matching brackets/parens in both read-only and editable panes
  • Read-only pane (original side in split mode, deleted files) is unaffected — no editable keybindings fire
  • Revert is undoable — Cmd+Z after revert restores the pre-revert edits (the revert() dispatch goes through CM6's history)
  • Existing tests in editable-diff-view.test.ts pass unchanged
  • No regressions in other editable elements (prompt composer, profile fields, preview tab textarea)

Key Decisions

# Decision Choice Rejected alternatives
D1 CM6 opt-out mechanism Extend CLIPBOARD_SELF_SELECTOR with data-amc-clipboard="codemirror" stopPropagation on container (can't block window capture); CM6 class check (couples global handler to CM6 internals)
D2 Extension scope history + defaultKeymap + historyKeymap + bracketMatching Full editing suite with closeBrackets, indentOnInput, autocompletion (overkill for diff review); strict minimum without bracketMatching
D3 History placement In editableExtensions() (only when readOnly: false) In baseExtensions() (wasteful allocation on read-only panes)
D4 Per-chord handling Z/Y/A: delegate to CM6; C/X/V: global handler bridges clipboard Blanket opt-out for all chords (copy/cut/paste fail in iframe without bridge); blanket keep (undo/redo/select-all broken)
D5 Cut deletion method execCommand("delete") for CM6 targets (fires beforeinput) range.deleteContents() (current — bypasses CM6 transactions); CM6 view API call (requires access to EditorView from global handler)
D6 Cmd+A scope when CM6 focused Editor-scoped select-all via CM6's defaultKeymap Panel-wide select-all (current behavior — wrong when editing in an editor)
D7 Revert + undo interaction Revert is undoable (Cmd+Z after revert recovers edits) — update the revert() docstring to match Non-undoable revert with history clearing (worse UX — accidental revert is unrecoverable)

Constraints & Invariants

  • The CLIPBOARD_SELF_SELECTOR (data-amc-clipboard="self") contract for paste-only opt-out is unchanged — existing consumers (profile fields) are unaffected.
  • The global clipboard handler's behavior for non-CM6 editable targets (prompt composer, textareas, profile fields) is unchanged.
  • External content updates (updateOriginal, updateModified) are excluded from history via Transaction.addToHistory.of(false) AND from the onChange callback via the externalUpdate annotation (#837). Both annotations coexist; the new history() extension respects addToHistory by default.
  • The revert() dispatch currently annotates with externalUpdate.of(true) (suppresses onChange) but does NOT annotate with addToHistory.of(false) — the revert is recorded in the undo stack (D7). The docstring at DiffEditorHandle.revert (line 273) and EditableDiffViewProps.onRevert (line 57) still say "clear undo history" and must be updated.
  • No new dependencies beyond @codemirror/commands in session-ui.

Implementation

session-ui/package.json

Add @codemirror/commands 6.11.0 to dependencies (compatible with installed @codemirror/state 6.7.1 and @codemirror/view 6.43.9).

editable-diff-view-core.ts

Import history, defaultKeymap, historyKeymap from @codemirror/commands, bracketMatching from @codemirror/language, and keymap from @codemirror/view.

Add to baseExtensions():

keymap.of([...defaultKeymap, ...historyKeymap]),
bracketMatching(),

Add to editableExtensions() when readOnly is false:

history(),

Update DiffEditorHandle interface docstring for revert (line 273) — change "clear undo history" to "the revert itself is undoable via Cmd+Z". Similarly update the onRevert prop docstring in editable-diff-view.tsx (line 57). The revert() implementation already uses annotations: externalUpdate.of(true) to suppress onChange; no code change needed — only the docstrings are wrong.

editable-diff-view.tsx

Add data-amc-clipboard="codemirror" to the container <div>.

global-clipboard.ts

Add constant:

const CLIPBOARD_EDITOR_SELECTOR = '[data-amc-clipboard="codemirror"]'

In the onKeyDown handler, immediately after the non-editable branch exits (after line 287's return), detect CM6 before any editable-target chord handling:

// --- Managed editor (CodeMirror 6) — delegate undo/redo/select-all, bridge clipboard ---
const insideEditor = target instanceof Element && target.closest(CLIPBOARD_EDITOR_SELECTOR)
if (insideEditor && (key === "z" || key === "y" || key === "a")) return

In extractSelection(), when cutting inside a CM6 target, use execCommand("delete") instead of range.deleteContents() so CM6's beforeinput handler tracks the deletion:

if (opts.cut) {
  if (el.closest(CLIPBOARD_EDITOR_SELECTOR)) {
    el.ownerDocument.execCommand("delete")
  } else {
    range.deleteContents()
    dispatchInput(el, "deleteByCut")
  }
}

Source

Follow-up to PR #300 (editable CodeMirror diff viewer).

Adversarial Review Notes

Two rounds of manual review (no amico tooling — weaker claim than tooling-backed).

Round 1: 13 questions across three lenses (technical correctness, completeness, contradictions). 11 confirmed, 1 blocking (resolved as D7), 2 advisory.

Round 2: 17 checks against the current codebase after the #837 externalUpdate annotation landed. All passed. Two clarifications folded in: pinned @codemirror/commands version to 6.11.0, specified exact insertion point for the insideEditor check.

Blocking (resolved in round 1): The revert() docstring says "clear undo history" but the implementation doesn't clear it. With history() added, revert becomes undoable. Resolution: accept undoable revert as the correct behavior (D7), update the docstring.

Advisory obligations:

  1. History persistence across readOnly togglehistory() is inside the editableCompartment. CM6's historyField is a module-level singleton, so the undo stack likely survives setReadOnly(true)setReadOnly(false), but this is untested. Add a test.
  2. Emacs-style Ctrl+V conflict — on macOS, Ctrl+V (Emacs cursorPageDown) is intercepted by the global clipboard handler as paste. This is a known limitation; standard macOS users expect Cmd+V for paste. Documenting, not fixing.

Verified mechanisms:

  • event.defaultPrevented blocks CM6 keymap processing (CM6 view source: eventBelongsToEditor line 4870)
  • execCommand("insertText"/"delete") fires beforeinput → CM6's MutationObserver catches the DOM mutation → transaction is created and tracked in history
  • window.getSelection().toString() correctly reads CM6's rendered selection (drawSelection() does not suppress native selection)
  • command.tsx returns early for clipboard chords on editable targets (line 409-413) — no interference
  • Read-only pane keydown: global handler exits via non-editable branch; CM6's undo command is a no-op on read-only state

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions