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
14 changes: 14 additions & 0 deletions scripts/monacoChordOwnership.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,20 @@ type PerPlatform = Partial<Record<PlatformName, Chord>>;
* `chords` is per platform because Monaco's keymap is not the same on all
* three: Cmd+E collides only on macOS, Ctrl+Shift+R only off it. A row that
* claimed to be universal would hide which command is actually losing its key.
*
* A CHORD THAT CANNOT BE ALLOW-LISTED, recorded here so the next person hunting
* for a free key does not have to rediscover it: `Mod+Alt+C` — `Alt+Meta+C` on
* macOS — is `toggleFindCaseSensitive`, and it fails the palette test above.
* Monaco registers it with `registerEditorCommand`, not `registerEditorAction`,
* and only the latter contributes a `MenuId.CommandPalette` entry
* (`editorExtensions.js`), so there is no palette fallback: taking the key would
* leave find-case-sensitivity reachable by mouse alone. Its `kbExpr` is a bare
* `EditorContextKeys.focus`, so it is live whenever the editor has focus rather
* than only while the find widget is open. Insert Column wanted this chord —
* `Mod+Alt+C` pairs with Insert Table's `Mod+Alt+T` — and got `Mod+Shift+C`
* instead. The same reasoning disqualifies `toggleFindWholeWord` (`Alt+Meta+W`),
* `toggleFindRegex` (`Alt+Meta+R`) and `toggleFindInSelection` (`Alt+Meta+L`),
* which are registered the same way.
*/
type Override = {
/** The app registration: a Monaco action id, or `addCommand(fn)` for a bare command. */
Expand Down
40 changes: 29 additions & 11 deletions scripts/shortcutRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -732,22 +732,40 @@ test('Mod+K belongs to Insert Link, on every platform', () => {
*
* Dropping a CHORD is not dropping a command: `addAction` without `keybindings`
* still puts the verb in the command palette (`Mod+P`, which runs Monaco's
* `editor.action.quickCommand`), which is where a rarely-used destructive edit
* belongs.
* `editor.action.quickCommand`).
*
* WHAT DECIDES IT IS HOW HARD THE EDIT IS BY HAND, not whether it destroys
* anything. This table used to say the opposite — that destructive table verbs
* get no keys because a mis-fire costs too much — and that rule is gone rather
* than sitting here next to the one the code follows: Delete Column destroys
* more than Delete Row does and has a chord (`Mod+Shift+Backspace`), so the old
* rationale could not have been what was being applied. The hazard it named is
* also gone in fact: `Mod+Shift+K` is unbound on this branch and the `Mod+K`
* namespace no longer exists, so the near-neighbour that started all this is not
* there to slip onto.
*
* The four verbs, and what each one is worth a key for:
*
* Insert Row Mod+Enter / Mod+Shift+Enter, and Tab at the last cell
* — the frequent one, so it gets the cheapest keys there are
* Insert Column Mod+Shift+C — mid-frequency, and fiddly by hand
* Delete Column Mod+Shift+Backspace — NO manual fallback: it means editing
* the pipes on every row without slipping once
* Delete Row palette only — trivial by hand. Put the caret on the line,
* select it, delete. A chord buys nothing the keyboard does
* not already do, and an unearned chord is one more key to
* mis-fire.
*
* So the test of a new row here is not "is it dangerous" but "can the user
* already do this without us".
*/
const TABLE_VERBS_WITHOUT_A_CHORD: Record<string, string> = {
'table-insert-row':
'Mod+Enter owns it now — inside a table, "insert a line below" already means "insert a row below"',
'table-delete-row':
'Mod+K Shift+R sat one slip from what was then Monaco\'s Mod+Shift+K (delete line, unbound ' +
'here now), and a mis-fired destructive table edit is much worse than a mis-fired insert',
'table-delete-column': 'the same, one key over',
'table-insert-column':
'it was on Mod+K C, and Mod+K is Insert Link now, so the sequence cannot be typed. Mod+Alt+C ' +
"was the intended replacement and is not available: on macOS it is Monaco's toggleFindCaseSensitive, " +
'live whenever the editor has focus and registered with registerEditorCommand rather than ' +
'registerEditorAction — so it has no command palette entry to fall back on, and taking it would ' +
'leave find-case-sensitivity reachable only by mouse. Palette-only until a chord is chosen',
'trivial by hand — put the caret on the line, select it, delete — so a chord buys nothing the ' +
'keyboard already does. This is the only one of the four with a manual fallback that good, ' +
'which is why it is the only one left without a key',
};

test('the table verbs with no chord are still commands, and advertise nothing', () => {
Expand Down
70 changes: 48 additions & 22 deletions src/lib/components/Editor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -1511,20 +1511,19 @@
// The rest of the table verbs. They are actions rather than bare
// commands because a user has to be able to FIND them — in the command
// palette, in the shortcuts panel — which a nameless `addCommand`
// cannot be. NONE of the four carries a keybinding now, and an action
// with no keybinding is still a command palette entry, which is where a
// verb you reach for twice a year belongs.
// cannot be.
//
// WHY THE DESTRUCTIVE PAIR HAS NO CHORD. They were on `Mod+K Shift+R`
// and `Mod+K Shift+C`, one slip from what was then Monaco's own
// `Mod+Shift+K` — delete line. That neighbour is unbound now (see the
// removal rule in `createEditor`), and the verbs still get no chord: the
// hazard was never only the neighbour. A mis-fired insert is an undo; a
// mis-fired delete of the row you were reading is the kind of thing that
// gets noticed three edits later.
//
// WHY INSERT ROW HAS NO CHORD EITHER: `Mod+Enter` owns it now, one
// keystroke instead of two, and Tab at the end of the table appends one.
// WHAT DECIDES WHICH ONE GETS A KEY is how hard the edit is by hand,
// not whether it is destructive. Insert Row is on `Mod+Enter` already
// (inside a table, "insert the line below" IS "insert the row below")
// and on Tab at the last cell, so it needs nothing here. Delete Row is
// the one verb left with no chord at all: putting the caret on the line
// and deleting it is the whole operation, so a chord would buy nothing.
// The two column verbs both have chords BECAUSE the manual route is
// editing the pipes on every row of the table without slipping —
// which for delete is not a fallback anyone would actually take.
// The reasoning is set out once, in TABLE_VERBS_WITHOUT_A_CHORD in
// shortcutRegistry.test.ts, and the per-verb notes are at each binding.
editor.addAction({
id: "table-insert-row",
label: t('menu.insertTableRow', lang),
Expand All @@ -1544,15 +1543,20 @@
editor.addAction({
id: "table-insert-column",
label: t('menu.insertTableColumn', lang),
// NO CHORD, for now. It was on `Mod+K C`, which cannot be typed any
// more: `Mod+K` is Insert Link and is no longer a prefix. The intended
// replacement, `Mod+Alt+C`, is not available — on macOS that is
// Monaco's `toggleFindCaseSensitive`, live whenever the editor has
// focus, and registered with `registerEditorCommand` rather than
// `registerEditorAction`, so it has no command palette entry to fall
// back on. Taking it would leave a user no way to toggle case in find
// except the mouse, which is the one thing no override in
// monacoChordOwnership.spec.ts is allowed to do.
// C for Column, and one of the "insert a structure" family: it sits
// with Ctrl/Cmd+Shift+7, +8 and +9, which insert the three list
// kinds. Monaco binds nothing on Ctrl/Cmd+Shift+C on any of the three
// platforms, as a whole chord or as a prefix.
//
// It was on `Mod+K C`, which cannot be typed any more: `Mod+K` is
// Insert Link and is no longer a prefix. `Mod+Alt+C` was the obvious
// replacement and had to be refused — see the note above
// DELIBERATE_OVERRIDES in monacoChordOwnership.spec.ts. Palette-only
// was tried first and is too weak for a verb people reach for while
// building a table: Mod+P, type the name, Enter.
keybindings: [
monaco.KeyMod.CtrlCmd | monaco.KeyMod.Shift | monaco.KeyCode.KeyC,
],
run: () => {
editTable("insert-column");
},
Expand All @@ -1561,6 +1565,28 @@
editor.addAction({
id: "table-delete-column",
label: t('menu.deleteTableColumn', lang),
// WHY THIS VERB GETS A KEY WHEN DELETE ROW DOES NOT: there is no way
// to do it by hand. Deleting a row is selecting a line and pressing
// delete; deleting a column means editing the pipes on every row of
// the table without slipping once. A shortcut for the first buys
// nothing over what the keyboard already does. For the second it is
// the only practical route.
//
// WHY BACKSPACE AND NOT A LETTER: Ctrl/Cmd+Shift+D is free, but D is
// the physical neighbour of the C above — one slip turns "insert a
// column" into "delete a column", which is precisely the shape of the
// complaint that started this rework (`Mod+K Shift+R` sat one slip
// from Monaco's delete-line). A destructive verb does not go next to
// its constructive counterpart. Backspace is across the keyboard and
// already means "remove", so there is no mnemonic to learn.
//
// Monaco binds nothing on Ctrl/Cmd+Shift+Backspace on any platform.
// Its nearest neighbour is Cmd+Backspace (`deleteAllLeft`) one
// modifier away, which is an ordinary undoable edit rather than a
// structural one.
keybindings: [
monaco.KeyMod.CtrlCmd | monaco.KeyMod.Shift | monaco.KeyCode.Backspace,
],
run: () => {
editTable("delete-column");
},
Expand Down
36 changes: 27 additions & 9 deletions src/lib/utils/shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,33 @@ export const SHORTCUTS: readonly ShortcutEntry[] = [
group: 'edit',
editorAction: true,
},
// INSERT COLUMN IS NOT HERE ANY MORE. It was on `Mod+K C`, and `Mod+K` is
// Insert Link now, so the sequence cannot be typed. `Mod+Alt+C` was the
// intended replacement and is not available: on macOS it is Monaco's
// `toggleFindCaseSensitive`, bound whenever the editor has focus, and
// registered as an editor COMMAND rather than an action — so it has no
// command palette entry to fall back on, which is the one thing every
// override in monacoChordOwnership.spec.ts requires. Insert Column is
// palette-only until a chord is chosen for it; see
// TABLE_VERBS_WITHOUT_A_CHORD in shortcutRegistry.test.ts.
{
id: 'table-insert-column',
labelKey: 'menu.insertTableColumn',
// C for Column, and it joins the Mod+Shift+7/8/9 family above: those are all
// "insert a structure", which is what this is.
//
// NOT `Mod+Alt+C`, which was the obvious pick and is not available — on macOS
// that is Monaco's `toggleFindCaseSensitive`. The reason it cannot be taken is
// recorded in monacoChordOwnership.spec.ts, where the next person hunting for
// a free chord will be.
chords: ['Mod+Shift+C'],
group: 'edit',
editorAction: true,
},
{
id: 'table-delete-column',
labelKey: 'menu.deleteTableColumn',
// NOT a letter, and that is the whole point. `Mod+Shift+D` is free, but D is
// the physical neighbour of C — one slip would turn "insert a column" into
// "delete a column". A destructive verb must not sit next to its constructive
// counterpart; that is the defect the old `Mod+K Shift+R` had, one slip from
// Monaco's delete-line. Backspace is across the keyboard and already means
// "remove" everywhere, so it carries no mnemonic to learn.
chords: ['Mod+Shift+Backspace'],
group: 'edit',
editorAction: true,
},

// ---------------------------------------------------------------- keys
//
Expand Down
Loading