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 },