Skip to content
Merged
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
74 changes: 74 additions & 0 deletions scripts/shortcutRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,80 @@ test('the panel renders the platform modifier the user is on', () => {
assert.equal(shortcutLabel('no-such-command', 'Cmd'), undefined);
});

/**
* What each modifier the registry writes is CALLED on each platform.
*
* `Mod` was the only one `formatChord` ever substituted, and its docblock said
* everything else "is already the literal the key carries". That was false for
* `Alt`: Apple keyboards print `⌥ option`, the word "Alt" appears on no modern
* Mac keyboard, and so the panel told Mac users to press a key their hardware
* does not have — `Alt+Left`/`Alt+Right`, the two navigation rows.
*
* The two columns are what stops a repeat. A modifier is listed here with BOTH
* of its names, so a row added later that reaches for a key macOS names
* differently has to say so here, and the test below then requires the panel to
* actually render that difference. A modifier absent from this table fails
* outright rather than defaulting to "same word everywhere", which is the
* assumption that produced the defect.
*/
const MODIFIER_NAMES: Record<string, { macos: string; windows: string }> = {
Mod: { macos: 'Cmd', windows: 'Ctrl' },
// Ctrl and Shift really are spelled the same on both: a Mac keyboard prints
// `control` and `shift`. These two rows are the claim the docblock got right.
Ctrl: { macos: 'Ctrl', windows: 'Ctrl' },
Shift: { macos: 'Shift', windows: 'Shift' },
Alt: { macos: 'Opt', windows: 'Alt' },
};

/**
* The modifiers of a chord: every `+`-separated part except the last, which is
* the key. Handles a chord SEQUENCE (`Mod+K T`) the way the rest of this file
* does, one chord at a time.
*/
function modifiersOf(chord: string): string[] {
return chord.split(' ').flatMap((single) => single.split('+').slice(0, -1));
}

test('every modifier the panel prints is what that platform calls the key', () => {
// Enumerated from SHORTCUTS rather than hand-listed, so a row added later with
// a modifier nobody has classified fails here instead of silently rendering
// its raw registry spelling to a user.
const used = new Set(SHORTCUTS.flatMap((entry: ShortcutEntry) => entry.chords.flatMap(modifiersOf)));
assert.ok(used.size > 2, `the registry uses ${used.size} distinct modifiers`);

assert.deepEqual(
[...used].filter((token) => !(token in MODIFIER_NAMES)),
[],
'nothing says what these keys are called on each platform — add them to MODIFIER_NAMES, ' +
'with the macOS name spelled the way the key is actually printed on a Mac keyboard',
);

for (const entry of SHORTCUTS) {
for (const chord of entry.chords) {
const raw = modifiersOf(chord);
const rendered = {
macos: modifiersOf(formatChord(chord, modifierFor('macos'))),
windows: modifiersOf(formatChord(chord, modifierFor('windows'))),
};
for (const platform of ['macos', 'windows'] as const) {
assert.equal(
rendered[platform].length,
raw.length,
`${entry.id}: rendering "${chord}" for ${platform} changed the shape of the chord`,
);
raw.forEach((token, i) => {
assert.equal(
rendered[platform][i],
MODIFIER_NAMES[token][platform],
`${entry.id}: the panel shows "${formatChord(chord, modifierFor(platform))}" on ${platform}, ` +
`but ${platform} calls that key "${MODIFIER_NAMES[token][platform]}", not "${rendered[platform][i]}"`,
);
});
}
}
}
});

test('the panel’s groups are visibly separated, not run together', () => {
// The five sections stacked with nothing between them, so each heading sat
// flush against the last row of the group above and read as belonging to it —
Expand Down
42 changes: 35 additions & 7 deletions src/lib/utils/shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ export type ShortcutEntry = {
*/
readonly labelKey: string;
/**
* The chords, most-advertised first. `Mod` renders as `Cmd` on macOS and
* `Ctrl` elsewhere; a literal `Ctrl` stays `Ctrl` on every platform, which is
* what tab cycling actually binds.
* The chords, most-advertised first. Written in the spelling the CODE uses,
* which is not always the spelling a user reads: `Mod` renders as `Cmd` on
* macOS and `Ctrl` elsewhere, and `Alt` renders as `Opt` on macOS. A
* literal `Ctrl` stays `Ctrl` on every platform, which is what tab cycling
* actually binds.
*
* Consumers with room for one chord (the app menu, the toolbar tooltip) show
* `chords[0]`; the panel shows all of them.
Expand Down Expand Up @@ -402,13 +404,39 @@ export function modifierFor(platform: string): 'Cmd' | 'Ctrl' {
}

/**
* A chord template as a user should read it on `platform`.
* The words each platform prints on the keys the two platforms name
* differently, keyed by that platform's `Mod` word.
*
* Only `Mod` is substituted. Everything else — `Ctrl`, `Alt`, `F5`, `Shift` —
* is already the literal the key carries.
* `Cmd` IS macOS here, and nothing else produces these two values: they come
* from `modifierFor` above, which is the one place the os type is read. So a
* caller holding a modifier has already told this table which platform it is
* rendering for, even though it never passes a platform.
*
* `Ctrl` and `Shift` are absent on purpose — a Mac keyboard prints `control`
* and `shift`, so those words need no translation. `Alt` is here because it
* needs one: Apple keyboards print `⌥ option` and the word "Alt" appears on no
* modern Mac keyboard.
*/
const PLATFORM_KEY_WORDS = {
Cmd: { Mod: 'Cmd', Alt: 'Opt' },
Ctrl: { Mod: 'Ctrl', Alt: 'Alt' },
} as const;

/**
* A chord template as a user should read it on `modifier`'s platform.
*
* The keys whose NAME differs across platforms are substituted — `Mod` and
* `Alt`, per `PLATFORM_KEY_WORDS`. Everything else — `Ctrl`, `Shift`, `F5`,
* the key itself — is already the literal the key carries on both.
*
* `Alt` used to be in that second list, and that was wrong: the panel advertised
* `Alt+Left` to a Mac user, who has no key called Alt to press. Adding a modifier
* that macOS renames without adding it here fails
* `scripts/shortcutRegistry.test.ts`.
*/
export function formatChord(chord: string, modifier: 'Cmd' | 'Ctrl'): string {
return chord.replace(/\bMod\b/g, modifier);
const words: Record<string, string> = PLATFORM_KEY_WORDS[modifier];
return chord.replace(/\b(?:Mod|Alt)\b/g, (token) => words[token]);
}

const byId = new Map(SHORTCUTS.map((entry) => [entry.id, entry]));
Expand Down
Loading