diff --git a/.changeset/reject-empty-keychord-segments.md b/.changeset/reject-empty-keychord-segments.md new file mode 100644 index 000000000..a17135b69 --- /dev/null +++ b/.changeset/reject-empty-keychord-segments.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Reject malformed configured and extension key chords instead of silently rebinding them after empty `+` segments. diff --git a/src/extension-api/keys.test.ts b/src/extension-api/keys.test.ts index 0d8812c6a..c9bb301b7 100644 --- a/src/extension-api/keys.test.ts +++ b/src/extension-api/keys.test.ts @@ -87,6 +87,28 @@ describe("parseKeyChord", () => { expect(parseKeyChord("")).toHaveProperty("error"); }); + test("refuses chords with an empty segment around +", () => { + expect(parseKeyChord("s+")).toHaveProperty("error"); + expect(parseKeyChord("+s")).toHaveProperty("error"); + expect(parseKeyChord("ctrl++s")).toHaveProperty("error"); + expect(parseKeyChord("ctrl+s+")).toHaveProperty("error"); + }); + + test("keeps the literal plus key valid, with or without surrounding whitespace", () => { + expect(parsed("+")).toEqual({ + base: "+", + ctrl: false, + meta: false, + option: false, + shift: false, + }); + expect(parsed(" + ")).toEqual(parsed("+")); + }); + + test("tolerates whitespace around otherwise valid chord components", () => { + expect(parsed("ctrl + s")).toEqual(parsed("ctrl+s")); + }); + test("refuses shift on symbols and digits, keeps it for letters and named keys", () => { // Shifted symbols have no layout-independent identity; the binding must // name the character shift produces instead. diff --git a/src/extension-api/keys.ts b/src/extension-api/keys.ts index 50c56adcb..0e98c5687 100644 --- a/src/extension-api/keys.ts +++ b/src/extension-api/keys.ts @@ -70,15 +70,19 @@ const MODIFIER_TOKENS: Record> = { * registration instead of silently never firing. */ export function parseKeyChord(chord: string): ParsedKeyChord | { error: string } { - const tokens = chord - .split("+") - .map((token) => token.trim()) - .filter((token) => token.length > 0); - // A literal "+" binding arrives as empty tokens; treat the lone "+" specially. - if (tokens.length === 0) { - return chord.trim() === "+" - ? { base: "+", ctrl: false, meta: false, option: false, shift: false } - : { error: `Empty key chord "${chord}"` }; + // The literal "+" binding (optionally whitespace-padded) splits into two + // empty segments; recognize it before empty segments are rejected below. + if (chord.trim() === "+") { + return { base: "+", ctrl: false, meta: false, option: false, shift: false }; + } + + if (chord.trim().length === 0) { + return { error: `Empty key chord "${chord}"` }; + } + + const tokens = chord.split("+").map((token) => token.trim()); + if (tokens.some((token) => token.length === 0)) { + return { error: `Key chord "${chord}" has an empty segment around "+"` }; } const parsed: ParsedKeyChord = { diff --git a/src/ui/lib/keymap.test.ts b/src/ui/lib/keymap.test.ts index 7e09b6599..152f64c1e 100644 --- a/src/ui/lib/keymap.test.ts +++ b/src/ui/lib/keymap.test.ts @@ -106,6 +106,16 @@ describe("resolveCommandKeys", () => { expect(issues[0]?.message).toContain("not a usable key chord"); }); + test("a malformed chord with an empty + segment does not claim another command's shortcut", () => { + const { keys, issues } = resolve({ "hunk.app.quit": ["s+", "ctrl+q"] }); + + expect(keys.get("hunk.app.quit")).toEqual(["ctrl+q"]); + // "s+" must not be normalized into "s" and steal it from its default owner. + expect(keys.get("hunk.view.toggleFilesPane")).toEqual(["s"]); + expect(issues).toHaveLength(1); + expect(issues[0]?.message).toContain("not a usable key chord"); + }); + test("unknown ids are reported, softly when they look like extension commands", () => { const { keys, issues } = resolve({ "hunk.app.quti": "x", "ghost.command": "z" });