From 3801e1ac7b32c34a24d9c90a32e509a801175b69 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Fri, 28 Jun 2019 11:53:00 -0700 Subject: [PATCH] Show notification when missing keyboard layout --- .../keybinding/browser/keymapService.ts | 106 ++++++++++++------ .../test/browserKeyboardMapper.test.ts | 12 +- 2 files changed, 80 insertions(+), 38 deletions(-) diff --git a/src/vs/workbench/services/keybinding/browser/keymapService.ts b/src/vs/workbench/services/keybinding/browser/keymapService.ts index cff794267bf..590dab59664 100644 --- a/src/vs/workbench/services/keybinding/browser/keymapService.ts +++ b/src/vs/workbench/services/keybinding/browser/keymapService.ts @@ -27,6 +27,8 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions as ConfigExtensions, IConfigurationRegistry, IConfigurationNode } from 'vs/platform/configuration/common/configurationRegistry'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { INavigatorWithKeyboard } from 'vs/workbench/services/keybinding/common/navigatorKeyboard'; +import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; +import { ICommandService } from 'vs/platform/commands/common/commands'; export class BrowserKeyboardMapperFactoryBase { // keyboard mapper @@ -68,7 +70,10 @@ export class BrowserKeyboardMapperFactoryBase { return this._keymapInfos.map(keymapInfo => keymapInfo.layout); } - protected constructor() { + protected constructor( + private _notificationService: INotificationService, + private _commandService: ICommandService + ) { this._keyboardMapper = null; this._initialized = false; this._keymapInfos = []; @@ -101,7 +106,7 @@ export class BrowserKeyboardMapperFactoryBase { this._keymapInfos.splice(index, 1); } - getMatchedKeymapInfo(keyMapping: IKeyboardMapping | null): KeymapInfo | null { + getMatchedKeymapInfo(keyMapping: IKeyboardMapping | null): { result: KeymapInfo, score: number } | null { if (!keyMapping) { return null; } @@ -111,7 +116,10 @@ export class BrowserKeyboardMapperFactoryBase { if (usStandard) { let maxScore = usStandard.getScore(keyMapping); if (maxScore === 0) { - return usStandard; + return { + result: usStandard, + score: 0 + }; } let result = usStandard; @@ -119,7 +127,10 @@ export class BrowserKeyboardMapperFactoryBase { let score = this._mru[i].getScore(keyMapping); if (score > maxScore) { if (score === 0) { - return this._mru[i]; + return { + result: this._mru[i], + score: 0 + }; } maxScore = score; @@ -127,12 +138,18 @@ export class BrowserKeyboardMapperFactoryBase { } } - return result; + return { + result, + score: maxScore + }; } for (let i = 0; i < this._mru.length; i++) { if (this._mru[i].fuzzyEqual(keyMapping)) { - return this._mru[i]; + return { + result: this._mru[i], + score: 0 + }; } } @@ -160,11 +177,27 @@ export class BrowserKeyboardMapperFactoryBase { setActiveKeyMapping(keymap: IKeyboardMapping | null) { let matchedKeyboardLayout = this.getMatchedKeymapInfo(keymap); if (matchedKeyboardLayout) { + let score = matchedKeyboardLayout.score; + + if (keymap && score < 0) { + // the keyboard layout doesn't actually match the key event or the keymap from chromium + this._notificationService.prompt( + Severity.Info, + nls.localize('missing.keyboardlayout', 'Fail to find matching keyboard layout'), + [{ + label: nls.localize('keyboardLayoutMissing.configure', "Configure"), + run: () => this._commandService.executeCommand('workbench.action.openKeyboardLayoutPicker') + }], + ); + + return; + } + if (!this._activeKeymapInfo) { - this._activeKeymapInfo = matchedKeyboardLayout; + this._activeKeymapInfo = matchedKeyboardLayout.result; } else if (keymap) { - if (matchedKeyboardLayout.getScore(keymap) > this._activeKeymapInfo.getScore(keymap)) { - this._activeKeymapInfo = matchedKeyboardLayout; + if (matchedKeyboardLayout.result.getScore(keymap) > this._activeKeymapInfo.getScore(keymap)) { + this._activeKeymapInfo = matchedKeyboardLayout.result; } } } @@ -341,13 +374,15 @@ export class BrowserKeyboardMapperFactoryBase { }; } - const matchedKeyboardLayout = this.getMatchedKeymapInfo(ret); + return ret; - if (matchedKeyboardLayout) { - return matchedKeyboardLayout.mapping; - } + // const matchedKeyboardLayout = this.getMatchedKeymapInfo(ret); + + // if (matchedKeyboardLayout) { + // return matchedKeyboardLayout.result.mapping; + // } - return null; + // return null; }); } catch { // getLayoutMap can throw if invoked from a nested browsing context @@ -378,11 +413,8 @@ export class BrowserKeyboardMapperFactoryBase { } export class BrowserKeyboardMapperFactory extends BrowserKeyboardMapperFactoryBase { - public static readonly INSTANCE = new BrowserKeyboardMapperFactory(); - // keyboard mapper - - private constructor() { - super(); + constructor(notificationService: INotificationService, commandService: ICommandService) { + super(notificationService, commandService); const platform = isWindows ? 'win' : isMacintosh ? 'darwin' : 'linux'; @@ -511,21 +543,25 @@ class BrowserKeymapService extends Disposable implements IKeymapService { private _userKeyboardLayout: UserKeyboardLayout; private readonly layoutChangeListener = this._register(new MutableDisposable()); + private readonly _factory: BrowserKeyboardMapperFactory; constructor( @IEnvironmentService environmentService: IEnvironmentService, @IFileService fileService: IFileService, + @INotificationService notificationService: INotificationService, + @ICommandService commandService: ICommandService, @IConfigurationService private configurationService: IConfigurationService, ) { super(); const keyboardConfig = configurationService.getValue<{ layout: string }>('keyboard'); const layout = keyboardConfig.layout; + this._factory = new BrowserKeyboardMapperFactory(notificationService, commandService); this.registerKeyboardListener(); if (layout && layout !== 'autodetect') { // set keyboard layout - BrowserKeyboardMapperFactory.INSTANCE.setKeyboardLayout(layout); + this._factory.setKeyboardLayout(layout); } this._register(configurationService.onDidChangeConfiguration(e => { @@ -535,9 +571,9 @@ class BrowserKeymapService extends Disposable implements IKeymapService { if (layout === 'autodetect') { this.registerKeyboardListener(); - BrowserKeyboardMapperFactory.INSTANCE.onKeyboardLayoutChanged(); + this._factory.onKeyboardLayoutChanged(); } else { - BrowserKeyboardMapperFactory.INSTANCE.setKeyboardLayout(layout); + this._factory.setKeyboardLayout(layout); this.layoutChangeListener.clear(); } } @@ -546,24 +582,24 @@ class BrowserKeymapService extends Disposable implements IKeymapService { this._userKeyboardLayout = new UserKeyboardLayout(environmentService.keyboardLayoutResource, fileService); this._userKeyboardLayout.initialize().then(() => { if (this._userKeyboardLayout.keyboardLayout) { - BrowserKeyboardMapperFactory.INSTANCE.registerKeyboardLayout(this._userKeyboardLayout.keyboardLayout); + this._factory.registerKeyboardLayout(this._userKeyboardLayout.keyboardLayout); this.setUserKeyboardLayoutIfMatched(); } }); this._register(this._userKeyboardLayout.onDidChange(() => { - let userKeyboardLayouts = BrowserKeyboardMapperFactory.INSTANCE.keymapInfos.filter(layout => layout.isUserKeyboardLayout); + let userKeyboardLayouts = this._factory.keymapInfos.filter(layout => layout.isUserKeyboardLayout); if (userKeyboardLayouts.length) { if (this._userKeyboardLayout.keyboardLayout) { userKeyboardLayouts[0].update(this._userKeyboardLayout.keyboardLayout); } else { - BrowserKeyboardMapperFactory.INSTANCE.removeKeyboardLayout(userKeyboardLayouts[0]); + this._factory.removeKeyboardLayout(userKeyboardLayouts[0]); } } else { if (this._userKeyboardLayout.keyboardLayout) { - BrowserKeyboardMapperFactory.INSTANCE.registerKeyboardLayout(this._userKeyboardLayout.keyboardLayout); + this._factory.registerKeyboardLayout(this._userKeyboardLayout.keyboardLayout); } } @@ -576,39 +612,39 @@ class BrowserKeymapService extends Disposable implements IKeymapService { const layout = keyboardConfig.layout; if (layout && this._userKeyboardLayout.keyboardLayout) { - if (getKeyboardLayoutId(this._userKeyboardLayout.keyboardLayout.layout) === layout && BrowserKeyboardMapperFactory.INSTANCE.activeKeymap) { + if (getKeyboardLayoutId(this._userKeyboardLayout.keyboardLayout.layout) === layout && this._factory.activeKeymap) { - if (!this._userKeyboardLayout.keyboardLayout.equal(BrowserKeyboardMapperFactory.INSTANCE.activeKeymap)) { - BrowserKeyboardMapperFactory.INSTANCE.setActiveKeymapInfo(this._userKeyboardLayout.keyboardLayout); + if (!this._userKeyboardLayout.keyboardLayout.equal(this._factory.activeKeymap)) { + this._factory.setActiveKeymapInfo(this._userKeyboardLayout.keyboardLayout); } } } } registerKeyboardListener() { - this.layoutChangeListener.value = BrowserKeyboardMapperFactory.INSTANCE.onDidChangeKeyboardMapper(() => { + this.layoutChangeListener.value = this._factory.onDidChangeKeyboardMapper(() => { this._onDidChangeKeyboardMapper.fire(); }); } getKeyboardMapper(dispatchConfig: DispatchConfig): IKeyboardMapper { - return BrowserKeyboardMapperFactory.INSTANCE.getKeyboardMapper(dispatchConfig); + return this._factory.getKeyboardMapper(dispatchConfig); } public getCurrentKeyboardLayout(): IKeyboardLayoutInfo | null { - return BrowserKeyboardMapperFactory.INSTANCE.activeKeyboardLayout; + return this._factory.activeKeyboardLayout; } public getAllKeyboardLayouts(): IKeyboardLayoutInfo[] { - return BrowserKeyboardMapperFactory.INSTANCE.keyboardLayouts; + return this._factory.keyboardLayouts; } public getRawKeyboardMapping(): IKeyboardMapping | null { - return BrowserKeyboardMapperFactory.INSTANCE.activeKeyMapping; + return this._factory.activeKeyMapping; } public validateCurrentKeyboardMapping(keyboardEvent: IKeyboardEvent): void { - BrowserKeyboardMapperFactory.INSTANCE.validateCurrentKeyboardMapping(keyboardEvent); + this._factory.validateCurrentKeyboardMapping(keyboardEvent); } } diff --git a/src/vs/workbench/services/keybinding/test/browserKeyboardMapper.test.ts b/src/vs/workbench/services/keybinding/test/browserKeyboardMapper.test.ts index 8c06d950aed..1099564ebf7 100644 --- a/src/vs/workbench/services/keybinding/test/browserKeyboardMapper.test.ts +++ b/src/vs/workbench/services/keybinding/test/browserKeyboardMapper.test.ts @@ -8,10 +8,13 @@ import 'vs/workbench/services/keybinding/browser/keyboardLayouts/de.darwin'; import { KeyboardLayoutContribution } from 'vs/workbench/services/keybinding/browser/keyboardLayouts/_.contribution'; import { BrowserKeyboardMapperFactoryBase } from '../browser/keymapService'; import { KeymapInfo, IKeymapInfo } from '../common/keymapInfo'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { INotificationService } from 'vs/platform/notification/common/notification'; +import { ICommandService } from 'vs/platform/commands/common/commands'; class TestKeyboardMapperFactory extends BrowserKeyboardMapperFactoryBase { - constructor() { - super(); + constructor(notificationService: INotificationService, commandService: ICommandService) { + super(notificationService, commandService); let keymapInfos: IKeymapInfo[] = KeyboardLayoutContribution.INSTANCE.layoutInfos; this._keymapInfos.push(...keymapInfos.map(info => (new KeymapInfo(info.layout, info.secondaryLayouts, info.mapping, info.isUserKeyboardLayout)))); @@ -23,7 +26,10 @@ class TestKeyboardMapperFactory extends BrowserKeyboardMapperFactoryBase { suite('keyboard layout loader', () => { - const instance: TestKeyboardMapperFactory = new TestKeyboardMapperFactory(); + let instantiationService: TestInstantiationService = new TestInstantiationService(); + let notitifcationService = instantiationService.stub(INotificationService, {}); + let commandService = instantiationService.stub(ICommandService, {}); + let instance = new TestKeyboardMapperFactory(notitifcationService, commandService); test('load default US keyboard layout', () => { assert.notEqual(instance.activeKeyboardLayout, null); -- GitLab