Skip to content

Commit f8e6a53

Browse files
fix(hotkeys): make copy line up/down hotkeys editable
Copy line(s) up/down were listed in the Keyboard Shortcuts panel but marked `editable: false`, because they were served by CodeMirror's built-in `defaultKeymap` and so never consulted marimo's hotkey config. Route both commands through the existing override mechanism already used by `toggleComment`/`toggleBlockComment`: add them to OVERRIDDEN_COMMANDS so the hardcoded bindings are filtered out of the default keymap, then re-bind them in `overrideKeymap` using the configured hotkey. Default keybindings are unchanged. Closes #7056
1 parent c1bccc9 commit f8e6a53

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

frontend/src/core/codemirror/keymaps/__tests__/keymaps.test.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
/* Copyright 2026 Marimo. All rights reserved. */
22

3-
import { defaultKeymap as originalDefaultKeymap } from "@codemirror/commands";
3+
import {
4+
copyLineDown,
5+
copyLineUp,
6+
defaultKeymap as originalDefaultKeymap,
7+
} from "@codemirror/commands";
48
import { describe, expect, it } from "vitest";
5-
import { HotkeyProvider } from "@/core/hotkeys/hotkeys";
9+
import {
10+
HotkeyProvider,
11+
OverridingHotkeyProvider,
12+
} from "@/core/hotkeys/hotkeys";
613
import { visibleForTesting } from "../keymaps";
714

815
const { defaultKeymap, defaultVimKeymap, overrideKeymap, OVERRIDDEN_COMMANDS } =
@@ -40,4 +47,26 @@ describe("keymaps", () => {
4047
expect(keys.some((k) => k.run === command)).toBe(true);
4148
}
4249
});
50+
51+
it.each([
52+
["cell.copyLineUp", copyLineUp, "Alt-Shift-ArrowUp"],
53+
["cell.copyLineDown", copyLineDown, "Alt-Shift-ArrowDown"],
54+
] as const)(
55+
"%s is bound to the configured hotkey, not CodeMirror's default",
56+
(action, command, defaultKey) => {
57+
// Without an override, the binding matches CodeMirror's default key.
58+
expect(
59+
overrideKeymap(HotkeyProvider.create()).find((k) => k.run === command)
60+
?.key,
61+
).toBe(defaultKey);
62+
63+
// With an override, the binding follows the user's configured key.
64+
// This is what `editable: true` on these hotkeys promises.
65+
expect(
66+
overrideKeymap(
67+
new OverridingHotkeyProvider({ [action]: "Ctrl-d" }),
68+
).find((k) => k.run === command)?.key,
69+
).toBe("Ctrl-d");
70+
},
71+
);
4372
});

frontend/src/core/codemirror/keymaps/keymaps.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
/* Copyright 2026 Marimo. All rights reserved. */
22

33
import {
4+
copyLineDown,
5+
copyLineUp,
46
cursorCharLeft,
57
cursorCharRight,
68
cursorLineDown,
@@ -94,6 +96,8 @@ export function keymapBundle(
9496
const OVERRIDDEN_COMMANDS = new Set<Command | undefined>([
9597
toggleComment,
9698
toggleBlockComment,
99+
copyLineUp,
100+
copyLineDown,
97101
]);
98102

99103
const defaultKeymap = once(() => {
@@ -111,6 +115,14 @@ const overrideKeymap = (keymap: HotkeyProvider): readonly KeyBinding[] => {
111115
key: keymap.getHotkey("cell.toggleBlockComment").key,
112116
run: toggleBlockComment,
113117
},
118+
{
119+
key: keymap.getHotkey("cell.copyLineUp").key,
120+
run: copyLineUp,
121+
},
122+
{
123+
key: keymap.getHotkey("cell.copyLineDown").key,
124+
run: copyLineDown,
125+
},
114126
];
115127
};
116128

frontend/src/core/hotkeys/hotkeys.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,11 @@ const DEFAULT_HOT_KEY = {
256256
name: "Copy line(s) up",
257257
group: "Editing",
258258
key: "Alt-Shift-ArrowUp",
259-
editable: false,
260259
},
261260
"cell.copyLineDown": {
262261
name: "Copy line(s) down",
263262
group: "Editing",
264263
key: "Alt-Shift-ArrowDown",
265-
editable: false,
266264
},
267265

268266
// Markdown

0 commit comments

Comments
 (0)