Bugfix: fallbackBIndings creates duplicate bindings (#41)

The last update lead to bugs with duplicate bindings, this addresses the problem by marking implicitly added bindings (via fallback). In addition this adds a missing command that started leading to problems in my testing: `Deactivate User Keybindings`.
haberdashPI · Oct 14, 2024 · ed363c2 · ed363c2
1 parent 34a4b6a
commit ed363c2
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 16 deletions.
diff --git a/package.json b/package.json
@@ -3,7 +3,7 @@
   "displayName": "Master Key",
   "publisher": "haberdashPI",
   "description": "Master your keybindings with documentation, discoverability, modal bindings, macros and expressive configuration",
-  "version": "0.3.9",
+  "version": "0.3.8",
   "icon": "logo.png",
   "repository": {
     "url": "https://github.com/haberdashPi/vscode-master-key"
@@ -283,4 +283,4 @@
     "zod": "^3.22.4",
     "zod-validation-error": "^2.1.0"
   }
-}
+}
diff --git a/src/web/keybindings/config.ts b/src/web/keybindings/config.ts
@@ -40,6 +40,22 @@ function fromZip64(str: string): string {
     return result || '';
 }
 
+export async function clearUserBindings() {
+    if (configState) {
+        const config = vscode.workspace.getConfiguration('master-key');
+        const storage = config.get<IStorage>('storage') || {};
+        storage.userBindings = undefined;
+        config.update('storage', storage, vscode.ConfigurationTarget.Global);
+        const newBindings: string = fromZip64(storage.presetBindings || '');
+        const newParsedBindings = processParsing(await parseBindings(newBindings));
+        if (newParsedBindings) {
+            bindings = newParsedBindings;
+            return newParsedBindings;
+        }
+    }
+    return undefined;
+}
+
 export async function createUserBindings(
     userBindings: string
 ): Promise<Bindings | undefined> {

diff --git a/src/web/keybindings/index.ts b/src/web/keybindings/index.ts
@@ -8,13 +8,13 @@ import {
     ParsedResult,
     ErrorResult,
 } from './parsing';
-import * as config from './config';
 import {processBindings, IConfigKeyBinding, Bindings} from './processing';
 import {isSingleCommand} from '../utils';
 import {uniq, pick} from 'lodash';
 import replaceAll from 'string.prototype.replaceall';
 import {Utils} from 'vscode-uri';
-import {createBindings, createUserBindings} from './config';
+import {clearUserBindings, createBindings, createUserBindings} from './config';
+import * as config from './config';
 const JSONC = require('jsonc-simple-parser');
 const TOML = require('smol-toml');
 
@@ -552,6 +552,13 @@ async function activateBindings(preset?: Preset) {
     }
 }
 
