Skip to content

fix(editor): give Mod+Shift+K back to Delete Line #825

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
Sep 22, 2026
Merged

fix(editor): give Mod+Shift+K back to Delete Line #825

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
96 changes: 13 additions & 83 deletions scripts/formatShortcutKeymap.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
import assert from 'node:assert/strict';
import test from 'node:test';

import { KeyCode } from 'monaco-editor/esm/vs/editor/common/standalone/standaloneEnums.js';
import { KeyMod } from 'monaco-editor/esm/vs/editor/common/services/editorBaseApi.js';

import { getEditorToolbarTools } from '../src/lib/utils/editorToolbar.js';
import {
OperatingSystem,
PLATFORMS,
bareCommands,
chordOf,
documentKeymap,
editorKeymap,
registeredActions,
viewerCommandTable,
type Chord,
} from './keymapHarness.js';
import type { ViewerCommand } from '../src/lib/utils/viewerKeymap.js';
import { readRustBackend, readSource, sliceBetween, sliceFrom } from './sourceTree.js';
import { readRustBackend, readSource } from './sourceTree.js';

/*
* Issues #121 (six formatting commands with no shortcut) and #392 (Ctrl+T means
Expand Down Expand Up @@ -269,84 +264,19 @@ test('a chord that both layers answer means the same thing in both', () => {
* so the check is against ALL of Monaco's chords rather than a remembered
* eleven — including Quote's deliberate `inPlaceReplace.down` override, which
* is now argued in that file's allow-list instead of asserted by absence here.
*
* What is left in this file is the other direction, below: a Monaco default the
* app takes OFF the keymap rather than takes over. Nothing of ours claims that
* key, so it is not a chord-ownership question and does not belong in the
* ownership file; both ends of it are read out of the installed Monaco.
*/

// ------------------------------------- a core Monaco binding, taken back off

/**
* The `-`-prefixed rules `Editor.svelte` hands `monaco.editor.addKeybindingRules`,
* evaluated with Monaco's own `KeyMod`/`KeyCode`.
*
* The same trick as the rest of this harness: the numbers come from the real
* enums, so a rule that names the wrong chord cannot look right here.
*/
function removedKeybindings(): Array<{ keybinding: number; command: string }> {
const marker = 'monaco.editor.addKeybindingRules(';
const literal = sliceBetween(readSource('src/lib/components/Editor.svelte'), marker, ');').slice(marker.length);
return new Function('monaco', `return ${literal};`)({ KeyMod, KeyCode });
}

