various fixes to make git and gitlens vscode extension work

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/api-tests/src/monaco-api.spec.js

@@ -26,11 +26,14 @@ describe('Monaco API', async function () {
     const { MonacoEditor } = require('@theia/monaco/lib/browser/monaco-editor');
     const { MonacoResolvedKeybinding } = require('@theia/monaco/lib/browser/monaco-resolved-keybinding');
     const { MonacoTextmateService } = require('@theia/monaco/lib/browser/textmate/monaco-textmate-service');
+    const { CommandRegistry } = require('@theia/core/lib/common/command');
 
     const container = window.theia.container;
     const editorManager = container.get(EditorManager);
     const workspaceService = container.get(WorkspaceService);
     const textmateService = container.get(MonacoTextmateService);
+    const commands = container.get(CommandRegistry);
+
     /** @type {MonacoEditor} */
     let monacoEditor;
 
@@ -107,4 +110,27 @@ describe('Monaco API', async function () {
         assert.deepStrictEqual(monacoColorMap, textMateColorMap, 'Expected textmate colors to have the same index in the monaco color map.');
     });
 
+    it('OpenerService.open', async () => {
+        const hoverContribution = monacoEditor.getControl().getContribution('editor.contrib.hover');
+        assert.isDefined(hoverContribution);
+        if (!('_openerService' in hoverContribution)) {
+            assert.fail('hoverContribution does not have OpenerService');
+            return;
+        }
+        /** @type {monaco.services.OpenerService} */
+        const openerService = hoverContribution['_openerService'];
+
+        let opened = false;
+        const id = '__test:OpenerService.open';
+        const unregisterCommand = commands.registerCommand({ id }, {
+            execute: arg => (console.log(arg), opened = arg === 'foo')
+        });
+        try {
+            await openerService.open(monaco.Uri.parse('command:' + id + '?"foo"'));
+            assert.isTrue(opened);
+        } finally {
+            unregisterCommand.dispose();
+        }
+    });
+
 });

packages/plugin-ext/src/main/browser/plugin-command-open-handler.ts → packages/core/src/browser/command-open-handler.ts

@@ -15,33 +15,37 @@
  ********************************************************************************/
 
 import { injectable, inject } from 'inversify';
-import URI from '@theia/core/lib/common/uri';
-import { OpenHandler } from '@theia/core/lib/browser/opener-service';
-import { Schemes } from '../../common/uri-components';
-import { CommandService } from '@theia/core/lib/common/command';
+import { CommandService } from '../common/command';
+import URI from '../common/uri';
+import { OpenHandler } from './opener-service';
 
 @injectable()
