Skip to content

feat(shortcuts): list Undo, Redo, Cut, Copy and Paste in the shortcuts panel #1049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PathGao merged 1 commit into from
Oct 29, 2026
Merged

feat(shortcuts): list Undo, Redo, Cut, Copy and Paste in the shortcuts panel #1049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion scripts/editor.chords.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions scripts/keymapHarness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {
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(' ');
}
69 changes: 5 additions & 64 deletions scripts/shortcuts.editor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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<string, string> = {
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
Expand Down Expand Up @@ -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<string, string> = {
'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<string, string> = {};

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));
Expand Down Expand Up @@ -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<string, string> = {
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<string, string> = {};

test('every contextual key the registry advertises is bound to the handler it names', () => {
const advertised = SHORTCUTS.filter((entry) => entry.editorCommand);
Expand Down Expand Up @@ -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)) {
Expand Down
52 changes: 2 additions & 50 deletions scripts/shortcuts.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
/*
Expand All @@ -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<string, string> = {
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', () => {
Expand Down Expand Up @@ -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`,
);
}
Expand Down
24 changes: 15 additions & 9 deletions src/lib/utils/shortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 },
Expand Down
Loading