diff --git a/examples/api-tests/src/keybindings.spec.js b/examples/api-tests/src/keybindings.spec.js new file mode 100644 index 0000000000000..b6aca41931562 --- /dev/null +++ b/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); + }); + +}); diff --git a/examples/browser/package.json b/examples/browser/package.json index 94758411ded8f..87e182eb7d730 100644 --- a/examples/browser/package.json +++ b/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", diff --git a/packages/core/src/browser/keybinding.ts b/packages/core/src/browser/keybinding.ts index 460616fed5c53..6deae681b6008 100644 --- a/packages/core/src/browser/keybinding.ts +++ b/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. */ diff --git a/packages/core/src/common/command.ts b/packages/core/src/common/command.ts index 1ceead5f8d8e3..f6b4b6f7f0285 100644 --- a/packages/core/src/common/command.ts +++ b/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(); readonly onWillExecuteCommand = this.onWillExecuteCommandEmitter.event; + protected readonly onDidExecuteCommandEmitter = new Emitter(); + readonly onDidExecuteCommand = this.onDidExecuteCommandEmitter.event; + constructor( @inject(ContributionProvider) @named(CommandContribution) protected readonly contributionProvider: ContributionProvider @@ -270,10 +280,11 @@ export class CommandRegistry implements CommandService { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any async executeCommand(commandId: string, ...args: any[]): Promise { - 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 { - await WaitUntilEvent.fire(this.onWillExecuteCommandEmitter, { commandId }, 30000); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + protected async fireWillExecuteCommand(commandId: string, args: any[]): Promise { + await WaitUntilEvent.fire(this.onWillExecuteCommandEmitter, { commandId, args }, 30000); } /** diff --git a/packages/monaco/src/browser/monaco-command-service.ts b/packages/monaco/src/browser/monaco-command-service.ts index 982fc76a2b23a..79281f9959b77 100644 --- a/packages/monaco/src/browser/monaco-command-service.ts +++ b/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(); + protected readonly onWillExecuteCommandEmitter = new Emitter(); + protected readonly onDidExecuteCommandEmitter = new Emitter(); - 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 { - return this._onWillExecuteCommand.event; + return this.onWillExecuteCommandEmitter.event; + } + + get onDidExecuteCommand(): monaco.IEvent { + 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 { - 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 diff --git a/packages/monaco/src/typings/monaco/index.d.ts b/packages/monaco/src/typings/monaco/index.d.ts index 5fbf871b2d74c..f04c1683e58f6 100644 --- a/packages/monaco/src/typings/monaco/index.d.ts +++ b/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; - executeCommand(commandId: string, ...args: any[]): Promise; - executeCommand(commandId: string, ...args: any[]): Promise; + onWillExecuteCommand: monaco.Event; + onDidExecuteCommand: monaco.Event; + executeCommand(commandId: string, ...args: any[]): Promise; } } @@ -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; + private readonly _onWillExecuteCommand: monaco.Emitter; + private readonly _onDidExecuteCommand: monaco.Emitter; + executeCommand(commandId: string, ...args: any[]): Promise; executeCommand(commandId: string, ...args: any[]): Promise; } diff --git a/packages/plugin-ext/src/main/browser/webview/webview.ts b/packages/plugin-ext/src/main/browser/webview/webview.ts index 93162847c1e49..aa1f3003c8f83 100644 --- a/packages/plugin-ext/src/main/browser/webview/webview.ts +++ b/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 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) {