WIP more tests

examples/api-tests/src/keybindings.spec.js

@@ -0,0 +1,96 @@
+/********************************************************************************
+ * Copyright (C) 2020 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+// @ts-check
+describe('Keybindings', function () {
+
+    const { assert } = chai;
+
+    const { Disposable, DisposableCollection } = require('@theia/core/lib/common/disposable');
+    const { TerminalService } = require('@theia/terminal/lib/browser/base/terminal-service');
+    const { TerminalCommands } = require('@theia/terminal/lib/browser/terminal-frontend-contribution');
+    const { ApplicationShell } = require('@theia/core/lib/browser/shell/application-shell');
+    const { KeybindingRegistry } = require('@theia/core/lib/browser/keybinding');
+    const { CommandRegistry } = require('@theia/core/lib/common/command');
+    const { Deferred } = require('@theia/core/lib/common/promise-util');
+    const { Key } = require('@theia/core/lib/browser/keys');
+    const { EditorManager } = require('@theia/editor/lib/browser/editor-manager');
+    const Uri = require('@theia/core/lib/common/uri');
+    const { WorkspaceService } = require('@theia/workspace/lib/browser/workspace-service');
+    const { MonacoEditor } = require('@theia/monaco/lib/browser/monaco-editor');
+
+    /** @type {import('inversify').Container} */
+    const container = window['theia'].container;
+    /** @type {import('@theia/terminal/lib/browser/base/terminal-service').TerminalService} */
+    const terminalService = container.get(TerminalService);
+    const applicationShell = container.get(ApplicationShell);
+    const keybindings = container.get(KeybindingRegistry);
+    const commands = container.get(CommandRegistry);
+    const editorManager = container.get(EditorManager);
+    const workspaceService = container.get(WorkspaceService);
+
+    const toTearDown = new DisposableCollection();
+    afterEach(() => toTearDown.dispose());
+
+    it('partial keybinding should not override full in the same scope', async () => {
+        const terminal = /** @type {import('@theia/terminal/lib/browser/terminal-widget-impl').TerminalWidgetImpl} */
+            (await terminalService.newTerminal({}));
+        toTearDown.push(Disposable.create(() => terminal.dispose()));
+        terminalService.open(terminal, { mode: 'activate' });
+        await applicationShell.waitForActivation(terminal.id);
+        const waitForCommand = new Deferred();
+        toTearDown.push(commands.onWillExecuteCommand(e => waitForCommand.resolve(e.commandId)));
+        keybindings.dispatchKeyDown({
+            code: Key.KEY_K.code,
+            metaKey: true,
+            ctrlKey: true
+        }, terminal.node);
+        const executedCommand = await waitForCommand.promise;
+        assert.equal(executedCommand, TerminalCommands.TERMINAL_CLEAR.id);
+    });
+
+    it("disabled keybinding should not override enabled", async () => {
+        const id = '__test:keybindings.left';
+        toTearDown.push(commands.registerCommand({ id }, {
+            execute: () => { }
+        }));
+        toTearDown.push(keybindings.registerKeybinding({
+            command: '__test:keybindings.left',
+            keybinding: 'left',
+            when: 'false'
+        }, true));
+
+        const editor = await editorManager.open(new Uri.default(workspaceService.tryGetRoots()[0].uri).resolve('package.json'), {
+            mode: 'activate',
+            selection: {
+                start: {
+                    line: 0,
+                    character: 1
+                }
+            }
+        });
+        toTearDown.push(editor);
+
+        const waitForCommand = new Deferred();
+        toTearDown.push(commands.onWillExecuteCommand(e => waitForCommand.resolve(e.commandId)));
+        keybindings.dispatchKeyDown({
+            code: Key.ARROW_LEFT.code
+        }, editor.node);
+        const executedCommand = await waitForCommand.promise;
+        assert.notEqual(executedCommand, id);
+    });
+
+});

examples/browser/package.json

@@ -60,7 +60,7 @@
     "watch": "yarn build --watch",
     "start": "theia start --plugins=local-dir:../../plugins",
     "start:debug": "yarn start --log-level=debug",
-    "test": "theia test . --test-spec=../api-tests/**/*.spec.js",
+    "test": "theia test . --plugins=local-dir:../../plugins --test-spec=../api-tests/**/*.spec.js",
     "test:debug": "yarn test --test-inspect",
     "coverage": "yarn test --test-coverage && yarn coverage:report",
     "coverage:report": "nyc report --reporter=html",

packages/core/src/browser/keybinding.ts

