From eeaf506e65c934642233a1d45e05231339bc4b30 Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Tue, 19 May 2020 08:44:20 +0000 Subject: [PATCH] fix #7836: respect keybinding scope and registration order during evaluation Also match only first keybinding on keydown, don't traverse over all keybindings. Signed-off-by: Anton Kosyakov --- CHANGELOG.md | 1 + examples/api-tests/src/keybindings.spec.js | 34 ++++- examples/api-tests/src/typescript.spec.js | 42 ++++++ packages/core/src/browser/keybinding.spec.ts | 91 ++++++++---- packages/core/src/browser/keybinding.ts | 139 ++++++++---------- .../monaco/src/browser/monaco-keybinding.ts | 5 +- packages/monaco/src/typings/monaco/index.d.ts | 16 +- .../keybindings-contribution-handler.ts | 2 +- .../browser/terminal-frontend-contribution.ts | 98 ++++++------ 9 files changed, 256 insertions(+), 172 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b23d5585b0914..c736d476a1a27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Breaking changes: - [scm] support file tree mode in Source Control view. Classes that extend ScmWidget will likely require changes [#7505](https://github.com/eclipse-theia/theia/pull/7505) - [task] removed `taskId` from `TaskTerminalWidgetOpenerOptions` [#7765](https://github.com/eclipse-theia/theia/pull/7765) +- [core] `KeybindingRegistry` registers a new keybinding with a higher priority than previously in the same scope [#7839](https://github.com/eclipse-theia/theia/pull/7839) ## v1.1.0 diff --git a/examples/api-tests/src/keybindings.spec.js b/examples/api-tests/src/keybindings.spec.js index b6aca41931562..30df977f67068 100644 --- a/examples/api-tests/src/keybindings.spec.js +++ b/examples/api-tests/src/keybindings.spec.js @@ -20,6 +20,8 @@ describe('Keybindings', function () { const { assert } = chai; const { Disposable, DisposableCollection } = require('@theia/core/lib/common/disposable'); + const { isOSX } = require('@theia/core/lib/common/os'); + const { CommonCommands } = require('@theia/core/lib/browser/common-frontend-contribution'); 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'); @@ -30,7 +32,6 @@ describe('Keybindings', function () { 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; @@ -55,23 +56,23 @@ describe('Keybindings', function () { toTearDown.push(commands.onWillExecuteCommand(e => waitForCommand.resolve(e.commandId))); keybindings.dispatchKeyDown({ code: Key.KEY_K.code, - metaKey: true, - ctrlKey: true + metaKey: isOSX, + ctrlKey: !isOSX }, terminal.node); const executedCommand = await waitForCommand.promise; assert.equal(executedCommand, TerminalCommands.TERMINAL_CLEAR.id); }); - it("disabled keybinding should not override enabled", async () => { + 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', + command: id, keybinding: 'left', when: 'false' - }, true)); + })); const editor = await editorManager.open(new Uri.default(workspaceService.tryGetRoots()[0].uri).resolve('package.json'), { mode: 'activate', @@ -93,4 +94,25 @@ describe('Keybindings', function () { assert.notEqual(executedCommand, id); }); + it('later registered keybinding should has higher priority', async () => { + const id = '__test:keybindings.copy'; + toTearDown.push(commands.registerCommand({ id }, { + execute: () => { } + })); + const keybiding = keybindings.getKeybindingsForCommand(CommonCommands.COPY.id)[0]; + toTearDown.push(keybindings.registerKeybinding({ + command: id, + keybinding: keybiding.keybinding + })); + const waitForCommand = new Deferred(); + toTearDown.push(commands.onWillExecuteCommand(e => waitForCommand.resolve(e.commandId))); + keybindings.dispatchKeyDown({ + code: Key.KEY_C.code, + metaKey: isOSX, + ctrlKey: !isOSX + }); + const executedCommand = await waitForCommand.promise; + assert.equal(executedCommand, id); + }); + }); diff --git a/examples/api-tests/src/typescript.spec.js b/examples/api-tests/src/typescript.spec.js index 9083731fef4af..b74ebc00ef0e8 100644 --- a/examples/api-tests/src/typescript.spec.js +++ b/examples/api-tests/src/typescript.spec.js @@ -455,6 +455,48 @@ module.exports = (port, host, argv) => Promise.resolve() assert.equal(activeEditor.getControl().getModel().getWordAtPosition({ lineNumber, column }).word, 'Container'); }); + it('editor.action.triggerSuggest navigate', async function () { + const editor = await openEditor(serverUri); + // const { [|Container] } = require('inversify'); + editor.getControl().setPosition({ lineNumber: 5, column: 9 }); + editor.getControl().setSelection({ startLineNumber: 5, startColumn: 9, endLineNumber: 5, endColumn: 18 }); + // @ts-ignore + assert.equal(editor.getControl().getModel().getWordAtPosition(editor.getControl().getPosition()).word, 'Container'); + + const suggest = editor.getControl()._contributions['editor.contrib.suggestController']; + const getFocusedLabel = () => { + const focusedItem = suggest.widget.getValue().getFocusedItem(); + return focusedItem && focusedItem.item.completion.label; + }; + + assert.isUndefined(getFocusedLabel()); + assert.isFalse(contextKeyService.match('suggestWidgetVisible')); + + await commands.executeCommand('editor.action.triggerSuggest'); + await waitForAnimation(() => contextKeyService.match('suggestWidgetVisible') && getFocusedLabel() === 'Container'); + + assert.equal(getFocusedLabel(), 'Container'); + assert.isTrue(contextKeyService.match('suggestWidgetVisible')); + + keybindings.dispatchKeyDown('ArrowDown'); + await waitForAnimation(() => contextKeyService.match('suggestWidgetVisible') && getFocusedLabel() === 'ContainerModule'); + + assert.equal(getFocusedLabel(), 'ContainerModule'); + assert.isTrue(contextKeyService.match('suggestWidgetVisible')); + + keybindings.dispatchKeyDown('ArrowUp'); + await waitForAnimation(() => contextKeyService.match('suggestWidgetVisible') && getFocusedLabel() === 'Container'); + + assert.equal(getFocusedLabel(), 'Container'); + assert.isTrue(contextKeyService.match('suggestWidgetVisible')); + + keybindings.dispatchKeyDown('Escape'); + await waitForAnimation(() => !contextKeyService.match('suggestWidgetVisible') && getFocusedLabel() === undefined); + + assert.isUndefined(getFocusedLabel()); + assert.isFalse(contextKeyService.match('suggestWidgetVisible')); + }); + it('editor.action.rename', async function () { const editor = await openEditor(serverUri); // const |container = new Container(); diff --git a/packages/core/src/browser/keybinding.spec.ts b/packages/core/src/browser/keybinding.spec.ts index 0a48cddf483d5..9e16b039b94fe 100644 --- a/packages/core/src/browser/keybinding.spec.ts +++ b/packages/core/src/browser/keybinding.spec.ts @@ -226,15 +226,15 @@ describe('keybindings', () => { keybindingRegistry.setKeymap(KeybindingScope.WORKSPACE, keybindingsSpecific); - let bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.CtrlCmd] })]).full; - expect(bindings).to.have.lengthOf(1); + let match = keybindingRegistry.matchKeybiding([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.CtrlCmd] })]); + expect(match && match.kind).to.be.equal('full'); - bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_B, modifiers: [KeyModifier.CtrlCmd] })]).full; - expect(bindings).to.have.lengthOf(1); + match = keybindingRegistry.matchKeybiding([KeyCode.createKeyCode({ first: Key.KEY_B, modifiers: [KeyModifier.CtrlCmd] })]); + expect(match && match.kind).to.be.equal('full'); - bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] })]).full; - const keyCode = KeyCode.parse(bindings[0].keybinding); - expect(keyCode.key).to.be.equal(validKeyCode.key); + match = keybindingRegistry.matchKeybiding([KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] })]); + const keyCode = match && KeyCode.parse(match.binding.keybinding); + expect(keyCode?.key).to.be.equal(validKeyCode.key); }); it('should return partial keybinding matches', () => { @@ -249,17 +249,17 @@ describe('keybindings', () => { validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] })); validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_T })); - const bindings = keybindingRegistry.getKeybindingsForKeySequence(KeySequence.parse('ctrlcmd+x')); - expect(bindings.partial.length > 0); + const match = keybindingRegistry.matchKeybiding(KeySequence.parse('ctrlcmd+x')); + expect(match && match.kind).to.be.equal('partial'); }); - it('should not register a shadowing keybinding', () => { - const validKeyBinding = 'ctrlcmd+b a'; + it('should possible to override keybinding', () => { + const overridenKeybinding = 'ctrlcmd+b a'; const command = TEST_COMMAND_SHADOW.id; const keybindingShadowing: Keybinding[] = [ { command, - keybinding: validKeyBinding + keybinding: overridenKeybinding }, { command, @@ -270,27 +270,27 @@ describe('keybindings', () => { keybindingRegistry.registerKeybindings(...keybindingShadowing); const bindings = keybindingRegistry.getKeybindingsForCommand(command); - expect(bindings.length).to.be.equal(1); - expect(bindings[0].keybinding).to.be.equal(validKeyBinding); + expect(bindings.length).to.be.equal(2); + expect(bindings[0].keybinding).to.be.equal('ctrlcmd+b'); + expect(bindings[1].keybinding).to.be.equal(overridenKeybinding); }); - it('shadowed bindings should be returned last', () => { + it('overriden bindings should be returned last', () => { const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift] }); - let bindings: Keybinding[]; - const ignoredDefaultBinding: Keybinding = { + const overridenDefaultBinding: Keybinding = { keybinding: keyCode.toString(), - command: 'test.ignored-command' + command: 'test.overriden-default-command' }; const defaultBinding: Keybinding = { keybinding: keyCode.toString(), - command: 'test.workspace-command' + command: 'test.default-command' }; const userBinding: Keybinding = { keybinding: keyCode.toString(), - command: 'test.workspace-command' + command: 'test.user-command' }; const workspaceBinding: Keybinding = { @@ -298,35 +298,62 @@ describe('keybindings', () => { command: 'test.workspace-command' }; - keybindingRegistry.setKeymap(KeybindingScope.DEFAULT, [defaultBinding, ignoredDefaultBinding]); + keybindingRegistry.setKeymap(KeybindingScope.DEFAULT, [overridenDefaultBinding, defaultBinding]); keybindingRegistry.setKeymap(KeybindingScope.USER, [userBinding]); keybindingRegistry.setKeymap(KeybindingScope.WORKSPACE, [workspaceBinding]); // now WORKSPACE bindings are overriding the other scopes - bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full; - expect(bindings).to.have.lengthOf(3); - expect(bindings[0].command).to.be.equal(workspaceBinding.command); + let match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match?.kind).to.be.equal('full'); + expect(match?.binding?.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(2); - expect(bindings[0].command).to.be.equal(userBinding.command); + match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match?.kind).to.be.equal('full'); + expect(match?.binding?.command).to.be.equal(userBinding.command); keybindingRegistry.resetKeybindingsForScope(KeybindingScope.USER); // and finally it should fallback to DEFAULT bindings. - bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full; - expect(bindings).to.have.lengthOf(1); - expect(bindings[0].command).to.be.equal(defaultBinding.command); + match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match?.kind).to.be.equal('full'); + expect(match?.binding?.command).to.be.equal(defaultBinding.command); keybindingRegistry.resetKeybindingsForScope(KeybindingScope.DEFAULT); // now the registry should be empty - bindings = keybindingRegistry.getKeybindingsForKeySequence([keyCode]).full; - expect(bindings).to.be.empty; + match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match).to.be.undefined; + + }); + + it('should not match disabled keybindings', () => { + const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift] }); + + const defaultBinding: Keybinding = { + keybinding: keyCode.toString(), + command: 'test.workspace-command' + }; + const disableDefaultBinding: Keybinding = { + keybinding: keyCode.toString(), + command: '-test.workspace-command' + }; + + keybindingRegistry.setKeymap(KeybindingScope.DEFAULT, [defaultBinding]); + let match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match?.kind).to.be.equal('full'); + expect(match?.binding?.command).to.be.equal(defaultBinding.command); + keybindingRegistry.setKeymap(KeybindingScope.USER, [disableDefaultBinding]); + match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match).to.be.undefined; + + keybindingRegistry.resetKeybindingsForScope(KeybindingScope.USER); + match = keybindingRegistry.matchKeybiding([keyCode]); + expect(match?.kind).to.be.equal('full'); + expect(match?.binding?.command).to.be.equal(defaultBinding.command); }); }); diff --git a/packages/core/src/browser/keybinding.ts b/packages/core/src/browser/keybinding.ts index 54c52e4811b6c..215fa5c3d88f3 100644 --- a/packages/core/src/browser/keybinding.ts +++ b/packages/core/src/browser/keybinding.ts @@ -29,7 +29,6 @@ import * as common from '../common/keybinding'; export enum KeybindingScope { DEFAULT, - DEFAULT_OVERRIDING, USER, WORKSPACE, END @@ -56,7 +55,7 @@ export interface ResolvedKeybinding extends Keybinding { export interface ScopedKeybinding extends Keybinding { /** Current keybinding scope */ - scope?: KeybindingScope; + scope: KeybindingScope; } export const KeybindingContribution = Symbol('KeybindingContribution'); @@ -100,7 +99,7 @@ export class KeybindingRegistry { protected keySequence: KeySequence = []; protected readonly contexts: { [id: string]: KeybindingContext } = {}; - protected readonly keymaps: Keybinding[][] = [...Array(KeybindingScope.length)].map(() => []); + protected readonly keymaps: ScopedKeybinding[][] = [...Array(KeybindingScope.length)].map(() => []); @inject(KeyboardLayoutService) protected readonly keyboardLayoutService: KeyboardLayoutService; @@ -167,11 +166,12 @@ export class KeybindingRegistry { /** * Register a default keybinding to the registry. * + * Keybindings registered later have higher priority during evaluation. + * * @param binding - * @param override Override existed keybinding */ - registerKeybinding(binding: Keybinding, override?: boolean): Disposable { - return this.doRegisterKeybinding(binding, !!override ? KeybindingScope.DEFAULT_OVERRIDING : undefined); + registerKeybinding(binding: Keybinding): Disposable { + return this.doRegisterKeybinding(binding); } /** @@ -219,12 +219,10 @@ export class KeybindingRegistry { protected doRegisterKeybinding(binding: Keybinding, scope: KeybindingScope = KeybindingScope.DEFAULT): Disposable { try { this.resolveKeybinding(binding); - if (this.containsKeybinding(this.keymaps[scope], binding) && scope !== KeybindingScope.DEFAULT_OVERRIDING) { - throw new Error(`"${binding.keybinding}" is in collision with something else [scope:${scope}]`); - } - this.keymaps[scope].push(binding); + const scoped = Object.assign(binding, { scope }); + this.keymaps[scope].unshift(scoped); return Disposable.create(() => { - const index = this.keymaps[scope].indexOf(binding); + const index = this.keymaps[scope].indexOf(scoped); if (index !== -1) { this.keymaps[scope].splice(index, 1); } @@ -260,42 +258,22 @@ export class KeybindingRegistry { } } - /** - * Checks for keySequence collisions in a list of Keybindings - * - * @param bindings the keybinding reference list - * @param binding the keybinding to test collisions for - */ - containsKeybinding(bindings: Keybinding[], binding: Keybinding): boolean { + containsKeybindingInScope(binding: Keybinding, scope = KeybindingScope.USER): boolean { const bindingKeySequence = this.resolveKeybinding(binding); - const collisions = this.getKeySequenceCollisions(bindings, bindingKeySequence) + const collisions = this.getKeySequenceCollisions(this.keymaps[scope], bindingKeySequence) .filter(b => b.context === binding.context && !b.when && !binding.when); - if (collisions.full.length > 0) { - 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 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 is ignored; ', - Keybinding.stringify(binding), ' would be shadowed by ', - collisions.shadow.map(b => Keybinding.stringify(b)).join(', ')); return true; } return false; } - containsKeybindingInScope(binding: Keybinding, scope = KeybindingScope.USER): boolean { - return this.containsKeybinding(this.keymaps[scope], binding); - } - /** * Return a user visible representation of a keybinding. */ @@ -363,30 +341,13 @@ export class KeybindingRegistry { } } - /** - * Finds collisions for a binding inside a list of bindings (error-free) - * - * @param bindings the reference bindings - * @param binding the binding to match - */ - protected getKeybindingCollisions(bindings: Keybinding[], binding: Keybinding): KeybindingRegistry.KeybindingsResult { - const result = new KeybindingRegistry.KeybindingsResult(); - try { - const bindingKeySequence = this.resolveKeybinding(binding); - result.merge(this.getKeySequenceCollisions(bindings, bindingKeySequence)); - } catch (error) { - this.logger.warn(error); - } - return result; - } - /** * Finds collisions for a key sequence inside a list of bindings (error-free) * * @param bindings the reference bindings * @param candidate the sequence to match */ - protected getKeySequenceCollisions(bindings: Keybinding[], candidate: KeySequence): KeybindingRegistry.KeybindingsResult { + protected getKeySequenceCollisions(bindings: ScopedKeybinding[], candidate: KeySequence): KeybindingRegistry.KeybindingsResult { const result = new KeybindingRegistry.KeybindingsResult(); for (const binding of bindings) { try { @@ -413,28 +374,6 @@ export class KeybindingRegistry { return result; } - /** - * Get the lists of keybindings matching fully or partially matching a KeySequence. - * - * @param keySequence The key sequence for which we are looking for keybindings. - */ - 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); - - if (scope === KeybindingScope.DEFAULT_OVERRIDING) { - matches.full.reverse(); - matches.partial.reverse(); - } - matches.full.forEach(binding => binding.scope = scope); - matches.partial.forEach(binding => binding.scope = scope); - result.merge(matches); - } - return result; - } - /** * Get the keybindings associated to commandId. * @@ -578,10 +517,9 @@ export class KeybindingRegistry { this.keyboardLayoutService.validateKeyCode(keyCode); this.keySequence.push(keyCode); - 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) { + const match = this.matchKeybiding(this.keySequence, event); + + if (match && match.kind === 'partial') { /* Accumulate the keysequence */ event.preventDefault(); event.stopPropagation(); @@ -592,14 +530,53 @@ export class KeybindingRegistry { priority: 2 }); } else { - if (full) { - this.executeKeyBinding(full, event); + if (match && match.kind === 'full') { + this.executeKeyBinding(match.binding, event); } this.keySequence = []; this.statusBar.removeElement('keybinding-status'); } } + /** + * Match first binding in the current context. + * Keybinidngs ordered by a scope and by a registration order within the scope. + * + * FIXME: + * This method should run very fast since it happens on each keystoke. We should reconsider how keybindings are stored. + * It should be possible to look up full and partial keybinding for given key sequnce for constant time using some kind of tree. + * Such tree should not contain disabled keybindings and be invalidated whenever the registry is changed. + */ + matchKeybiding(keySequence: KeySequence, event?: KeyboardEvent): KeybindingRegistry.Match { + let disabled: Set | undefined; + const isEnabled = (binding: ScopedKeybinding) => { + if (event && !this.isEnabled(binding, event)) { + return false; + } + const { command, context, when, keybinding } = binding; + if (binding.command.charAt(0) === '-') { + disabled = disabled || new Set(); + disabled.add(JSON.stringify({ command: command.substr(1), context, when, keybinding })); + return false; + } + return !disabled?.has(JSON.stringify({ command, context, when, keybinding })); + }; + + for (let scope = KeybindingScope.END; --scope >= KeybindingScope.DEFAULT;) { + for (const binding of this.keymaps[scope]) { + const resolved = this.resolveKeybinding(binding); + const compareResult = KeySequence.compare(keySequence, resolved); + if (compareResult === KeySequence.CompareResult.FULL && isEnabled(binding)) { + return { kind: 'full', binding }; + } + if (compareResult === KeySequence.CompareResult.PARTIAL && isEnabled(binding)) { + return { kind: 'partial', binding }; + } + } + } + return undefined; + } + /** * Return true of string a pseudo-command id, in other words a command id * that has a special meaning and that we won't find in the command @@ -641,6 +618,10 @@ export class KeybindingRegistry { } export namespace KeybindingRegistry { + export type Match = { + kind: 'full' | 'partial' + binding: ScopedKeybinding + } | undefined; export class KeybindingsResult { full: ScopedKeybinding[] = []; partial: ScopedKeybinding[] = []; diff --git a/packages/monaco/src/browser/monaco-keybinding.ts b/packages/monaco/src/browser/monaco-keybinding.ts index 0c4a444ed7f50..580553adc3050 100644 --- a/packages/monaco/src/browser/monaco-keybinding.ts +++ b/packages/monaco/src/browser/monaco-keybinding.ts @@ -29,10 +29,7 @@ export class MonacoKeybindingContribution implements KeybindingContribution { registerKeybindings(registry: KeybindingRegistry): void { const defaultKeybindings = monaco.keybindings.KeybindingsRegistry.getDefaultKeybindings(); - // register in reverse order to align with Monaco dispatch logic: - // https://github.com/TypeFox/vscode/blob/70b8db24a37fafc77247de7f7cb5bb0195120ed0/src/vs/platform/keybinding/common/keybindingResolver.ts#L302 - for (let i = defaultKeybindings.length - 1; i >= 0; i--) { - const item = defaultKeybindings[i]; + for (const item of defaultKeybindings) { const command = this.commands.validate(item.command); if (command) { const when = item.when && item.when.serialize(); diff --git a/packages/monaco/src/typings/monaco/index.d.ts b/packages/monaco/src/typings/monaco/index.d.ts index 671635c5aea48..dea6561d4b8bc 100644 --- a/packages/monaco/src/typings/monaco/index.d.ts +++ b/packages/monaco/src/typings/monaco/index.d.ts @@ -75,7 +75,8 @@ declare module monaco.editor { 'editor.contrib.referencesController': monaco.referenceSearch.ReferencesController 'editor.contrib.hover': ModesHoverController 'css.editor.codeLens': CodeLensContribution - 'editor.contrib.quickFixController': QuickFixController + 'editor.contrib.quickFixController': QuickFixController, + 'editor.contrib.suggestController': monaco.suggest.SuggestController } readonly _modelData: { cursor: ICursor @@ -1187,6 +1188,19 @@ declare module monaco.suggest { Top, Inline, Bottom } + // https://github.com/theia-ide/vscode/blob/d24b5f70c69b3e75cd10c6b5247a071265ccdd38/src/vs/editor/contrib/suggest/suggestController.ts#L97 + export interface SuggestController { + readonly widget: { + getValue(): SuggestWidget + } + } + export interface SuggestWidget { + getFocusedItem(): ISelectedSuggestion | undefined; + } + export interface ISelectedSuggestion { + item: CompletionItem; + } + // https://github.com/theia-ide/vscode/blob/standalone/0.19.x/src/vs/editor/contrib/suggest/suggest.ts#L28 export interface CompletionItem { completion: monaco.languages.CompletionItem; diff --git a/packages/plugin-ext/src/main/browser/keybindings/keybindings-contribution-handler.ts b/packages/plugin-ext/src/main/browser/keybindings/keybindings-contribution-handler.ts index 24d3307afdfce..2cd13363a57af 100644 --- a/packages/plugin-ext/src/main/browser/keybindings/keybindings-contribution-handler.ts +++ b/packages/plugin-ext/src/main/browser/keybindings/keybindings-contribution-handler.ts @@ -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, true)); + toDispose.push(this.keybindingRegistry.registerKeybinding(keybinding)); } } return toDispose; diff --git a/packages/terminal/src/browser/terminal-frontend-contribution.ts b/packages/terminal/src/browser/terminal-frontend-contribution.ts index 2fb97c15660a0..ff064c131e947 100644 --- a/packages/terminal/src/browser/terminal-frontend-contribution.ts +++ b/packages/terminal/src/browser/terminal-frontend-contribution.ts @@ -397,55 +397,6 @@ export class TerminalFrontendContribution implements TerminalService, CommandCon } registerKeybindings(keybindings: KeybindingRegistry): void { - keybindings.registerKeybinding({ - command: TerminalCommands.NEW.id, - keybinding: 'ctrl+shift+`' - }); - keybindings.registerKeybinding({ - command: TerminalCommands.NEW_ACTIVE_WORKSPACE.id, - keybinding: 'ctrl+`' - }); - keybindings.registerKeybinding({ - command: TerminalCommands.TERMINAL_CLEAR.id, - keybinding: 'ctrlcmd+k', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.TERMINAL_FIND_TEXT.id, - keybinding: 'ctrlcmd+f', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.TERMINAL_FIND_TEXT_CANCEL.id, - keybinding: 'esc', - context: TerminalKeybindingContexts.terminalHideSearch - }); - keybindings.registerKeybinding({ - command: TerminalCommands.SCROLL_LINE_UP.id, - keybinding: 'ctrl+shift+up', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.SCROLL_LINE_DOWN.id, - keybinding: 'ctrl+shift+down', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.SCROLL_TO_TOP.id, - keybinding: 'shift-home', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.SCROLL_PAGE_UP.id, - keybinding: 'shift-pageUp', - context: TerminalKeybindingContexts.terminalActive - }); - keybindings.registerKeybinding({ - command: TerminalCommands.SCROLL_PAGE_DOWN.id, - keybinding: 'shift-pageDown', - context: TerminalKeybindingContexts.terminalActive - }); - /* Register passthrough keybindings for combinations recognized by xterm.js and converted to control characters. @@ -529,6 +480,55 @@ export class TerminalFrontendContribution implements TerminalService, CommandCon context: TerminalKeybindingContexts.terminalActive }); } + + keybindings.registerKeybinding({ + command: TerminalCommands.NEW.id, + keybinding: 'ctrl+shift+`' + }); + keybindings.registerKeybinding({ + command: TerminalCommands.NEW_ACTIVE_WORKSPACE.id, + keybinding: 'ctrl+`' + }); + keybindings.registerKeybinding({ + command: TerminalCommands.TERMINAL_CLEAR.id, + keybinding: 'ctrlcmd+k', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.TERMINAL_FIND_TEXT.id, + keybinding: 'ctrlcmd+f', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.TERMINAL_FIND_TEXT_CANCEL.id, + keybinding: 'esc', + context: TerminalKeybindingContexts.terminalHideSearch + }); + keybindings.registerKeybinding({ + command: TerminalCommands.SCROLL_LINE_UP.id, + keybinding: 'ctrl+shift+up', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.SCROLL_LINE_DOWN.id, + keybinding: 'ctrl+shift+down', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.SCROLL_TO_TOP.id, + keybinding: 'shift-home', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.SCROLL_PAGE_UP.id, + keybinding: 'shift-pageUp', + context: TerminalKeybindingContexts.terminalActive + }); + keybindings.registerKeybinding({ + command: TerminalCommands.SCROLL_PAGE_DOWN.id, + keybinding: 'shift-pageDown', + context: TerminalKeybindingContexts.terminalActive + }); } async newTerminal(options: TerminalWidgetOptions): Promise {