fix(vim): let mapleader shadow built-in keys like <Space> and , (#2337) (#2355)

* fix(vim): let mapleader shadow built-in keys like <Space> and , (#2337)

codemirror-vim prefers a full match over a partial one, so a user mapping
that starts with <Space> or , never fires: the built-in <Space>->l and
,->repeatLastCharacterSearch commands run first and clear the key buffer.

While an rc mapping begins with a key a built-in uses, unmap the built-in;
restore it on the next applyRcCommands pass so mapclear()'s user/default
boundary stays honest (Vim.unmap does not update defaultKeymapLength).

* fix(vim): pass the context arg Vim.unmap's typings require (#2337)

src/features/vim/__tests__/vim.test.ts

@@ -156,6 +156,35 @@ describe("Vim engine", () => {
     view.destroy();
   });
 
+  it.each([
+    ['let mapleader = " "', ["<Space>", "f", "f"]],
+    ['let mapleader = ","', [",", "f", "f"]],
+    ['let mapleader = "\\\\"', ["\\", "f", "f"]],
+  ])("runs a <leader> mapping whose first key shadows a built-in (%s)", async (leader, keys) => {
+    const ran: string[] = [];
+    const d = registerAction({ id: "navigate.gotoFile", title: "x", run: () => void ran.push("navigate.gotoFile") });
+    const view = makeView();
+    const cm = getCM(view)!;
+    applyRcCommands(parseVimrc(`${leader}\nnmap <leader>ff <Action>(GotoFile)`).commands);
+    for (const k of keys) Vim.handleKey(cm as never, k, "user");
+    await tick();
+    expect(ran).toEqual(["navigate.gotoFile"]);
+    view.destroy();
+    d();
+  });
+
+  it("restores the built-in key the leader shadowed once the rc stops using it (#2337)", async () => {
+    const view = makeView();
+    const cm = getCM(view)!;
+    applyRcCommands(parseVimrc('let mapleader = " "\nnmap <leader>ff <Action>(GotoFile)').commands);
+    // A later rc without the space leader puts <Space> → l back.
+    applyRcCommands(parseVimrc("nmap gd <Action>(GotoDeclaration)").commands);
+    Vim.handleKey(cm as never, "<Space>", "user");
+    await tick();
+    expect(view.state.selection.main.head).toBe(1);
+    view.destroy();
+  });
+
   it("keeps an IME composition out of the document in Normal mode", async () => {
     const transient: boolean[] = [];
     const view = makeView([

src/features/vim/engine.ts

@@ -302,11 +302,58 @@ export function defineExCommands() {
   Vim.defineEx("action", "action", (_cm, params) => void runIdeAction(params.args?.[0]));
 }
 
+/**
+ * The first key of a mapping's lhs — `<Space>` / `,` / `x` and so on. Vim's own
+ * single-key built-ins occupy most of these: codemirror-vim prefers a full
+ * match over a partial one, so `<Space>` → `l` or `,` → repeat-find would fire
+ * before a `<leader>…` sequence can complete (#2337). The fix is to unmap the
+ * built-in while a user mapping starts with that key.
+ *
+ * `Vim.unmap` splices the built-in out of the default keymap. To undo that when
+ * the rc file stops using the key, the removed default entry is re-added via
+ * `_mapCommand` on the next apply (it lands in the user region and mapclear
+ * drops it, so re-adding each round is idempotent).
+ */
+const LEADER_BUILTIN_DEFAULTS: Record<string, Record<string, unknown>> = {
+  "<Space>": { keys: "<Space>", type: "keyToKey", toKeys: "l" },
+  "<S-Space>": { keys: "<S-Space>", type: "keyToKey", toKeys: "w" },
+  "<C-Space>": { keys: "<C-Space>", type: "keyToKey", toKeys: "W" },
+  "<BS>": { keys: "<BS>", type: "keyToKey", toKeys: "h" },
+  "<S-BS>": { keys: "<S-BS>", type: "keyToKey", toKeys: "b" },
+  "<C-BS>": { keys: "<C-BS>", type: "keyToKey", toKeys: "B" },
+  "<Del>": { keys: "<Del>", type: "keyToKey", toKeys: "x" },
+  ",": { keys: ",", type: "motion", motion: "repeatLastCharacterSearch", motionArgs: { forward: false } },
+  ";": { keys: ";", type: "motion", motion: "repeatLastCharacterSearch", motionArgs: { forward: true } },
+};
+
+/** Built-ins unmapped to make room for a user mapping's first key, by key. */
+const shadowedBuiltins = new Map<string, Record<string, unknown> | null>();
+
+function firstKey(lhs: string): string {
+  return /^<[CSMA]-*\w+>|^./i.exec(lhs)?.[0] ?? lhs;
+}
+
 /** Applies rc commands (replacing earlier rc mappings). Returns problems to report. */
 export function applyRcCommands(commands: readonly RcCommand[]): string[] {
   defineExCommands();
   Vim.mapclear();
   const problems: string[] = [];
+  // Restore the built-ins the previous rc shadowed, then unmap whatever this
+  // one's first keys collide with. Restoring first keeps mapclear's
+  // user/default boundary honest — it assumes the default region never shrank.
+  for (const [key, entry] of shadowedBuiltins) {
+    if (entry) Vim._mapCommand(entry as never);
+    shadowedBuiltins.delete(key);
+  }
+
+  for (const c of commands) {
+    if (c.kind !== "map" && c.kind !== "mapEx") continue;
+    const key = firstKey(c.lhs);
+    // The builtins this shadows are context-less; `undefined` matches them.
+    if (!shadowedBuiltins.has(key) && Vim.unmap(key, undefined as unknown as string)) {
+      shadowedBuiltins.set(key, LEADER_BUILTIN_DEFAULTS[key] ?? null);
+    }
+  }
   for (const c of commands) {
     try {
       if (c.kind === "set") {