From d01fcd3168fd9bdabdad37cd065189f6088d6dcb Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 6 Aug 2018 11:38:43 +0200 Subject: [PATCH] move "keyevent to printbale key"-logic into service, removes duplicated code and fixes #55387 --- .../common/abstractKeybindingService.ts | 16 +++++++++++++- .../platform/keybinding/common/keybinding.ts | 6 ++++++ .../test/common/mockKeybindingService.ts | 4 ++++ src/vs/platform/list/browser/listService.ts | 15 ++++--------- .../outline/electron-browser/outlinePanel.ts | 21 +++---------------- .../electron-browser/keybindingService.ts | 21 +++++++++++++++++++ 6 files changed, 53 insertions(+), 30 deletions(-) diff --git a/src/vs/platform/keybinding/common/abstractKeybindingService.ts b/src/vs/platform/keybinding/common/abstractKeybindingService.ts index 075cb92fe87..4c6c989f169 100644 --- a/src/vs/platform/keybinding/common/abstractKeybindingService.ts +++ b/src/vs/platform/keybinding/common/abstractKeybindingService.ts @@ -5,7 +5,7 @@ 'use strict'; import * as nls from 'vs/nls'; -import { ResolvedKeybinding, Keybinding } from 'vs/base/common/keyCodes'; +import { ResolvedKeybinding, Keybinding, KeyCode } from 'vs/base/common/keyCodes'; import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { KeybindingResolver, IResolveResult } from 'vs/platform/keybinding/common/keybindingResolver'; @@ -204,4 +204,18 @@ export abstract class AbstractKeybindingService extends Disposable implements IK return shouldPreventDefault; } + + mightProducePrintableCharacter(event: IKeyboardEvent): boolean { + if (event.ctrlKey || event.metaKey) { + // ignore ctrl/cmd-combination but not shift/alt-combinatios + return false; + } + // weak check for certain ranges. this is properly implemented in a subclass + // with access to the KeyboardMapperFactory. + if ((event.keyCode >= KeyCode.KEY_A && event.keyCode <= KeyCode.KEY_Z) + || (event.keyCode >= KeyCode.KEY_0 && event.keyCode <= KeyCode.KEY_9)) { + return true; + } + return false; + } } diff --git a/src/vs/platform/keybinding/common/keybinding.ts b/src/vs/platform/keybinding/common/keybinding.ts index c491ec7a8d0..7110738c198 100644 --- a/src/vs/platform/keybinding/common/keybinding.ts +++ b/src/vs/platform/keybinding/common/keybinding.ts @@ -77,5 +77,11 @@ export interface IKeybindingService { getKeybindings(): ResolvedKeybindingItem[]; customKeybindingsCount(): number; + + /** + * Will the given key event produce a character that's rendered on screen, e.g. in a + * text box. *Note* that the results of this function can be incorrect. + */ + mightProducePrintableCharacter(event: IKeyboardEvent): boolean; } diff --git a/src/vs/platform/keybinding/test/common/mockKeybindingService.ts b/src/vs/platform/keybinding/test/common/mockKeybindingService.ts index 8a44c7bd8db..a8b00dfb030 100644 --- a/src/vs/platform/keybinding/test/common/mockKeybindingService.ts +++ b/src/vs/platform/keybinding/test/common/mockKeybindingService.ts @@ -124,4 +124,8 @@ export class MockKeybindingService implements IKeybindingService { dispatchEvent(e: IKeyboardEvent, target: IContextKeyServiceTarget): boolean { return false; } + + mightProducePrintableCharacter(e: IKeyboardEvent): boolean { + return false; + } } diff --git a/src/vs/platform/list/browser/listService.ts b/src/vs/platform/list/browser/listService.ts index d40542626af..8df85fe9180 100644 --- a/src/vs/platform/list/browser/listService.ts +++ b/src/vs/platform/list/browser/listService.ts @@ -31,6 +31,7 @@ import { TPromise } from 'vs/base/common/winjs.base'; import { onUnexpectedError, canceled } from 'vs/base/common/errors'; import { KeyCode } from 'vs/base/common/keyCodes'; import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent'; +import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; export type ListWidget = List | PagedList | ITree; @@ -578,7 +579,8 @@ export class HighlightingTreeController extends WorkbenchTreeController { constructor( options: IControllerOptions, private readonly onType: () => any, - @IConfigurationService configurationService: IConfigurationService + @IConfigurationService configurationService: IConfigurationService, + @IKeybindingService private readonly _keybindingService: IKeybindingService, ) { super(options, configurationService); } @@ -591,16 +593,7 @@ export class HighlightingTreeController extends WorkbenchTreeController { if (this.upKeyBindingDispatcher.has(event.keyCode)) { return false; } - if (event.ctrlKey || event.metaKey) { - // ignore ctrl/cmd-combination but not shift/alt-combinatios - return false; - } - // crazy -> during keydown focus moves to the input box - // and because of that the keyup event is handled by the - // input field - if (event.keyCode >= KeyCode.KEY_A && event.keyCode <= KeyCode.KEY_Z) { - // todo@joh this is much weaker than using the KeyboardMapperFactory - // but due to layering-challanges that's not available here... + if (this._keybindingService.mightProducePrintableCharacter(event)) { this.onType(); return true; } diff --git a/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts b/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts index c61097b03d3..4123498f29d 100644 --- a/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts +++ b/src/vs/workbench/parts/outline/electron-browser/outlinePanel.ts @@ -50,7 +50,6 @@ import { IViewletViewOptions } from 'vs/workbench/browser/parts/views/viewsViewl import { CollapseAction } from 'vs/workbench/browser/viewlet'; import { IViewsService } from 'vs/workbench/common/views'; import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; -import { KeyboardMapperFactory } from 'vs/workbench/services/keybinding/electron-browser/keybindingService'; import { OutlineConfigKeys, OutlineViewFiltered, OutlineViewFocused, OutlineViewId } from './outline'; import { OutlineController, OutlineDataSource, OutlineItemComparator, OutlineItemCompareType, OutlineItemFilter, OutlineRenderer, OutlineTreeState } from '../../../../editor/contrib/documentSymbols/outlineTree'; import { IResourceInput } from 'vs/platform/editor/common/editor'; @@ -256,12 +255,12 @@ export class OutlinePanel extends ViewletPanel { @IEditorService private readonly _editorService: IEditorService, @IMarkerService private readonly _markerService: IMarkerService, @IConfigurationService private readonly _configurationService: IConfigurationService, + @IKeybindingService private readonly _keybindingService: IKeybindingService, @IConfigurationService configurationService: IConfigurationService, - @IKeybindingService keybindingService: IKeybindingService, @IContextKeyService contextKeyService: IContextKeyService, @IContextMenuService contextMenuService: IContextMenuService, ) { - super(options, keybindingService, contextMenuService, configurationService); + super(options, _keybindingService, contextMenuService, configurationService); this._outlineViewState.restore(this._storageService); this._contextKeyFocused = OutlineViewFocused.bindTo(contextKeyService); this._contextKeyFiltered = OutlineViewFiltered.bindTo(contextKeyService); @@ -326,8 +325,6 @@ export class OutlinePanel extends ViewletPanel { const $this = this; const controller = new class extends OutlineController { - private readonly _mapper = KeyboardMapperFactory.INSTANCE; - constructor() { super({}, $this.configurationService); } @@ -340,22 +337,10 @@ export class OutlinePanel extends ViewletPanel { if (this.upKeyBindingDispatcher.has(event.keyCode)) { return false; } - if (event.ctrlKey || event.metaKey) { - // ignore ctrl/cmd-combination but not shift/alt-combinatios - return false; - } // crazy -> during keydown focus moves to the input box // and because of that the keyup event is handled by the // input field - const mapping = this._mapper.getRawKeyboardMapping(); - if (!mapping) { - return false; - } - const keyInfo = mapping[event.code]; - if (!keyInfo) { - return false; - } - if (keyInfo.value) { + if ($this._keybindingService.mightProducePrintableCharacter(event)) { $this._input.focus(); return true; } diff --git a/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts b/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts index c0521529d6e..775b74e6e76 100644 --- a/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts +++ b/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts @@ -540,6 +540,27 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService { let pretty = unboundCommands.sort().join('\n// - '); return '// ' + nls.localize('unboundCommands', "Here are other available commands: ") + '\n// - ' + pretty; } + + mightProducePrintableCharacter(event: IKeyboardEvent): boolean { + if (event.ctrlKey || event.metaKey) { + // ignore ctrl/cmd-combination but not shift/alt-combinatios + return false; + } + // consult the KeyboardMapperFactory to check the given event for + // a printable value. + const mapping = KeyboardMapperFactory.INSTANCE.getRawKeyboardMapping(); + if (!mapping) { + return false; + } + const keyInfo = mapping[event.code]; + if (!keyInfo) { + return false; + } + if (keyInfo.value) { + return true; + } + return false; + } } let schemaId = 'vscode://schemas/keybindings'; -- GitLab