test('Mod+Shift+K deletes no line here, and Delete Line keeps its palette entry', () => {
// WHY THE KEY GOES. `editor.action.deleteLines` is VS Code's convention,
// inherited by standalone Monaco and inherited again by this app, where it
// silently destroys the line the caret is on. None of the mainstream Markdown
// editors — Typora, Obsidian, iA Writer, Bear — binds it: it is a code-editor
// key that leaked into a prose editor, and it was being mis-fired.
//
// WHY IT IS A REMOVAL RULE. Binding the chord to a no-op `addCommand` would
// make the key dead AND leave a fake command sitting in front of Monaco's;
// a rule whose command id starts with `-` is what Monaco itself reads as
// "drop this default", which is the shape a core binding has to be undone in.

// 1. That mechanism, read out of the installed Monaco rather than assumed.
const resolver = readSource(
new URL('../node_modules/monaco-editor/esm/vs/platform/keybinding/common/keybindingResolver.js', import.meta.url),
);
assert.match(
sliceFrom(resolver, 'static handleRemovals('),
/rule\.command\.charAt\(0\) === '-'/,
"Monaco no longer drops defaults by a '-'-prefixed command id",
);

// 2. The default being removed, likewise: both the id and the chord come from
// Monaco's source, so a rename or a re-chord upstream fails here instead of
// leaving behind a rule that quietly matches nothing.
const linesOperations = readSource(
new URL('../node_modules/monaco-editor/esm/vs/editor/contrib/linesOperations/browser/linesOperations.js', import.meta.url),
);
assert.match(
sliceBetween(linesOperations, "id: 'editor.action.deleteLines'", 'run(_accessor'),
/primary: 2048 \/\* KeyMod\.CtrlCmd \*\/ \| 1024 \/\* KeyMod\.Shift \*\/ \| 41 \/\* KeyCode\.KeyK \*\//,
'Monaco binds Delete Line to some other chord now',
// ------------------------------------- a core Monaco binding, left alone

test('Mod+Shift+K deletes a line, as it does in Monaco', () => {
// #652 took this key off as a guard against `Mod+K Shift+R` (Delete Row),
// one slip away. That chord and the whole `Mod+K` namespace are gone, so the
// guard protected nothing and cost users a key they asked for back (#821).
// Nothing of ours may claim the chord either; `monacoChordOwnership.spec.ts`
// holds that side.
assert.doesNotMatch(
readSource('src/lib/components/Editor.svelte'),
/-editor\.action\.deleteLines/,
'Editor.svelte removes Monaco\'s Delete Line key again',
);

// 3. The rule the app registers, and nothing else: this is a keymap subtraction
// with one entry, and a second one arriving unremarked is a decision nobody
// wrote down.
assert.deepEqual(removedKeybindings(), [
{ keybinding: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KeyK, command: '-editor.action.deleteLines' },
]);

// 4. And nothing of the app's took the chord instead. The command stays in the
// command palette — dropping a KEY is not dropping a command, the same rule
// the table delete verbs live under — but the keystroke now does nothing.
for (const platform of PLATFORMS) {
const chord = platform.mac ? 'Shift+Meta+K' : 'Ctrl+Shift+K';
assert.equal(chordOf(removedKeybindings()[0].keybinding, platform.os), chord);
for (const [id, chords] of editorKeymap(platform.mac, platform.os)) {
assert.ok(!chords.includes(chord), `${platform.name}: ${id} binds ${chord}, the chord just unbound`);
}
for (const command of bareCommands()) {
assert.notEqual(
chordOf(command.binding, platform.os),
chord,
`${platform.name}: ${command.handler} binds ${chord}, the chord just unbound`,
);
}
}
});
5 changes: 2 additions & 3 deletions scripts/shortcutRegistry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,8 @@ test('Mod+K belongs to Insert Link, on every platform', () => {
* 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.
* also gone in fact: the `Mod+K` namespace no longer exists, so no table verb
* sits one slip from Monaco's delete-line (`Mod+Shift+K`) any more.
*
* The four verbs, and what each one is worth a key for:
*
Expand Down
27 changes: 0 additions & 27 deletions src/lib/components/Editor.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,28 +851,6 @@
EDITING_KEY_CONTEXT,
);

// ⌘⇧K / Ctrl+Shift+K, TAKEN BACK. It is `editor.action.deleteLines`, VS
// Code's convention inherited by standalone Monaco and inherited again by
// this app — a chord that silently destroys the line the caret is on. This
// is a Markdown editor: Typora, Obsidian, iA Writer and Bear all leave that
// chord alone, and it was being mis-fired here.
//
// A REMOVAL RULE, not a no-op command on the same chord. A `-`-prefixed
// command id is what Monaco's own resolver reads as "drop this default"
// (`KeybindingResolver.handleRemovals`); binding a do-nothing command
// instead would leave the key dead AND leave a fake command sitting in
// front of Monaco's, which is worse than either.
//
// The COMMAND stays: an action with no key is still a command palette
// entry (`Mod+P`), which is where the table delete verbs live for exactly
// the same reason. The key goes, the verb does not.
const removedKeybindings = monaco.editor.addKeybindingRules([
{
keybinding: monaco.KeyMod.CtrlCmd | monaco.KeyMod.Shift | monaco.KeyCode.KeyK,
command: "-editor.action.deleteLines",
},
]);

editorReady = true;

// After the view-state / anchor-line restore above, deliberately: an
Expand All @@ -897,11 +875,6 @@
semanticTokens.dispose();
documentSymbols.dispose();
foldingRanges.dispose();
// The keybinding rules are global to the Monaco module, not to this
// editor, so they are disposed with it rather than left to pile up one
// copy per mount — this component is rebuilt every time a tab goes to
// reading mode and back.
removedKeybindings.dispose();

if (editor && currentTabId) writeEditorPosition(currentTabId);

Expand Down
Loading