fixup! Fixes for Vscode emacs extension

packages/core/src/browser/keybinding.spec.ts

@@ -253,7 +253,7 @@ describe('keybindings', () => {
         expect(bindings.partial.length > 0);
     });
 
-    it('should register a shadowing keybinding', () => {
+    it('should not register a shadowing keybinding', () => {
         const validKeyBinding = 'ctrlcmd+b a';
         const command = TEST_COMMAND_SHADOW.id;
         const keybindingShadowing: Keybinding[] = [
@@ -270,22 +270,22 @@ describe('keybindings', () => {
         keybindingRegistry.registerKeybindings(...keybindingShadowing);
 
         const bindings = keybindingRegistry.getKeybindingsForCommand(command);
-        expect(bindings.length).to.be.equal(2);
+        expect(bindings.length).to.be.equal(1);
         expect(bindings[0].keybinding).to.be.equal(validKeyBinding);
     });
 
-    it('shadowed bindings should also be returned', () => {
+    it('shadowed bindings should not be returned', () => {
         const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift] });
         let bindings: Keybinding[];
 
-        const defaultBinding: Keybinding = {
+        const ignoredDefaultBinding: Keybinding = {
             keybinding: keyCode.toString(),
-            command: 'test.workspace-command'
+            command: 'test.ignored-command'
         };
 
-        const secondDefaultBinding: Keybinding = {
+        const defaultBinding: Keybinding = {
             keybinding: keyCode.toString(),
-            command: 'test.second-command'
+            command: 'test.workspace-command'
         };
 
         const userBinding: Keybinding = {
@@ -298,7 +298,7 @@ describe('keybindings', () => {
             command: 'test.workspace-command'
         };
 
-        keybindingRegistry.setKeymap(KeybindingScope.DEFAULT, [defaultBinding, secondDefaultBinding]);
+        keybindingRegistry.setKeymap(KeybindingScope.DEFAULT, [defaultBinding, ignoredDefaultBinding]);
         keybindingRegistry.setKeymap(KeybindingScope.USER, [userBinding]);
         keybindingRegistry.setKeymap(KeybindingScope.WORKSPACE, [workspaceBinding]);
         // now WORKSPACE bindings are overriding the other scopes
@@ -318,9 +318,8 @@ describe('keybindings', () => {
         // and finally it should fallback to DEFAULT bindings.
 
         bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full;
-        expect(bindings).to.have.lengthOf(2);
+        expect(bindings).to.have.lengthOf(1);
         expect(bindings[0].command).to.be.equal(defaultBinding.command);
-        expect(bindings[1].command).to.be.equal(secondDefaultBinding.command);
 
         keybindingRegistry.resetKeybindingsForScope(KeybindingScope.DEFAULT);
         // now the registry should be empty

packages/core/src/browser/keybinding.ts

@@ -29,6 +29,7 @@ import * as common from '../common/keybinding';
 
 export enum KeybindingScope {
     DEFAULT,
+    PLUGIN,
     USER,
     WORKSPACE,
     END
@@ -188,12 +189,13 @@ export class KeybindingRegistry {
     }
 
     /**
-     * Register a default keybinding to the registry.
+     * Register a keybinding to the registry.
      *
      * @param binding
+     * @param scope keybinding scope, {@link KeybindingScope.DEFAULT} if not provided.
      */
-    registerKeybinding(binding: Keybinding): Disposable {
-        return this.doRegisterKeybinding(binding, KeybindingScope.DEFAULT);
+    registerKeybinding(binding: Keybinding, scope: KeybindingScope = KeybindingScope.DEFAULT): Disposable {
+        return this.doRegisterKeybinding(binding, scope);
     }
 
     /**
@@ -241,7 +243,9 @@ export class KeybindingRegistry {
     protected doRegisterKeybinding(binding: Keybinding, scope: KeybindingScope = KeybindingScope.DEFAULT): Disposable {
         try {
             this.resolveKeybinding(binding);
-            this.containsKeybinding(this.keymaps[scope], binding);
+            if (this.containsKeybinding(this.keymaps[scope], binding) && scope !== KeybindingScope.PLUGIN) {
+                throw new Error(`"${binding.keybinding}" is in collision with something else [scope:${scope}]`);
+            }
             this.keymaps[scope].push(binding);
             return Disposable.create(() => {
                 const index = this.keymaps[scope].indexOf(binding);
@@ -292,19 +296,19 @@ export class KeybindingRegistry {
             .filter(b => b.context === binding.context && !b.when && !binding.when);
 
         if (collisions.full.length > 0) {
-            this.logger.warn('Collided keybinding detected; ',
+            this.logger.warn('Collided keybinding is ignored; ',
                 Keybinding.stringify(binding), ' collided with ',
                 collisions.full.map(b => Keybinding.stringify(b)).join(', '));
             return true;
         }
         if (collisions.partial.length > 0) {
-            this.logger.warn('Shadowing keybinding detected; ',
+            this.logger.warn('Shadowing keybinding is ignored; ',
                 Keybinding.stringify(binding), ' shadows ',
                 collisions.partial.map(b => Keybinding.stringify(b)).join(', '));
             return true;
         }
         if (collisions.shadow.length > 0) {
-            this.logger.warn('Shadowed keybinding detected; ',
+            this.logger.warn('Shadowed keybinding is ignored; ',
                 Keybinding.stringify(binding), ' would be shadowed by ',
                 collisions.shadow.map(b => Keybinding.stringify(b)).join(', '));
             return true;
@@ -450,6 +454,10 @@ export class KeybindingRegistry {
             matches.partial = matches.partial.filter(
                 binding => this.getKeybindingCollisions(result.partial, binding).partial.length === 0);
 
+            if (scope === KeybindingScope.PLUGIN) {
+                matches.full.reverse();
+                matches.partial.reverse();
+            }
             result.merge(matches);
         }
         this.sortKeybindingsByPriority(result.full);
@@ -560,8 +568,7 @@ export class KeybindingRegistry {
             return false;
         }
 
-        for (let i = bindings.length - 1; i >= 0; i--) {
-            const binding = bindings[i];
+        for (const binding of bindings) {
             if (this.isEnabled(binding, event)) {
                 if (this.isPseudoCommand(binding.command)) {
                     /* Don't do anything, let the event propagate.  */

packages/plugin-ext/src/main/browser/keybindings/keybindings-contribution-handler.ts

@@ -17,7 +17,7 @@
 import { injectable, inject } from 'inversify';
 import { PluginContribution, Keybinding as PluginKeybinding } from '../../../common';
 import { Keybinding } from '@theia/core/lib/common/keybinding';
-import { KeybindingRegistry } from '@theia/core/lib/browser/keybinding';
+import { KeybindingRegistry, KeybindingScope } from '@theia/core/lib/browser/keybinding';
 import { OS } from '@theia/core/lib/common/os';
 import { Disposable } from '@theia/core/lib/common/disposable';
 import { DisposableCollection } from '@theia/core';
@@ -36,7 +36,7 @@ export class KeybindingsContributionPointHandler {
         for (const raw of contributions.keybindings) {
             const keybinding = this.toKeybinding(raw);
             if (keybinding) {
-                toDispose.push(this.keybindingRegistry.registerKeybinding(keybinding));
+                toDispose.push(this.keybindingRegistry.registerKeybinding(keybinding, KeybindingScope.PLUGIN));
             }
         }
         return toDispose;