From e07542d2addb70251f0e5706852702df5999a573 Mon Sep 17 00:00:00 2001 From: PathGao <42336971+PathGao@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:08:28 +0800 Subject: [PATCH] feat(shortcuts): list Undo, Redo, Cut, Copy and Paste in the shortcuts panel Undo and Redo are Monaco's own commands, so their rows name the Monaco command and editor.chords.spec.ts checks them against Monaco's keymap. toMonacoLabel moves into keymapHarness so that spec can use it. --- scripts/editor.chords.spec.ts | 18 ++++++++- scripts/keymapHarness.ts | 46 +++++++++++++++++++++ scripts/shortcuts.editor.test.ts | 69 +++----------------------------- scripts/shortcuts.test.ts | 52 +----------------------- src/lib/utils/shortcuts.ts | 24 ++++++----- 5 files changed, 85 insertions(+), 124 deletions(-) diff --git a/scripts/editor.chords.spec.ts b/scripts/editor.chords.spec.ts index 9c6ef09f..9a0d8a3a 100644 --- a/scripts/editor.chords.spec.ts +++ b/scripts/editor.chords.spec.ts @@ -3,7 +3,8 @@ import assert from 'node:assert/strict'; import { decodeKeybinding, type KeyCodeChord } from 'monaco-editor/esm/vs/base/common/keybindings.js'; import { describe, test, beforeAll, expect, vi } from 'vitest'; -import { bareCommands, type Chord, editorKeymap, label, PLATFORMS } from './keymapHarness.js'; +import { SHORTCUTS } from '../src/lib/utils/shortcuts.js'; +import { bareCommands, type Chord, editorKeymap, label, PLATFORMS, toMonacoLabel } from './keymapHarness.js'; /* * WHO OWNS A CHORD: the app, or Monaco? @@ -520,6 +521,21 @@ describe('Monaco chord ownership', () => { } }); + test('a shortcut the panel credits to Monaco is one Monaco binds and the app leaves alone', () => { + const rows = SHORTCUTS.filter((entry) => entry.monacoCommand); + assert.ok(rows.length >= 2, `${rows.length} registry rows name a Monaco command`); + for (const entry of rows) { + for (const platform of PLATFORMS) { + const app = appEditorChords(platform.mac, platform.os); + for (const chord of entry.chords.map((c) => toMonacoLabel(c, platform.mac))) { + const where = `${platform.name}: ${entry.id} (${chord})`; + assert.ok(defaultsFor(platform.name).get(chord)?.includes(entry.monacoCommand!), `${where} is not Monaco's ${entry.monacoCommand}`); + assert.deepEqual(app.get(chord) ?? [], [], `${where} is taken by the app`); + } + } + } + }); + test('the unrecoverable set has no allow-list escape hatch', () => { // The rule above only holds if nobody can answer a protected collision by // appending a row to DELIBERATE_OVERRIDES, which is what the next person diff --git a/scripts/keymapHarness.ts b/scripts/keymapHarness.ts index 5540ca75..424fb818 100644 --- a/scripts/keymapHarness.ts +++ b/scripts/keymapHarness.ts @@ -301,3 +301,49 @@ export type BareCommand = EditorKeyCommand; export function bareCommands(): BareCommand[] { return editorKeyCommands({ KeyMod, KeyCode } as never); } + +/** + * The key names Monaco's `KeyCodeUtils.toString` prints, where they differ from + * the way the registry spells the key for a human. + */ +const MONACO_KEY_NAMES: Record = { + Left: 'LeftArrow', + Right: 'RightArrow', +}; + +/** + * A registry chord as the harness spells the same chord. + * + * The registry is written for a reader (`Mod+Shift+E`); Monaco's decoder emits + * `Shift+Meta+E` on macOS and `Ctrl+Shift+E` elsewhere, in its own modifier + * order. Translating in this direction — registry into harness — rather than + * the other way keeps the registry human-facing, and a mistake here makes + * assertions FAIL rather than pass, because the comparison is against chords + * the real code produced. + */ +export function toMonacoLabel(chord: string, mac: boolean): Chord { + return chord + .split(' ') // a chord SEQUENCE ("Mod+K T") is two chords + .map((single) => { + const parts = single.split('+'); + const key = parts.pop()!; + const mods = new Set(parts); + const ctrl = mods.has('Ctrl') || (mods.has('Mod') && !mac); + // A literal `Meta` stays Meta on every platform, the same way a literal + // `Ctrl` does. The registry has no such row — the chords that need this + // spelling are the unadvertised ones named in + // DOCUMENT_CHORDS_NOT_ADVERTISED, which are written in registry syntax so + // the exemption list and the registry read alike. + const meta = mods.has('Meta') || (mods.has('Mod') && mac); + return [ + ctrl && 'Ctrl', + mods.has('Shift') && 'Shift', + mods.has('Alt') && 'Alt', + meta && 'Meta', + MONACO_KEY_NAMES[key] ?? key, + ] + .filter(Boolean) + .join('+'); + }) + .join(' '); +} diff --git a/scripts/shortcuts.editor.test.ts b/scripts/shortcuts.editor.test.ts index bfb5cc5a..458836c5 100644 --- a/scripts/shortcuts.editor.test.ts +++ b/scripts/shortcuts.editor.test.ts @@ -5,7 +5,7 @@ import { getEditorToolbarTools } from '../src/lib/utils/editorToolbar.js'; import { t } from '../src/lib/utils/i18n.js'; import { formatChord, SHORTCUTS } from '../src/lib/utils/shortcuts.js'; import type { ViewerCommand } from '../src/lib/utils/viewerKeymap.js'; -import { bareCommands, type Chord, chordOf, documentKeymap, editorKeymap, OperatingSystem, PLATFORMS, registeredActions } from './keymapHarness.js'; +import { bareCommands, type Chord, chordOf, documentKeymap, editorKeymap, OperatingSystem, PLATFORMS, registeredActions, toMonacoLabel } from './keymapHarness.js'; // The editor half of the shortcut contract: the keymap `utils/editorActions.ts` // declares (actions and nameless key commands), run through `keymapHarness.ts`, @@ -34,52 +34,6 @@ describe('shortcut registry', () => { */ // ------------------------------------------------ registry chord -> Monaco label - /** - * The key names Monaco's `KeyCodeUtils.toString` prints, where they differ from - * the way the registry spells the key for a human. - */ - const MONACO_KEY_NAMES: Record = { - Left: 'LeftArrow', - Right: 'RightArrow', - }; - - /** - * A registry chord as the harness spells the same chord. - * - * The registry is written for a reader (`Mod+Shift+E`); Monaco's decoder emits - * `Shift+Meta+E` on macOS and `Ctrl+Shift+E` elsewhere, in its own modifier - * order. Translating in this direction — registry into harness — rather than - * the other way keeps the registry human-facing, and a mistake here makes - * assertions FAIL rather than pass, because the comparison is against chords - * the real code produced. - */ - function toMonacoLabel(chord: string, mac: boolean): Chord { - return chord - .split(' ') // a chord SEQUENCE ("Mod+K T") is two chords - .map((single) => { - const parts = single.split('+'); - const key = parts.pop()!; - const mods = new Set(parts); - const ctrl = mods.has('Ctrl') || (mods.has('Mod') && !mac); - // A literal `Meta` stays Meta on every platform, the same way a literal - // `Ctrl` does. The registry has no such row — the chords that need this - // spelling are the unadvertised ones named in - // DOCUMENT_CHORDS_NOT_ADVERTISED, which are written in registry syntax so - // the exemption list and the registry read alike. - const meta = mods.has('Meta') || (mods.has('Mod') && mac); - return [ - ctrl && 'Ctrl', - mods.has('Shift') && 'Shift', - mods.has('Alt') && 'Alt', - meta && 'Meta', - MONACO_KEY_NAMES[key] ?? key, - ] - .filter(Boolean) - .join('+'); - }) - .join(' '); - } - test('the chord translator agrees with the harness on chords both already know', () => { // If `toMonacoLabel` were broken every assertion below would fail, not pass — // but a silent disagreement about ONE key name would look like a real bug in @@ -167,10 +121,7 @@ describe('shortcut registry', () => { * decision with a reason, in the shape this repo already uses for * `KNOWN_LAYER_DIVERGENCES`. */ - const NOT_ADVERTISED: Record = { - 'custom-copy': - 'the OS clipboard convention, not an app shortcut; its Ctrl+V twin is a bare addCommand with no id, so listing one without the other is the more confusing half-answer', - }; + const NOT_ADVERTISED: Record = {}; test('every keybinding the editor registers is either advertised or consciously not', () => { const advertised = new Set(SHORTCUTS.filter((entry) => entry.editorAction).map((entry) => entry.id)); @@ -322,18 +273,8 @@ describe('shortcut registry', () => { // HANDLER instead, and the handler is looked up in the component's real // `addCommand` calls with the keybinding evaluated by Monaco's own `KeyMod`. - /** - * Nameless commands the editor binds that are deliberately not advertised. - * - * The two clipboard chords, for the reason `NOT_ADVERTISED` already gives for - * their `custom-copy` twin: they are OS conventions rather than app shortcuts. - * Everything else the editor binds without a name has to be in the panel. - */ - const BARE_NOT_ADVERTISED: Record = { - cutToClipboard: - 'the OS clipboard convention, not an app shortcut; bound only because Monaco leaves cut/paste unbound in a browser', - pasteFromClipboard: 'the same, one key over', - }; + /** Nameless commands the editor binds that are deliberately not advertised, with the reason. */ + const BARE_NOT_ADVERTISED: Record = {}; test('every contextual key the registry advertises is bound to the handler it names', () => { const advertised = SHORTCUTS.filter((entry) => entry.editorCommand); @@ -366,7 +307,7 @@ describe('shortcut registry', () => { unexplained, [], 'these keys do something extra in the editor and the shortcuts panel never mentions it; ' + - 'add a row to SHORTCUTS (group `keys`) or a reason to BARE_NOT_ADVERTISED', + 'add a row to SHORTCUTS or a reason to BARE_NOT_ADVERTISED', ); for (const handler of Object.keys(BARE_NOT_ADVERTISED)) { diff --git a/scripts/shortcuts.test.ts b/scripts/shortcuts.test.ts index 29b30be1..c435b428 100644 --- a/scripts/shortcuts.test.ts +++ b/scripts/shortcuts.test.ts @@ -10,7 +10,7 @@ import type { ViewMode } from '../src/lib/utils/titlebarToolbar.js'; import type { ViewerCommand } from '../src/lib/utils/viewerKeymap.js'; import { tabListMenuItems, tabMenuItems } from '../src/lib/utils/tabContextMenu.js'; import { translations } from './allTranslations.js'; -import { type Chord, documentKeymap, PLATFORMS, viewerCommandCalls } from './keymapHarness.js'; +import { type Chord, documentKeymap, PLATFORMS, toMonacoLabel, viewerCommandCalls } from './keymapHarness.js'; describe('shortcut registry', () => { /* @@ -34,54 +34,6 @@ describe('shortcut registry', () => { * the vim adapter all live in a browser. */ - // ------------------------------------------------ registry chord -> Monaco label - - /** - * The key names Monaco's `KeyCodeUtils.toString` prints, where they differ from - * the way the registry spells the key for a human. - */ - const MONACO_KEY_NAMES: Record = { - Left: 'LeftArrow', - Right: 'RightArrow', - }; - - /** - * A registry chord as the harness spells the same chord. - * - * The registry is written for a reader (`Mod+Shift+E`); Monaco's decoder emits - * `Shift+Meta+E` on macOS and `Ctrl+Shift+E` elsewhere, in its own modifier - * order. Translating in this direction — registry into harness — rather than - * the other way keeps the registry human-facing, and a mistake here makes - * assertions FAIL rather than pass, because the comparison is against chords - * the real code produced. - */ - function toMonacoLabel(chord: string, mac: boolean): Chord { - return chord - .split(' ') // a chord SEQUENCE ("Mod+K T") is two chords - .map((single) => { - const parts = single.split('+'); - const key = parts.pop()!; - const mods = new Set(parts); - const ctrl = mods.has('Ctrl') || (mods.has('Mod') && !mac); - // A literal `Meta` stays Meta on every platform, the same way a literal - // `Ctrl` does. The registry has no such row — the chords that need this - // spelling are the unadvertised ones named in - // DOCUMENT_CHORDS_NOT_ADVERTISED, which are written in registry syntax so - // the exemption list and the registry read alike. - const meta = mods.has('Meta') || (mods.has('Mod') && mac); - return [ - ctrl && 'Ctrl', - mods.has('Shift') && 'Shift', - mods.has('Alt') && 'Alt', - meta && 'Meta', - MONACO_KEY_NAMES[key] ?? key, - ] - .filter(Boolean) - .join('+'); - }) - .join(' '); - } - // ------------------------------------------------------------- sanity guards test('the registry was actually read, and is internally coherent', () => { @@ -110,7 +62,7 @@ describe('shortcut registry', () => { // that implements it, or it cannot be checked and must not be advertised. for (const entry of SHORTCUTS) { assert.ok( - entry.editorAction || entry.editorCommand || entry.documentCommands, + entry.editorAction || entry.editorCommand || entry.documentCommands || entry.monacoCommand, `${entry.id} names no implementation, so nothing here can confirm its chord fires`, ); } diff --git a/src/lib/utils/shortcuts.ts b/src/lib/utils/shortcuts.ts index 6f7ee1e7..ef211ad6 100644 --- a/src/lib/utils/shortcuts.ts +++ b/src/lib/utils/shortcuts.ts @@ -16,12 +16,6 @@ * is fired at the real code and has to run the command it is labelled with. * The registry is not a source of truth about behaviour — the handlers are — * it is a claim about them that a test refuses to let drift. - * - * WHAT IS NOT HERE - * - * The clipboard chords (`Mod+C`, `Mod+V`). They are OS conventions rather than - * app shortcuts, and `Mod+V` is registered as a bare `addCommand` with no id, - * so listing Copy without Paste would be the more confusing half-answer. */ /** @@ -89,14 +83,19 @@ export type ShortcutEntry = { * The name of the handler `Editor.svelte` binds this chord to with a bare * `editor.addCommand` — no action id, no label, no menu entry. * - * The `keys` group needs this because there is no action id to check against: - * a contextual key IS a nameless command. Recording the handler's name keeps - * the row as verifiable as every other one — `shortcuts.test.ts` reads + * Contextual keys and Cut/Paste need this because there is no action id to + * check against. Recording the handler's name keeps the row as verifiable as + * every other one — `shortcuts.editor.test.ts` reads * the component's `addCommand` calls, evaluates the keybinding with Monaco's * own `KeyMod`/`KeyCode`, and requires that this chord be bound to this * function. */ readonly editorCommand?: string; + /** + * Monaco's own command for this chord, which the app leaves alone (Undo, + * Redo). `editor.chords.spec.ts` checks it against Monaco's real keymap. + */ + readonly monacoCommand?: string; /** * The command the document-level dispatcher means by this row's chords when * the editor does not have focus. @@ -224,6 +223,13 @@ export const SHORTCUTS: readonly ShortcutEntry[] = [ }, // ---------------------------------------------------------------- edit + { id: 'edit-undo', labelKey: 'menu.undo', chords: ['Mod+Z'], group: 'edit', monacoCommand: 'undo' }, + // Windows and Linux also take `Mod+Y`. macOS does not, and a row has one + // chord list for every platform, so only the chord all three answer is shown. + { id: 'edit-redo', labelKey: 'menu.redo', chords: ['Mod+Shift+Z'], group: 'edit', monacoCommand: 'redo' }, + { id: 'edit-cut', labelKey: 'menu.cut', chords: ['Mod+X'], group: 'edit', editorCommand: 'cutToClipboard' }, + { id: 'custom-copy', labelKey: 'menu.copy', chords: ['Mod+C'], group: 'edit', editorAction: true }, + { id: 'edit-paste', labelKey: 'menu.paste', chords: ['Mod+V'], group: 'edit', editorCommand: 'pasteFromClipboard' }, { id: 'fmt-bold', labelKey: 'menu.bold', chords: ['Mod+B'], group: 'edit', editorAction: true }, { id: 'fmt-italic', labelKey: 'menu.italic', chords: ['Mod+I'], group: 'edit', editorAction: true }, { id: 'fmt-underline', labelKey: 'menu.underline', chords: ['Mod+U'], group: 'edit', editorAction: true },