[Keybinding] Normalize key sequences to US layout

Some bindings, i.e. 'Toggle Line Comment' or 'Open Terminal' are bound to a sequence that are not directly reachable on many non-US keyboard layouts. For instance 'ctrlcmd+/' to toggle line comments doesn't work on german kb layouts, because the slash is above the `7` and therefore the shortcut is seen as `cmdctrl+shift+7` This change translates the given chord to a normalized US keysequence if the character is one of the US layout keys. I.e. for `cmdctrl+shift+7` we also know the typed character is `/` so if there is no direct keybinding we test against a US-keyboard layouted version. Fixes #1244 Signed-off-by: Sven Efftinge <[email protected]>
eclipse-theia · Dec 14, 2018 · ac9c215 · ac9c215
1 parent f6ee123
commit ac9c215
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 21 deletions.
diff --git a/packages/core/src/browser/keybinding.spec.ts b/packages/core/src/browser/keybinding.spec.ts
@@ -497,18 +497,23 @@ describe('keys api', () => {
     });
 
     it('it should be a modifier only', () => {
-
         const keyCode = KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd] });
         expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd] }));
         expect(keyCode.isModifierOnly()).to.be.true;
     });
 
     it('it should be multiple modifiers only', () => {
-
         const keyCode = KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd, KeyModifier.Alt] });
         expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd, KeyModifier.Alt] }));
         expect(keyCode.isModifierOnly()).to.be.true;
     });
+
+    it('it should translate non US layout chords properly', () => {
+        // mimic a german layout, i.e. the '/' is on the 'Shift+7'.
+        const keyCode = new KeyCode(KeyCode.parse('ctrlcmd+shift+7').keystroke, '/');
+        const normalized = keyCode.normalizeToUsLayout();
+        expect(normalized).to.be.deep.equal(KeyCode.parse('ctrlcmd+/'));
+    });
 });
 
 const TEST_COMMAND: Command = {

diff --git a/packages/core/src/browser/keybinding.ts b/packages/core/src/browser/keybinding.ts
@@ -290,27 +290,39 @@ export class KeybindingRegistry {
     protected getKeySequenceCollisions(bindings: Keybinding[], keySequence: KeyCode[]): KeybindingRegistry.KeybindingsResult {
         const result = new KeybindingRegistry.KeybindingsResult();
 
-        for (const registeredBinding of bindings) {
-            try {
-                const bindingKeySequence = KeySequence.parse(registeredBinding.keybinding);
-                const compareResult = KeySequence.compare(keySequence, bindingKeySequence);
-                switch (compareResult) {
-                    case KeySequence.CompareResult.FULL: {
-                        result.full.push(registeredBinding);
-                        break;
-                    }
-                    case KeySequence.CompareResult.PARTIAL: {
-                        result.partial.push(registeredBinding);
-                        break;
-                    }
-                    case KeySequence.CompareResult.SHADOW: {
-                        result.shadow.push(registeredBinding);
-                        break;
-                    }
-                    default: {
-                        break;
+        /**
+         * compare the given KeySequence with a particular binding
+         */
+        function compareBinding(candidate: KeyCode[], binding: Keybinding) {
+            const bindingKeySequence = KeySequence.parse(binding.keybinding);
+            const compareResult = KeySequence.compare(candidate, bindingKeySequence);
+            switch (compareResult) {
+                case KeySequence.CompareResult.FULL: {
+                    result.full.push(binding);
+                    break;
+                }
+                case KeySequence.CompareResult.PARTIAL: {
+                    result.partial.push(binding);
+                    break;
+                }
+                case KeySequence.CompareResult.SHADOW: {
+                    result.shadow.push(binding);
+                    break;
+                }
+                default: {
+                    // no match. Let's try with a US keborad normalized version if there is one.
+                    const normalizedUs = candidate.map(k => k.normalizeToUsLayout());
+                    if (normalizedUs.indexOf(undefined) === -1) {
+                        compareBinding(normalizedUs as KeyCode[], binding);
                     }
+                    break;
                 }
+            }
+        }
+
+        for (const registeredBinding of bindings) {
+            try {
+                compareBinding(keySequence, registeredBinding);
             } catch (error) {
                 this.logger.warn(error);
             }

diff --git a/packages/core/src/browser/keys.ts b/packages/core/src/browser/keys.ts
@@ -194,6 +194,30 @@ export class KeyCode {
         }
     }
 
+    public normalizeToUsLayout(): KeyCode | undefined {
+        if (this.shift && this.character && EASY_TO_KEY[this.character]) {
+            const modifiers: KeyModifier[] = [];
+            if (isOSX) {
+                if (this.meta) {
+                    modifiers.push(KeyModifier.CtrlCmd);
+                }
+                if (this.ctrl) {
+                    modifiers.push(KeyModifier.MacCtrl);
+                }
+            } else {
+                if (this.ctrl) {
+                    modifiers.push(KeyModifier.CtrlCmd);
+                }
+            }
+            if (this.alt) {
+                modifiers.push(KeyModifier.Alt);
+            }
+
+            return KeyCode.createKeyCode(<Keystroke>{ first: EASY_TO_KEY[this.character], modifiers });
+        }
+        return undefined;
+    }
+
     /* Return true of string is a modifier M1 to M4 */
     public static isModifierString(key: string) {
         if (key === KeyModifier.CtrlCmd