GH-6428: Refactored the built-in monaco commands.

Closes #6428 Signed-off-by: Akos Kitta <kittaakos@typefox.io>
eclipse-theia · Apr 6, 2020 · 40b1d45 · 40b1d45
1 parent d96e6f1
commit 40b1d45
Show file tree

Hide file tree

Showing 17 changed files with 342 additions and 162 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,13 +4,15 @@
 
 Breaking changes:
 
+- [core] Removed the `core.find` and `core.replace` commands as they did not work without the `@theia/monaco` extension, or an opened Monaco editor. Use the Monaco-specific `actions.find` and `editor.action.startFindReplaceAction` commands instead. [#7474](https://github.com/eclipse-theia/theia/issues/7474)
 - [plugin] removed `configStorage` argument from `PluginManager.registerPlugin`.
 Use `PluginManager.configStorage` property instead. [#7265](https://github.com/eclipse-theia/theia/pull/7265#discussion_r399956070)
 - [process] `TerminalProcess` doesn't handle shell quoting, the shell process arguments must be prepared from the caller. Removed all methods related to shell escaping inside this class. You should use functions located in `@theia/process/lib/common/shell-quoting.ts` in order to process arguments for shells.
 - [process/terminal] Moved shell escaping utilities into `@theia/process/lib/common/shell-quoting` and `@theia/process/lib/common/shell-command-builder` for creating shell inputs.
 
 ## v1.0.0
 
+- [core] `CommandRegistry#getAllHandlers` returns with the reversed order of handlers, so if a command has multiple handlers, any handler is considered to be more specific than the other subsequent handlers in the array. In other words, if `@theia/core` contributes a handler for a particular command and your extension also contributes a handler for the same command, the handler from your extension will have `0` index, and the handler from `@theia/core` will have `1` index when calling `getAllHandlers`.
 - [core] added functionality to ensure that nodes are refreshed properly on tree expansion [#7400](https://github.com/eclipse-theia/theia/pull/7400)
 - [core] added loading state for trees [#7249](https://github.com/eclipse-theia/theia/pull/7249)
 - [core] added the ability to customize the layout of view-containers [#6655](https://github.com/eclipse-theia/theia/pull/6655)

diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts
@@ -17,12 +17,12 @@
 /* eslint-disable max-len, @typescript-eslint/indent */
 
 import debounce = require('lodash.debounce');
-import { injectable, inject, postConstruct } from 'inversify';
+import { injectable, inject, postConstruct, unmanaged } from 'inversify';
 import { TabBar, Widget, Title } from '@phosphor/widgets';
 import { MAIN_MENU_BAR, MenuContribution, MenuModelRegistry } from '../common/menu';
-import { KeybindingContribution, KeybindingRegistry } from './keybinding';
+import { KeybindingContribution, KeybindingRegistry, NativeTextInputFocusContext } from './keybinding';
 import { FrontendApplicationContribution } from './frontend-application';
-import { CommandContribution, CommandRegistry, Command } from '../common/command';
+import { CommandContribution, CommandRegistry, Command, CommandHandler } from '../common/command';
 import { UriAwareCommandHandler } from '../common/uri-command-handler';
 import { SelectionService } from '../common/selection-service';
 import { MessageService } from '../common/message-service';
@@ -109,14 +109,9 @@ export namespace CommonCommands {
         id: 'core.redo',
         label: 'Redo'
     };
-
-    export const FIND: Command = {
-        id: 'core.find',
-        label: 'Find'
-    };
-    export const REPLACE: Command = {
-        id: 'core.replace',
-        label: 'Replace'
+    export const SELECT_ALL: Command = {
+        id: 'core.selectAll',
+        label: 'Select All'
     };
 
     export const NEXT_TAB: Command = {
@@ -240,6 +235,25 @@ export const supportPaste = browser.isNative || (!browser.isChrome && document.q
 
 export const RECENT_COMMANDS_STORAGE_KEY = 'commands';
 
+@injectable() export abstract class DomCommandHandler implements CommandHandler {
+    constructor(@unmanaged() protected domCommand: string) { }
+    execute(): void { document.execCommand(this.domCommand); }
+}
+@injectable() export abstract class NativeTextInputCommandHandler extends DomCommandHandler {
+    @inject(NativeTextInputFocusContext) protected readonly delegate: NativeTextInputFocusContext;
+    isEnabled(): boolean { return this.delegate.isEnabled(); }
+    isVisible(): boolean { return this.isEnabled(); }
+}
+@injectable() export class UndoHandler extends NativeTextInputCommandHandler {
+    constructor() { super('undo'); }
+}
+@injectable() export class RedoHandler extends NativeTextInputCommandHandler {
+    constructor() { super('redo'); }
+}
+@injectable() export class SelectAllHandler extends NativeTextInputCommandHandler {
+    constructor() { super('selectAll'); }
+}
+
 @injectable()
 export class CommonFrontendContribution implements FrontendApplicationContribution, MenuContribution, CommandContribution, KeybindingContribution, ColorContribution {
 
@@ -284,6 +298,15 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     @inject(PreferenceService)
     protected readonly preferenceService: PreferenceService;
 
+    @inject(UndoHandler)
+    protected readonly undoHandler: CommandHandler;
+
+    @inject(RedoHandler)
+    protected readonly redoHandler: CommandHandler;
+
+    @inject(SelectAllHandler)
+    protected readonly selectAllHandler: CommandHandler;
+
     @postConstruct()
     protected init(): void {
         this.contextKeyService.createKey<boolean>('isLinux', OS.type() === OS.Type.Linux);
@@ -389,15 +412,6 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             order: '1'
         });
 
-        registry.registerMenuAction(CommonMenus.EDIT_FIND, {
-            commandId: CommonCommands.FIND.id,
-            order: '0'
-        });
-        registry.registerMenuAction(CommonMenus.EDIT_FIND, {
-            commandId: CommonCommands.REPLACE.id,
-            order: '1'
-        });
-
         registry.registerMenuAction(CommonMenus.EDIT_CLIPBOARD, {
             commandId: CommonCommands.CUT.id,
             order: '0'
@@ -500,11 +514,9 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             }
         });
 
-        commandRegistry.registerCommand(CommonCommands.UNDO);
-        commandRegistry.registerCommand(CommonCommands.REDO);
-
-        commandRegistry.registerCommand(CommonCommands.FIND);
-        commandRegistry.registerCommand(CommonCommands.REPLACE);
+        commandRegistry.registerCommand(CommonCommands.UNDO, this.undoHandler);
+        commandRegistry.registerCommand(CommonCommands.REDO, this.redoHandler);
+        commandRegistry.registerCommand(CommonCommands.SELECT_ALL, this.selectAllHandler);
 
         commandRegistry.registerCommand(CommonCommands.NEXT_TAB, {
             isEnabled: () => this.shell.currentTabBar !== undefined,
@@ -577,7 +589,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             isEnabled: () => {
                 const currentWidget = this.shell.getCurrentWidget('main');
                 return currentWidget !== undefined &&
-                       this.shell.mainAreaTabBars.some(tb => tb.titles.some(title => title.owner !== currentWidget && title.closable));
+                    this.shell.mainAreaTabBars.some(tb => tb.titles.some(title => title.owner !== currentWidget && title.closable));
             },
             execute: () => {
                 const currentWidget = this.shell.getCurrentWidget('main');
@@ -699,19 +711,18 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             // Edition
             {
                 command: CommonCommands.UNDO.id,
-                keybinding: 'ctrlcmd+z'
+                keybinding: 'ctrlcmd+z',
+                context: NativeTextInputFocusContext.ID
             },
             {
                 command: CommonCommands.REDO.id,
-                keybinding: 'ctrlcmd+shift+z'
-            },
-            {
-                command: CommonCommands.FIND.id,
-                keybinding: 'ctrlcmd+f'
+                keybinding: 'ctrlcmd+shift+z',
+                context: NativeTextInputFocusContext.ID
             },
             {
-                command: CommonCommands.REPLACE.id,
-                keybinding: 'ctrlcmd+alt+f'
+                command: CommonCommands.SELECT_ALL.id,
+                keybinding: 'ctrlcmd+a',
+                context: NativeTextInputFocusContext.ID
             },
             // Tabs
             {

diff --git a/packages/core/src/browser/frontend-application-module.ts b/packages/core/src/browser/frontend-application-module.ts
@@ -30,11 +30,11 @@ import {
     InMemoryResources,
     messageServicePath
 } from '../common';
-import { KeybindingRegistry, KeybindingContext, KeybindingContribution } from './keybinding';
+import { KeybindingRegistry, KeybindingContext, KeybindingContribution, NativeTextInputFocusContext } from './keybinding';
 import { FrontendApplication, FrontendApplicationContribution, DefaultFrontendApplicationContribution } from './frontend-application';
 import { DefaultOpenerService, OpenerService, OpenHandler } from './opener-service';
 import { HttpOpenHandler } from './http-open-handler';
-import { CommonFrontendContribution } from './common-frontend-contribution';
+import { CommonFrontendContribution, UndoHandler, RedoHandler, SelectAllHandler } from './common-frontend-contribution';
 import {
     QuickOpenService, QuickCommandService, QuickCommandFrontendContribution, QuickOpenContribution,
     QuickOpenHandlerRegistry, CommandQuickOpenContribution, HelpQuickOpenHandler,
@@ -318,4 +318,11 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
     });
 
     bind(ContextMenuContext).toSelf().inSingletonScope();
+
+    bind(UndoHandler).toSelf().inSingletonScope();
+    bind(RedoHandler).toSelf().inSingletonScope();
+    bind(SelectAllHandler).toSelf().inSingletonScope();
+    bind(NativeTextInputFocusContext).toSelf().inSingletonScope();
+    bind(KeybindingContext).toService(NativeTextInputFocusContext);
+
 });
diff --git a/packages/core/src/browser/keybinding.ts b/packages/core/src/browser/keybinding.ts
@@ -91,6 +91,26 @@ export namespace KeybindingContexts {
         id: 'default.keybinding.context',
         isEnabled: () => false
     };
+
+}
+
+/**
+ * Keybinding context that is enabled when the focused HTML element is an `input` or a `textArea`.
+ */
+@injectable()
+export class NativeTextInputFocusContext implements KeybindingContext {
+
+    static readonly ID = 'nativeTextInputFocus';
+    readonly id = NativeTextInputFocusContext.ID;
+
+    /**
+     * `true` if the "focused" DOM element (`document.activeElement`) is an `input` or a `textArea`. Otherwise, `false`.
+     */
+    isEnabled(): boolean {
+        const { activeElement } = document;
+        return !!activeElement && ['input', 'textarea'].indexOf(activeElement.tagName.toLocaleLowerCase()) !== -1;
+    }
+
 }
 
 @injectable()

diff --git a/packages/core/src/common/command.ts b/packages/core/src/common/command.ts
@@ -310,16 +310,13 @@ export class CommandRegistry implements CommandService {
      */
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     getVisibleHandler(commandId: string, ...args: any[]): CommandHandler | undefined {
-        const handlers = this._handlers[commandId];
-        if (handlers) {
-            for (const handler of handlers) {
-                try {
-                    if (!handler.isVisible || handler.isVisible(...args)) {
-                        return handler;
-                    }
-                } catch (error) {
-                    console.error(error);
+        for (const handler of this.getAllHandlers(commandId)) {
+            try {
+                if (!handler.isVisible || handler.isVisible(...args)) {
+                    return handler;
                 }
+            } catch (error) {
+                console.error(error);
             }
         }
         return undefined;
@@ -330,16 +327,13 @@ export class CommandRegistry implements CommandService {
      */
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     getActiveHandler(commandId: string, ...args: any[]): CommandHandler | undefined {
-        const handlers = this._handlers[commandId];
-        if (handlers) {
-            for (const handler of handlers) {
-                try {
-                    if (!handler.isEnabled || handler.isEnabled(...args)) {
-                        return handler;
-                    }
-                } catch (error) {
-                    console.error(error);
+        for (const handler of this.getAllHandlers(commandId)) {
+            try {
+                if (!handler.isEnabled || handler.isEnabled(...args)) {
+                    return handler;
                 }
+            } catch (error) {
+                console.error(error);
             }
         }
         return undefined;
@@ -350,16 +344,13 @@ export class CommandRegistry implements CommandService {
      */
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     getToggledHandler(commandId: string, ...args: any[]): CommandHandler | undefined {
-        const handlers = this._handlers[commandId];
-        if (handlers) {
-            for (const handler of handlers) {
-                try {
-                    if (handler.isToggled && handler.isToggled(...args)) {
-                        return handler;
-                    }
-                } catch (error) {
-                    console.error(error);
+        for (const handler of this.getAllHandlers(commandId)) {
+            try {
+                if (handler.isToggled && handler.isToggled(...args)) {
+                    return handler;
                 }
+            } catch (error) {
+                console.error(error);
             }
         }
         return undefined;
@@ -371,7 +362,8 @@ export class CommandRegistry implements CommandService {
      */
     getAllHandlers(commandId: string): CommandHandler[] {
         const handlers = this._handlers[commandId];
-        return handlers ? handlers.slice() : [];
+        // We intentionally reverse the array of handlers, so if there are multiple handlers for a command, you can find the more specific, enabled one.
+        return handlers ? handlers.slice().reverse() : [];
     }
 
     /**

diff --git a/packages/core/src/electron-browser/keyboard/electron-common-commands.ts b/packages/core/src/electron-browser/keyboard/electron-common-commands.ts
@@ -0,0 +1,46 @@
+/********************************************************************************
+ * 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
+ ********************************************************************************/
+
+import { remote } from 'electron';
+import { injectable } from 'inversify';
+import { UndoHandler, RedoHandler, SelectAllHandler } from '../../browser/common-frontend-contribution';
+
+@injectable()
+export class ElectronUndoHandler extends UndoHandler {
+
+    execute(): void {
+        remote.getCurrentWebContents().undo();
+    }
+
+}
+
+@injectable()
+export class ElectronRedoHandler extends RedoHandler {
+
+    execute(): void {
+        remote.getCurrentWebContents().redo();
+    }
+
+}
+
+@injectable()
+export class ElectronSelectAllHandler extends SelectAllHandler {
+
+    execute(): void {
+        remote.getCurrentWebContents().selectAll();
+    }
+
+}
diff --git a/packages/core/src/electron-browser/keyboard/electron-keyboard-module.ts b/packages/core/src/electron-browser/keyboard/electron-keyboard-module.ts
@@ -18,11 +18,16 @@ import { ContainerModule } from 'inversify';
 import { KeyboardLayoutProvider, keyboardPath, KeyboardLayoutChangeNotifier } from '../../common/keyboard/keyboard-layout-provider';
 import { WebSocketConnectionProvider } from '../../browser/messaging/ws-connection-provider';
 import { ElectronKeyboardLayoutChangeNotifier } from './electron-keyboard-layout-change-notifier';
+import { UndoHandler, RedoHandler, SelectAllHandler } from '../../browser/common-frontend-contribution';
+import { ElectronUndoHandler, ElectronRedoHandler, ElectronSelectAllHandler } from './electron-common-commands';
 
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
     bind(KeyboardLayoutProvider).toDynamicValue(ctx =>
         WebSocketConnectionProvider.createProxy<KeyboardLayoutProvider>(ctx.container, keyboardPath)
     ).inSingletonScope();
     bind(ElectronKeyboardLayoutChangeNotifier).toSelf().inSingletonScope();
     bind(KeyboardLayoutChangeNotifier).toService(ElectronKeyboardLayoutChangeNotifier);
+    rebind(UndoHandler).to(ElectronUndoHandler).inSingletonScope();
+    rebind(RedoHandler).to(ElectronRedoHandler).inSingletonScope();
+    rebind(SelectAllHandler).to(ElectronSelectAllHandler).inSingletonScope();
 });
diff --git a/packages/monaco/src/browser/monaco-command-registry.ts b/packages/monaco/src/browser/monaco-command-registry.ts
@@ -63,7 +63,6 @@ export class MonacoCommandRegistry {
     protected execute(monacoHandler: MonacoEditorCommandHandler, ...args: any[]): any {
         const editor = this.monacoEditors.current;
         if (editor) {
-            editor.focus();
             return Promise.resolve(monacoHandler.execute(editor, ...args));
         }
         return Promise.resolve();