+async function deleteUserBindings() {
+    const bindings = await clearUserBindings();
+    if (bindings) {
+        insertKeybindingsIntoConfig(bindings);
+    }
+}
+
 async function selectUserBindings(file?: vscode.Uri) {
     if (!file) {
         const currentUri = vscode.window.activeTextEditor?.document.fileName;
@@ -649,6 +656,9 @@ export async function activate(context: vscode.ExtensionContext) {
     context.subscriptions.push(
         vscode.commands.registerCommand('master-key.selectUserBindings', selectUserBindings)
     );
+    context.subscriptions.push(
+        vscode.commands.registerCommand('master-key.removeUserBindings', deleteUserBindings)
+    );
     context.subscriptions.push(
         vscode.commands.registerCommand('master-key.editPreset', copyBindingsToNewFile)
     );

diff --git a/src/web/keybindings/parsing.ts b/src/web/keybindings/parsing.ts
@@ -287,7 +287,11 @@ export const bindingItem = z
         key: bindingKey,
         when: parsedWhen.array(),
         command: z.literal('master-key.do'),
-        mode: z.string().array().optional(),
+        mode: z
+            .string()
+            .or(z.object({implicit: z.string()}))
+            .array()
+            .optional(),
         prefixes: z.string().array().optional().default(['']),
         args: z
             .object({

diff --git a/src/web/keybindings/processing.ts b/src/web/keybindings/processing.ts
@@ -25,6 +25,7 @@ import {
     mapValues,
     merge,
     assignWith,
+    uniq,
 } from 'lodash';
 import {reifyStrings, EvalContext} from '../expressions';
 import {isSingleCommand, validateInput, IIndexed} from '../utils';
@@ -53,6 +54,7 @@ export function processBindings(spec: FullBindingSpec): [Bindings, string[]] {
     const r = expandKeySequencesAndResolveDuplicates(items, problems);
     items = r[0];
     const prefixCodes = r[1];
+    items = items.map(makeModesExplicit);
     items = items.map(moveModeToWhenClause);
     const newItems = items.map(i => movePrefixesToWhenClause(i, prefixCodes));
     const definitions = {...spec.define, prefixCodes: prefixCodes.codes};
@@ -481,7 +483,8 @@ function expandModes(
         }
     }
     return flatMap(items, (item: BindingItem): BindingItem[] => {
-        let itemModes = item.mode || [defaultMode.name];
+        // NOTE: we know there are no implicit modes inserted before this function is called
+        let itemModes = <string[]>(item.mode || [defaultMode.name]);
         if (itemModes.length > 0 && itemModes[0].startsWith('!')) {
             if (itemModes.some(x => !x.startsWith('!'))) {
                 problems.push(
@@ -497,19 +500,21 @@ function expandModes(
         }
 
         // add modes that are implicitly present due to fallbacks
-        const implicitModes: string[] = [];
+        const implicitModes: (string | {implicit: string})[] = [];
         for (const mode of itemModes) {
             const implicitMode = fallbacks[mode];
             if (implicitMode) {
-                implicitModes.push(implicitMode);
+                implicitModes.push({implicit: implicitMode});
             }
         }
-        itemModes = itemModes.concat(implicitModes);
+        const finalModes = uniq(
+            (<(string | {implicit: string})[]>itemModes).concat(implicitModes)
+        );
 
-        if (itemModes.length === 0) {
+        if (finalModes.length === 0) {
             return [item];
         } else {
-            return itemModes.map(m => ({...item, mode: [m]}));
+            return finalModes.map(m => ({...cloneDeep(item), mode: [m]}));
         }
     });
 }
@@ -574,6 +579,13 @@ export interface IConfigKeyBinding {
     };
 }
 
+function makeModesExplicit(binding: BindingItem) {
+    return {
+        ...binding,
+        mode: binding.mode?.map(m => (<{implicit: string}>m)?.implicit || m),
+    };
+}
+
 function itemToConfigBinding(
     item: BindingItem,
     defs: Record<string, unknown>
@@ -592,7 +604,9 @@ function itemToConfigBinding(
             ...item.args,
             prefixCode:
                 item.prefixes.length > 0 ? prefixCodes[item.prefixes[0]] : undefined,
-            mode: item.mode && item.mode.length > 0 ? item.mode[0] : undefined,
+            // NOTE: because we called makeModesExplicit before calling
+            // `itemToConfigBinding` we can safley assume the elements are strings
+            mode: item.mode && item.mode.length > 0 ? <string>item.mode[0] : undefined,
             key: <string>item.key,
         },
     };
@@ -802,6 +816,10 @@ function expandKeySequencesAndResolveDuplicates(
     return [sortedResult.map(i => i.item), prefixCodes];
 }
 
+function modeIsImplicit(mode: (string | {implicit: string})[] | undefined) {
+    return (<{implicit: string}>(mode || [])[0])?.implicit;
+}
+
 function addWithoutDuplicating(
     map: BindingMap,
     index: number,
@@ -837,6 +855,13 @@ function addWithoutDuplicating(
                 map[key] = {item: newItem, index};
                 return map;
             }
+        } else if (modeIsImplicit(newItem.mode)) {
+            // use existing item
+            return map;
+        } else if (modeIsImplicit(existingItem.mode)) {
+            // use existing item
+            map[key] = {item: newItem, index};
+            return map;
         }
 
         // else: we have two conflicting items

diff --git a/test/specs/config.ux.mts b/test/specs/config.ux.mts
@@ -55,6 +55,13 @@ describe('Configuration', () => {
             key = "ctrl+l"
             args.to = "right"
 
+            [[bind]]
+            path = "motion"
+            name = "double right"
+            key = "ctrl+shift+l"
+            args.to = "right"
+            computedArgs.value = 2
+
             [[bind]]
             name = "insert"
             key = "ctrl+i"
@@ -79,6 +86,14 @@ describe('Configuration', () => {
             when = "editorTextFocus"
             command = "cursorMove"
             args.to = "left"
+
+            [[bind]]
+            path = "motion"
+            name = "double left"
+            mode = "normal-left"
+            key = "ctrl+shift+l"
+            args.to = "left"
+            computedArgs.value = 2
         `);
 
         folder = fs.mkdtempSync(path.join(os.tmpdir(), 'master-key-test-'));
@@ -174,13 +189,25 @@ describe('Configuration', () => {
     });
 
     it('Can add fallback bindings', async () => {
-        await editor.moveCursor(1, 1);
         await enterModalKeys('escape');
         editor = await setupEditor('A simple test');
+        await editor.moveCursor(1, 1);
+        await movesCursorInEditor(
+            () => enterModalKeys(['ctrl', 'shift', 'l']),
+            [0, 2],
+            editor
+        );
         await enterModalKeys(['ctrl', 'u']);
         await waitForMode('normal-left');
         await movesCursorInEditor(() => enterModalKeys(['ctrl', 'l']), [0, 1], editor);
         await movesCursorInEditor(() => enterModalKeys(['ctrl', 'h']), [0, -1], editor);
+        await movesCursorInEditor(() => enterModalKeys(['ctrl', 'l']), [0, 1], editor);
+        await movesCursorInEditor(() => enterModalKeys(['ctrl', 'l']), [0, 1], editor);
+        await movesCursorInEditor(
+            () => enterModalKeys(['ctrl', 'shift', 'l']),
+            [0, -2],
+            editor
+        );
     });
 
     it('Can be loaded from a directory', async () => {
@@ -246,8 +273,7 @@ describe('Configuration', () => {
         expect(modeItem).toBeTruthy();
     });
 
-    // eslint-disable-next-line no-restricted-properties
-    it('Can add user bindings', async () => {
+    it('Can add and remove user bindings', async () => {
         editor = await setupEditor('A simple test');
         const userFile = `
             [[bind]]
@@ -265,8 +291,11 @@ describe('Configuration', () => {
         const editorView = await workbench.getEditorView();
         const keyEditor = (await editorView.openEditor('keybindings.json')) as TextEditor;
         const keyText = await keyEditor.getText();
-        console.log('[DEBUG]: key text — ' + keyText);
         expect(keyText).toMatch(/"ctrl\+shift\+k"/);
+
+        await workbench.executeCommand('Master Key: Deactivate User Keybindings');
+        const removedKeyText = await keyEditor.getText();
+        expect(removedKeyText).not.toMatch(/"ctrl\+shift\+k"/);
     });
 
     it('Can be removed', async () => {