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..9b1607e11 100644 --- a/src/extension-api/keys.test.ts +++ b/src/extension-api/keys.test.ts @@ -87,6 +87,31 @@ describe("parseKeyChord", () => { expect(parseKeyChord("")).toHaveProperty("error"); }); + test("rejects chords with an empty segment around a plus", () => { + // A missing component around "+" must be refused, not silently + // normalized into a different, real shortcut. + 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 chord valid", () => { + const literalPlus = { + base: "+", + ctrl: false, + meta: false, + option: false, + shift: false, + }; + expect(parsed("+")).toEqual(literalPlus); + expect(parsed(" + ")).toEqual(literalPlus); + }); + + test("tolerates whitespace around valid 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..148d50d8e 100644 --- a/src/extension-api/keys.ts +++ b/src/extension-api/keys.ts @@ -70,15 +70,15 @@ 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}"` }; + // A literal "+" binding splits into empty segments below; recognize it before + // segments are validated, so the intentional plus-key chord stays valid. + if (chord.trim() === "+") { + return { base: "+", ctrl: false, meta: false, option: false, shift: false }; + } + + const tokens = chord.split("+").map((token) => token.trim()); + if (tokens.some((token) => token.length === 0)) { + return { error: `Key chord "${chord}" has an empty component around "+"` }; } const parsed: ParsedKeyChord = { diff --git a/src/ui/lib/keymap.test.ts b/src/ui/lib/keymap.test.ts index 7e09b6599..431ff3021 100644 --- a/src/ui/lib/keymap.test.ts +++ b/src/ui/lib/keymap.test.ts @@ -106,6 +106,14 @@ describe("resolveCommandKeys", () => { expect(issues[0]?.message).toContain("not a usable key chord"); }); + test("a chord with an empty segment is rejected without claiming the real shortcut", () => { + const { keys, issues } = resolve({ "hunk.app.quit": ["s+", "ctrl+q"] }); + + expect(keys.get("hunk.app.quit")).toEqual(["ctrl+q"]); + 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" });