From 754ee2f9f401925d24f9b8ba8630f251de24b1e2 Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Tue, 14 Jan 2020 04:37:20 +0000 Subject: [PATCH] [keybinding] fix #6878: allow a user to change default keybindings Signed-off-by: Anton Kosyakov --- .../src/browser/keybindings-widget.tsx | 10 ++- .../keymaps/src/browser/keymaps-parser.ts | 2 +- .../keymaps/src/browser/keymaps-service.ts | 87 +++++++++---------- .../src/browser/monaco-editor-provider.ts | 20 +++++ packages/monaco/src/typings/monaco/index.d.ts | 5 ++ 5 files changed, 73 insertions(+), 51 deletions(-) diff --git a/packages/keymaps/src/browser/keybindings-widget.tsx b/packages/keymaps/src/browser/keybindings-widget.tsx index 986d4f947b2c9..7f92d2dbc6462 100644 --- a/packages/keymaps/src/browser/keybindings-widget.tsx +++ b/packages/keymaps/src/browser/keybindings-widget.tsx @@ -22,7 +22,7 @@ import { CommandRegistry, Emitter, Event } from '@theia/core/lib/common'; import { ReactWidget } from '@theia/core/lib/browser/widgets/react-widget'; import { KeybindingRegistry, SingleTextInputDialog, KeySequence, ConfirmDialog, Message, KeybindingScope, SingleTextInputDialogProps, Key } from '@theia/core/lib/browser'; import { KeymapsParser } from './keymaps-parser'; -import { KeymapsService, KeybindingJson } from './keymaps-service'; +import { KeymapsService } from './keymaps-service'; import { AlertMessage } from '@theia/core/lib/browser/widgets/alert-message'; /** @@ -524,9 +524,7 @@ export class KeybindingWidget extends ReactWidget { */ protected editKeybinding(item: KeybindingItem): void { const command = this.getRawValue(item.command); - const id = this.getRawValue(item.id); const keybinding = (item.keybinding) ? this.getRawValue(item.keybinding) : ''; - const context = (item.context) ? this.getRawValue(item.context) : ''; const dialog = new EditKeybindingDialog({ title: `Edit Keybinding For ${command}`, initialValue: keybinding, @@ -534,7 +532,11 @@ export class KeybindingWidget extends ReactWidget { }, this.keymapsService, item); dialog.open().then(async newKeybinding => { if (newKeybinding) { - await this.keymapsService.setKeybinding({ 'command': id, 'keybinding': newKeybinding, 'context': context }); + await this.keymapsService.setKeybinding({ + command: item.command, + keybinding: newKeybinding, + when: item.context + }, keybinding); } }); } diff --git a/packages/keymaps/src/browser/keymaps-parser.ts b/packages/keymaps/src/browser/keymaps-parser.ts index b0548bb5c74a2..0b9f8380cd7cc 100644 --- a/packages/keymaps/src/browser/keymaps-parser.ts +++ b/packages/keymaps/src/browser/keymaps-parser.ts @@ -17,7 +17,7 @@ import * as Ajv from 'ajv'; import * as parser from 'jsonc-parser'; import { injectable } from 'inversify'; -import { Keybinding } from '@theia/core/lib/browser'; +import { Keybinding } from '@theia/core/lib/common/keybinding'; export const keymapsSchema = { type: 'array', diff --git a/packages/keymaps/src/browser/keymaps-service.ts b/packages/keymaps/src/browser/keymaps-service.ts index 27b4ad362cb29..82cbd0ee3fb1b 100644 --- a/packages/keymaps/src/browser/keymaps-service.ts +++ b/packages/keymaps/src/browser/keymaps-service.ts @@ -17,29 +17,13 @@ import { inject, injectable, postConstruct } from 'inversify'; import URI from '@theia/core/lib/common/uri'; import { ResourceProvider, Resource } from '@theia/core/lib/common'; -import { Keybinding, KeybindingRegistry, KeybindingScope, OpenerService, open, WidgetOpenerOptions, Widget } from '@theia/core/lib/browser'; +import { OpenerService, open, WidgetOpenerOptions, Widget } from '@theia/core/lib/browser'; +import { KeybindingRegistry, KeybindingScope } from '@theia/core/lib/browser/keybinding'; +import { Keybinding } from '@theia/core/lib/common/keybinding'; import { UserStorageUri } from '@theia/userstorage/lib/browser'; import { KeymapsParser } from './keymaps-parser'; import * as jsoncparser from 'jsonc-parser'; -import { Emitter } from '@theia/core/lib/common/'; - -/** - * Representation of a JSON keybinding. - */ -export interface KeybindingJson { - /** - * The keybinding command. - */ - command: string, - /** - * The actual keybinding. - */ - keybinding: string, - /** - * The keybinding context. - */ - context: string, -} +import { Emitter } from '@theia/core/lib/common/event'; @injectable() export class KeymapsService { @@ -109,25 +93,45 @@ export class KeymapsService { /** * Set the keybinding in the JSON. - * @param keybindingJson the JSON keybindings. + * @param newKeybinding the JSON keybindings. */ - async setKeybinding(keybindingJson: KeybindingJson): Promise { + async setKeybinding(newKeybinding: Keybinding, oldKeybinding: string): Promise { if (!this.resource.saveContents) { return; } const content = await this.resource.readContents(); - const keybindings: KeybindingJson[] = content ? jsoncparser.parse(content) : []; - let updated = false; - for (let i = 0; i < keybindings.length; i++) { - if (keybindings[i].command === keybindingJson.command) { - updated = true; - keybindings[i].keybinding = keybindingJson.keybinding; + const keybindings: Keybinding[] = content ? jsoncparser.parse(content) : []; + let newAdded = false; + let oldRemoved = false; + for (const keybinding of keybindings) { + if (keybinding.command === newKeybinding.command && + (keybinding.context || '') === (newKeybinding.context || '') && + (keybinding.when || '') === (newKeybinding.when || '')) { + newAdded = true; + keybinding.keybinding = newKeybinding.keybinding; + } + if (keybinding.command === '-' + newKeybinding.command && + keybinding.keybinding === oldKeybinding && + (keybinding.context || '') === (newKeybinding.context || '') && + (keybinding.when || '') === (newKeybinding.when || '')) { + oldRemoved = false; } } - if (!updated) { - const item: KeybindingJson = { ...keybindingJson }; - keybindings.push(item); + if (!newAdded) { + keybindings.push(newKeybinding); } + if (!oldRemoved) { + keybindings.push({ + command: '-' + newKeybinding.command, + // TODO key: oldKeybinding, see https://github.com/eclipse-theia/theia/issues/6879 + keybinding: oldKeybinding, + context: newKeybinding.context, + when: newKeybinding.when + }); + } + // TODO use preference values to get proper json settings + // TODO handle dirty models properly + // TODO handle race conditions properly await this.resource.saveContents(JSON.stringify(keybindings, undefined, 4)); } @@ -140,22 +144,13 @@ export class KeymapsService { return; } const content = await this.resource.readContents(); - const keybindings: KeybindingJson[] = content ? jsoncparser.parse(content) : []; - const filtered = keybindings.filter(a => a.command !== commandId); + const keybindings: Keybinding[] = content ? jsoncparser.parse(content) : []; + const removedCommand = '-' + commandId; + const filtered = keybindings.filter(a => a.command !== commandId && a.command !== removedCommand); + // TODO use preference values to get proper json settings + // TODO handle dirty models properly + // TODO handle race conditions properly await this.resource.saveContents(JSON.stringify(filtered, undefined, 4)); } - /** - * Get the list of keybindings from the JSON. - * - * @returns the list of keybindings in JSON. - */ - async getKeybindings(): Promise { - if (!this.resource.saveContents) { - return []; - } - const content = await this.resource.readContents(); - return content ? jsoncparser.parse(content) : []; - } - } diff --git a/packages/monaco/src/browser/monaco-editor-provider.ts b/packages/monaco/src/browser/monaco-editor-provider.ts index 3efec40d79afc..d4a0970b7aa39 100644 --- a/packages/monaco/src/browser/monaco-editor-provider.ts +++ b/packages/monaco/src/browser/monaco-editor-provider.ts @@ -125,6 +125,8 @@ export class MonacoEditorProvider { }, toDispose); editor.onDispose(() => toDispose.dispose()); + this.suppressMonaconKeybindingListener(editor); + const standaloneCommandService = new monaco.services.StandaloneCommandService(editor.instantiationService); commandService.setDelegate(standaloneCommandService); this.installQuickOpenService(editor); @@ -144,6 +146,24 @@ export class MonacoEditorProvider { return editor; } + /** + * Suppresses Monaco keydown listener to avoid triggering default Monaco keybindings + * if they are overriden by a user. Monaco keybindings should be registered as Theia keybindings + * to allow a user to customize them. + */ + protected suppressMonaconKeybindingListener(editor: MonacoEditor): void { + let keydownListener: monaco.IDisposable | undefined; + for (const listener of editor.getControl()._standaloneKeybindingService._store._toDispose) { + if ('_type' in listener && listener['_type'] === 'keydown') { + keydownListener = listener; + break; + } + } + if (keydownListener) { + keydownListener.dispose(); + } + } + protected createEditor(uri: URI, override: IEditorOverrideServices, toDispose: DisposableCollection): Promise { if (DiffUris.isDiffUri(uri)) { return this.createMonacoDiffEditor(uri, override, toDispose); diff --git a/packages/monaco/src/typings/monaco/index.d.ts b/packages/monaco/src/typings/monaco/index.d.ts index 39a847a28fad3..22bc2c62aad69 100644 --- a/packages/monaco/src/typings/monaco/index.d.ts +++ b/packages/monaco/src/typings/monaco/index.d.ts @@ -48,6 +48,11 @@ declare module monaco.editor { setDecorations(decorationTypeKey: string, ranges: IDecorationOptions[]): void; setDecorationsFast(decorationTypeKey: string, ranges: IRange[]): void; trigger(source: string, handlerId: string, payload: any): void + _standaloneKeybindingService: { + _store: { + _toDispose: monaco.IDisposable[] + } + } } // https://github.com/TypeFox/vscode/blob/monaco/0.18.0/src/vs/editor/browser/widget/codeEditorWidget.ts#L107