-export class PluginCommandOpenHandler implements OpenHandler {
+export class CommandOpenHandler implements OpenHandler {
 
-    readonly id = 'plugin-command';
+    readonly id = 'command';
 
     @inject(CommandService)
     protected readonly commands: CommandService;
 
     canHandle(uri: URI): number {
-        return uri.scheme === Schemes.COMMAND ? 500 : -1;
+        return uri.scheme === 'command' ? 500 : -1;
     }
 
     async open(uri: URI): Promise<boolean> {
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         let args: any = [];
         try {
-            args = JSON.parse(uri.query);
-            if (!Array.isArray(args)) {
-                args = [args];
+            args = JSON.parse(decodeURIComponent(uri.query));
+        } catch {
+            // ignore and retry
+            try {
+                args = JSON.parse(uri.query);
+            } catch {
+                // ignore error
             }
-        } catch (e) {
-            // ignore error
+        }
+        if (!Array.isArray(args)) {
+            args = [args];
         }
         await this.commands.executeCommand(uri.path.toString(), ...args);
         return true;

packages/core/src/browser/frontend-application-module.ts

@@ -91,6 +91,7 @@ import { IconThemeApplicationContribution, IconThemeContribution, DefaultFileIco
 import { TreeLabelProvider } from './tree/tree-label-provider';
 import { ProgressBar } from './progress-bar';
 import { ProgressBarFactory, ProgressBarOptions } from './progress-bar-factory';
+import { CommandOpenHandler } from './command-open-handler';
 
 export { bindResourceProvider, bindMessageService, bindPreferenceService };
 
@@ -161,6 +162,9 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
     bind(HttpOpenHandler).toSelf().inSingletonScope();
     bind(OpenHandler).toService(HttpOpenHandler);
 
+    bind(CommandOpenHandler).toSelf().inSingletonScope();
+    bind(OpenHandler).toService(CommandOpenHandler);
+
     bindContributionProvider(bind, ApplicationShellLayoutMigration);
     bind<ApplicationShellLayoutMigration>(ApplicationShellLayoutMigration).toConstantValue({
         layoutVersion: 2.0,

packages/core/src/browser/http-open-handler.ts

@@ -20,6 +20,10 @@ import { OpenHandler } from './opener-service';
 import { WindowService } from './window/window-service';
 import { ExternalUriService } from './external-uri-service';
 
+export interface HttpOpenHandlerOptions {
+    openExternal?: boolean
+}
+
 @injectable()
 export class HttpOpenHandler implements OpenHandler {
 
@@ -31,8 +35,8 @@ export class HttpOpenHandler implements OpenHandler {
     @inject(ExternalUriService)
     protected readonly externalUriService: ExternalUriService;
 
-    canHandle(uri: URI): number {
-        return (uri.scheme.startsWith('http') || uri.scheme.startsWith('mailto')) ? 500 : 0;
+    canHandle(uri: URI, options?: HttpOpenHandlerOptions): number {
+        return ((options && options.openExternal) || uri.scheme.startsWith('http') || uri.scheme.startsWith('mailto')) ? 500 : 0;
     }
 
     async open(uri: URI): Promise<undefined> {

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

@@ -274,7 +274,7 @@ describe('keybindings', () => {
         expect(bindings[0].keybinding).to.be.equal(validKeyBinding);
     });
 
-    it('shadowed bindings should not be returned', () => {
+    it('shadowed bindings should be returned last', () => {
         const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift] });
         let bindings: Keybinding[];
 
@@ -304,14 +304,14 @@ describe('keybindings', () => {
         // now WORKSPACE bindings are overriding the other scopes
 
         bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full;
-        expect(bindings).to.have.lengthOf(1);
+        expect(bindings).to.have.lengthOf(3);
         expect(bindings[0].command).to.be.equal(workspaceBinding.command);
 
         keybindingRegistry.resetKeybindingsForScope(KeybindingScope.WORKSPACE);
         // now it should find USER bindings
 
         bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full;
-        expect(bindings).to.have.lengthOf(1);
+        expect(bindings).to.have.lengthOf(2);
         expect(bindings[0].command).to.be.equal(userBinding.command);
 
         keybindingRegistry.resetKeybindingsForScope(KeybindingScope.USER);

packages/core/src/browser/keybinding.ts

@@ -415,22 +415,15 @@ export class KeybindingRegistry {
 
     /**
      * Get the lists of keybindings matching fully or partially matching a KeySequence.
-     * The lists are sorted by priority (see #sortKeybindingsByPriority).
      *
      * @param keySequence The key sequence for which we are looking for keybindings.
-     * @param event The source keyboard event.
      */
-    getKeybindingsForKeySequence(keySequence: KeySequence, event?: KeyboardEvent): KeybindingRegistry.KeybindingsResult {
+    getKeybindingsForKeySequence(keySequence: KeySequence): KeybindingRegistry.KeybindingsResult {
         const result = new KeybindingRegistry.KeybindingsResult();
 
         for (let scope = KeybindingScope.END; --scope >= KeybindingScope.DEFAULT;) {
             const matches = this.getKeySequenceCollisions(this.keymaps[scope], keySequence);
 
-            matches.full = matches.full.filter(
-                binding => (!event || this.isEnabled(binding, event)) && this.getKeybindingCollisions(result.full, binding).full.length === 0);
-            matches.partial = matches.partial.filter(
-                binding => (!event || this.isEnabled(binding, event)) && this.getKeybindingCollisions(result.partial, binding).partial.length === 0);
-
             if (scope === KeybindingScope.DEFAULT_OVERRIDING) {
                 matches.full.reverse();
                 matches.partial.reverse();
@@ -439,8 +432,6 @@ export class KeybindingRegistry {
             matches.partial.forEach(binding => binding.scope = scope);
             result.merge(matches);
         }
-        this.sortKeybindingsByPriority(result.full);
-        this.sortKeybindingsByPriority(result.partial);
         return result;
     }
 
@@ -490,39 +481,6 @@ export class KeybindingRegistry {
         return result;
     }
 
-    /**
-     * Sort keybindings in-place, in order of priority.
-     *
-     * The only criterion right now is that a keybinding with a context has
-     * more priority than a keybinding with no context.
-     *
-     * @param keybindings Array of keybindings to be sorted in-place.
-     */
-    private sortKeybindingsByPriority(keybindings: Keybinding[]): void {
-        keybindings.sort((a: Keybinding, b: Keybinding): number => {
-
-            let acontext: KeybindingContext | undefined;
-            if (a.context) {
-                acontext = this.contexts[a.context];
-            }
-
-            let bcontext: KeybindingContext | undefined;
-            if (b.context) {
-                bcontext = this.contexts[b.context];
-            }
-
-            if (acontext && !bcontext) {
-                return -1;
-            }
-
-            if (!acontext && bcontext) {
-                return 1;
-            }
-
-            return 0;
-        });
-    }
-
     protected isActive(binding: Keybinding): boolean {
         /* Pseudo commands like "passthrough" are always active (and not found
            in the command registry).  */
@@ -620,7 +578,7 @@ export class KeybindingRegistry {
 
         this.keyboardLayoutService.validateKeyCode(keyCode);
         this.keySequence.push(keyCode);
-        const bindings = this.getKeybindingsForKeySequence(this.keySequence, event);
+        const bindings = this.getKeybindingsForKeySequence(this.keySequence);
         const full = bindings.full.find(binding => this.isEnabled(binding, event));
         const partial = bindings.partial.find(binding => (!full || binding.scope! > full.scope!) && this.isEnabled(binding, event));
         if (partial) {