@@ -504,11 +504,8 @@ export class KeybindingRegistry {
         } else {
             const command = this.commandRegistry.getCommand(binding.command);
             if (command) {
-                const commandHandler = this.commandRegistry.getActiveHandler(command.id, binding.args);
-
-                if (commandHandler) {
-                    commandHandler.execute(binding.args);
-                }
+                this.commandRegistry.executeCommand(binding.command, binding.args)
+                    .catch(e => console.error('Failed to execute command:', e));
 
                 /* Note that if a keybinding is in context but the command is
                    not active we still stop the processing here.  */
@@ -532,6 +529,17 @@ export class KeybindingRegistry {
         return true;
     }
 
+    dispatchKeyDown(event: KeyboardEventInit, target?: HTMLElement): void {
+        // Create a fake KeyboardEvent from the data provided
+        const emulatedKeyboardEvent = new KeyboardEvent('keydown', event);
+        // Force override the target
+        Object.defineProperty(emulatedKeyboardEvent, 'target', {
+            get: () => target,
+        });
+        // And re-dispatch
+        this.run(emulatedKeyboardEvent);
+    }
+
     /**
      * Run the command matching to the given keyboard event.
      */

packages/core/src/common/command.ts

@@ -87,6 +87,8 @@ export namespace Command {
 export interface CommandHandler {
     /**
      * Execute this handler.
+     *
+     * Don't call it directly, use `CommandService.executeCommand` instead.
      */
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     execute(...args: any[]): any;
@@ -118,8 +120,13 @@ export interface CommandContribution {
     registerCommands(commands: CommandRegistry): void;
 }
 
-export interface WillExecuteCommandEvent extends WaitUntilEvent {
+export interface CommandEvent {
     commandId: string;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    args: any[]
+}
+
+export interface WillExecuteCommandEvent extends WaitUntilEvent, CommandEvent {
 }
 
 export const commandServicePath = '/services/commands';
@@ -160,6 +167,9 @@ export class CommandRegistry implements CommandService {
     protected readonly onWillExecuteCommandEmitter = new Emitter<WillExecuteCommandEvent>();
     readonly onWillExecuteCommand = this.onWillExecuteCommandEmitter.event;
 
+    protected readonly onDidExecuteCommandEmitter = new Emitter<CommandEvent>();
+    readonly onDidExecuteCommand = this.onDidExecuteCommandEmitter.event;
+
     constructor(
         @inject(ContributionProvider) @named(CommandContribution)
         protected readonly contributionProvider: ContributionProvider<CommandContribution>
@@ -270,10 +280,11 @@ export class CommandRegistry implements CommandService {
      */
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     async executeCommand<T>(commandId: string, ...args: any[]): Promise<T | undefined> {
-        await this.fireWillExecuteCommand(commandId);
         const handler = this.getActiveHandler(commandId, ...args);
         if (handler) {
+            await this.fireWillExecuteCommand(commandId, args);
             const result = await handler.execute(...args);
+            this.onDidExecuteCommandEmitter.fire({ commandId, args });
             const command = this.getCommand(commandId);
             if (command) {
                 this.addRecentCommand(command);
@@ -285,8 +296,9 @@ export class CommandRegistry implements CommandService {
         throw Object.assign(new Error(`The command '${commandId}' cannot be executed. There are no active handlers available for the command.${argsMessage}`), { code: 'NO_ACTIVE_HANDLER' });
     }
 
-    protected async fireWillExecuteCommand(commandId: string): Promise<void> {
-        await WaitUntilEvent.fire(this.onWillExecuteCommandEmitter, { commandId }, 30000);
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    protected async fireWillExecuteCommand(commandId: string, args: any[]): Promise<void> {
+        await WaitUntilEvent.fire(this.onWillExecuteCommandEmitter, { commandId, args }, 30000);
     }
 
     /**

packages/monaco/src/browser/monaco-command-service.ts

@@ -27,37 +27,50 @@ export interface MonacoCommandServiceFactory {
 @injectable()
 export class MonacoCommandService implements ICommandService {
 
-    readonly _onWillExecuteCommand = new Emitter<ICommandEvent>();
+    protected readonly onWillExecuteCommandEmitter = new Emitter<ICommandEvent>();
+    protected readonly onDidExecuteCommandEmitter = new Emitter<ICommandEvent>();
 
-    protected delegate: ICommandService | undefined;
+    protected delegate: monaco.services.StandaloneCommandService | undefined;
     protected readonly delegateListeners = new DisposableCollection();
 
     constructor(
         @inject(CommandRegistry) protected readonly commandRegistry: CommandRegistry
-    ) { }
+    ) {
+        this.commandRegistry.onWillExecuteCommand(e => this.onWillExecuteCommandEmitter.fire(e));
+        this.commandRegistry.onDidExecuteCommand(e => this.onDidExecuteCommandEmitter.fire(e));
+    }
 
     get onWillExecuteCommand(): monaco.IEvent<ICommandEvent> {
-        return this._onWillExecuteCommand.event;
+        return this.onWillExecuteCommandEmitter.event;
+    }
+
+    get onDidExecuteCommand(): monaco.IEvent<ICommandEvent> {
+        return this.onDidExecuteCommandEmitter.event;
     }
 
-    setDelegate(delegate: ICommandService | undefined): void {
+    setDelegate(delegate: monaco.services.StandaloneCommandService | undefined): void {
         this.delegateListeners.dispose();
         this.delegate = delegate;
         if (this.delegate) {
-            this.delegateListeners.push(this.delegate._onWillExecuteCommand.event(event =>
-                this._onWillExecuteCommand.fire(event)
+            this.delegateListeners.push(this.delegate['_onWillExecuteCommand'].event(event =>
+                this.onWillExecuteCommandEmitter.fire(event)
+            ));
+            this.delegateListeners.push(this.delegate['_onDidExecuteCommand'].event(event =>
+                this.onDidExecuteCommandEmitter.fire(event)
             ));
         }
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     async executeCommand(commandId: any, ...args: any[]): Promise<any> {
-        const handler = this.commandRegistry.getActiveHandler(commandId, ...args);
-        if (handler) {
-            this._onWillExecuteCommand.fire({ commandId });
-            return handler.execute(...args);
+        try {
+            await this.commandRegistry.executeCommand(commandId, ...args);
+        } catch (e) {
+            if (e.code === 'NO_ACTIVE_HANDLER') {
+                return this.executeMonacoCommand(commandId, ...args);
+            }
+            throw e;
         }
-        return this.executeMonacoCommand(commandId, ...args);
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/monaco/src/typings/monaco/index.d.ts

@@ -288,12 +288,14 @@ declare module monaco.commands {
 
     export interface ICommandEvent {
         commandId: string;
+        args: any[];
     }
 
+    // https://github.com/TypeFox/vscode/blob/70b8db24a37fafc77247de7f7cb5bb0195120ed0/src/vs/platform/commands/common/commands.ts#L21
     export interface ICommandService {
-        readonly _onWillExecuteCommand: monaco.Emitter<ICommandEvent>;
-        executeCommand<T>(commandId: string, ...args: any[]): Promise<T>;
-        executeCommand(commandId: string, ...args: any[]): Promise<any>;
+        onWillExecuteCommand: monaco.Event<ICommandEvent>;
+        onDidExecuteCommand: monaco.Event<ICommandEvent>;
+        executeCommand<T = any>(commandId: string, ...args: any[]): Promise<T | undefined>;
     }
 
 }
@@ -493,9 +495,12 @@ declare module monaco.services {
         resolveDecorationOptions: monaco.editor.ICodeEditorService['resolveDecorationOptions'];
     }
 
+    // https://github.com/TypeFox/vscode/blob/70b8db24a37fafc77247de7f7cb5bb0195120ed0/src/vs/editor/standalone/browser/simpleServices.ts#L233
     export class StandaloneCommandService implements monaco.commands.ICommandService {
         constructor(instantiationService: monaco.instantiation.IInstantiationService);
-        readonly _onWillExecuteCommand: monaco.Emitter<monaco.commands.ICommandEvent>;
+        private readonly _onWillExecuteCommand: monaco.Emitter<monaco.commands.ICommandEvent>;
+        private readonly _onDidExecuteCommand: monaco.Emitter<monaco.commands.ICommandEvent>;
+
         executeCommand<T>(commandId: string, ...args: any[]): Promise<T>;
         executeCommand(commandId: string, ...args: any[]): Promise<any>;
     }

packages/plugin-ext/src/main/browser/webview/webview.ts

@@ -280,7 +280,7 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget {
             // Electron: workaround for https://github.com/electron/electron/issues/14258
             // We have to detect keyboard events in the <webview> and dispatch them to our
             // keybinding service because these events do not bubble to the parent window anymore.
-            this.dispatchKeyDown(data);
+            this.keybindings.dispatchKeyDown(data, this.element);
         }));
 
         this.style();
@@ -395,17 +395,6 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget {
         this.doSend('styles', { styles, activeTheme });
     }
 
-    protected dispatchKeyDown(event: KeyboardEventInit): void {
-        // Create a fake KeyboardEvent from the data provided
-        const emulatedKeyboardEvent = new KeyboardEvent('keydown', event);
-        // Force override the target
-        Object.defineProperty(emulatedKeyboardEvent, 'target', {
-            get: () => this.element,
-        });
-        // And re-dispatch
-        this.keybindings.run(emulatedKeyboardEvent);
-    }
-
     protected openLink(link: URI): void {
         const supported = this.toSupportedLink(link);
         if (supported) {