Skip to content

Commit

Permalink
Fixes #34031: Remove workaround for Backslash and IntlBackslash "swap…
Browse files Browse the repository at this point in the history 
…ped" on ISO keyboards - #24153
  • Loading branch information
@alexdima
alexdima committed Sep 21, 2017
1 parent 088bbdb commit 3a03183
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 151 deletions.
4 changes: 2 additions & 2 deletions src/vs/code/electron-main/app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@ export class CodeApplication {
});

// Keyboard layout changes
KeyboardLayoutMonitor.INSTANCE.onDidChangeKeyboardLayout(isISOKeyboard => {
KeyboardLayoutMonitor.INSTANCE.onDidChangeKeyboardLayout(() => {
if (this.windowsMainService) {
this.windowsMainService.sendToAll('vscode:keyboardLayoutChanged', isISOKeyboard);
this.windowsMainService.sendToAll('vscode:keyboardLayoutChanged', false);
}
});
}
Expand Down
44 changes: 4 additions & 40 deletions src/vs/code/electron-main/keyboard.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import * as nativeKeymap from 'native-keymap';
import { IDisposable } from 'vs/base/common/lifecycle';
import { isMacintosh } from 'vs/base/common/platform';
import { IStorageService } from 'vs/platform/storage/node/storage';
import Event, { Emitter, once } from 'vs/base/common/event';
import { ConfigWatcher } from 'vs/base/node/config';
Expand All @@ -21,59 +20,24 @@ export class KeyboardLayoutMonitor {

public static readonly INSTANCE = new KeyboardLayoutMonitor();

private _emitter: Emitter<boolean>;
private _emitter: Emitter<void>;
private _registered: boolean;
private _isISOKeyboard: boolean;

private constructor() {
this._emitter = new Emitter<boolean>();
this._emitter = new Emitter<void>();
this._registered = false;
this._isISOKeyboard = this._readIsISOKeyboard();
}

public onDidChangeKeyboardLayout(callback: (isISOKeyboard: boolean) => void): IDisposable {
public onDidChangeKeyboardLayout(callback: () => void): IDisposable {
if (!this._registered) {
this._registered = true;

nativeKeymap.onDidChangeKeyboardLayout(() => {
this._emitter.fire(this._isISOKeyboard);
this._emitter.fire();
});

if (isMacintosh) {
// See https://github.com/Microsoft/vscode/issues/24153
// On OSX, on ISO keyboards, Chromium swaps the scan codes
// of IntlBackslash and Backquote.
//
// The C++ methods can give the current keyboard type (ISO or not)
// only after a NSEvent was handled.
//
// We therefore poll.
setInterval(() => {
let newValue = this._readIsISOKeyboard();
if (this._isISOKeyboard === newValue) {
// no change
return;
}

this._isISOKeyboard = newValue;
this._emitter.fire(this._isISOKeyboard);

}, 3000);
}
}
return this._emitter.event(callback);
}

private _readIsISOKeyboard(): boolean {
// if (isMacintosh) {
// return nativeKeymap.isISOKeyboard();
// }
return false;
}

public isISOKeyboard(): boolean {
return this._isISOKeyboard;
}
}

export interface IKeybinding {
Expand Down
4 changes: 0 additions & 4 deletions src/vs/code/electron-main/window.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import product from 'vs/platform/node/product';
import pkg from 'vs/platform/node/package';
import { IWindowSettings, MenuBarVisibility, IWindowConfiguration, ReadyState, IRunActionInWindowRequest } from 'vs/platform/windows/common/windows';
import { IDisposable, dispose } from 'vs/base/common/lifecycle';
import { KeyboardLayoutMonitor } from 'vs/code/electron-main/keyboard';
import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform';
import { ICodeWindow } from 'vs/platform/windows/electron-main/windows';
import { IWorkspaceIdentifier, IWorkspacesMainService } from 'vs/platform/workspaces/common/workspaces';
Expand Down Expand Up @@ -550,9 +549,6 @@ export class CodeWindow implements ICodeWindow {
windowConfiguration.highContrast = isWindows && systemPreferences.isInvertedColorScheme() && (!windowConfig || windowConfig.autoDetectHighContrast);
windowConfiguration.accessibilitySupport = app.isAccessibilitySupportEnabled();

// Set Keyboard Config
windowConfiguration.isISOKeyboard = KeyboardLayoutMonitor.INSTANCE.isISOKeyboard();

// Theme
windowConfiguration.baseTheme = this.getBaseTheme();
windowConfiguration.backgroundColor = this.getBackgroundColor();
Expand Down
1 change: 0 additions & 1 deletion src/vs/platform/windows/common/windows.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ export interface IWindowConfiguration extends ParsedArgs, IOpenFileRequest {
workspace?: IWorkspaceIdentifier;
folderPath?: string;

isISOKeyboard?: boolean;
zoomLevel?: number;
fullscreen?: boolean;
highContrast?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/electron-browser/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function startup(configuration: IWindowConfiguration): TPromise<void> {

browser.setFullscreen(!!configuration.fullscreen);

KeyboardMapperFactory.INSTANCE._onKeyboardLayoutChanged(configuration.isISOKeyboard);
KeyboardMapperFactory.INSTANCE._onKeyboardLayoutChanged();

browser.setAccessibilitySupport(configuration.accessibilitySupport ? platform.AccessibilitySupport.Enabled : platform.AccessibilitySupport.Disabled);

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/electron-browser/window.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ export class ElectronWindow extends Themable {
});

// keyboard layout changed event
ipc.on('vscode:keyboardLayoutChanged', (event, isISOKeyboard: boolean) => {
KeyboardMapperFactory.INSTANCE._onKeyboardLayoutChanged(isISOKeyboard);
ipc.on('vscode:keyboardLayoutChanged', event => {
KeyboardMapperFactory.INSTANCE._onKeyboardLayoutChanged();
});

// keyboard layout changed event
Expand Down
22 changes: 1 addition & 21 deletions src/vs/workbench/services/keybinding/common/macLinuxKeyboardMapper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,10 +429,6 @@ class ScanCodeKeyCodeMapper {

export class MacLinuxKeyboardMapper implements IKeyboardMapper {

/**
* Is the keyboard type ISO (on Mac)
*/
private readonly _isISOKeyboard: boolean;
/**
* Is this the standard US keyboard layout?
*/
Expand All @@ -458,8 +454,7 @@ export class MacLinuxKeyboardMapper implements IKeyboardMapper {
*/
private readonly _scanCodeToDispatch: string[] = [];

constructor(isISOKeyboard: boolean, isUSStandard: boolean, rawMappings: IMacLinuxKeyboardMapping, OS: OperatingSystem) {
this._isISOKeyboard = isISOKeyboard;
constructor(isUSStandard: boolean, rawMappings: IMacLinuxKeyboardMapping, OS: OperatingSystem) {
this._isUSStandard = isUSStandard;
this._OS = OS;
this._codeInfo = [];
Expand Down Expand Up @@ -1055,21 +1050,6 @@ export class MacLinuxKeyboardMapper implements IKeyboardMapper {
code = ScanCode.Enter;
}

if (this._OS === OperatingSystem.Macintosh && this._isISOKeyboard) {
// See https://github.com/Microsoft/vscode/issues/24153
// On OSX, on ISO keyboards, Chromium swaps the scan codes
// of IntlBackslash and Backquote.

switch (code) {
case ScanCode.IntlBackslash:
code = ScanCode.Backquote;
break;
case ScanCode.Backquote:
code = ScanCode.IntlBackslash;
break;
}
}

const keyCode = keyboardEvent.keyCode;

if (
Expand Down
26 changes: 10 additions & 16 deletions src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import { onUnexpectedError } from 'vs/base/common/errors';
export class KeyboardMapperFactory {
public static INSTANCE = new KeyboardMapperFactory();

private _isISOKeyboard: boolean;
private _layoutInfo: nativeKeymap.IKeyboardLayoutInfo;
private _rawMapping: nativeKeymap.IKeyboardMapping;
private _keyboardMapper: IKeyboardMapper;
Expand All @@ -50,25 +49,21 @@ export class KeyboardMapperFactory {
public onDidChangeKeyboardMapper: Event<void> = this._onDidChangeKeyboardMapper.event;

private constructor() {
this._isISOKeyboard = false;
this._layoutInfo = null;
this._rawMapping = null;
this._keyboardMapper = null;
this._initialized = false;
}

public _onKeyboardLayoutChanged(isISOKeyboard: boolean): void {
isISOKeyboard = !!isISOKeyboard;
public _onKeyboardLayoutChanged(): void {
if (this._initialized) {
this._setKeyboardData(isISOKeyboard, nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
} else {
this._isISOKeyboard = isISOKeyboard;
this._setKeyboardData(nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
}
}

public getKeyboardMapper(dispatchConfig: DispatchConfig): IKeyboardMapper {
if (!this._initialized) {
this._setKeyboardData(this._isISOKeyboard, nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
this._setKeyboardData(nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
}
if (dispatchConfig === DispatchConfig.KeyCode) {
// Forcefully set to use keyCode
Expand All @@ -79,7 +74,7 @@ export class KeyboardMapperFactory {

public getCurrentKeyboardLayout(): nativeKeymap.IKeyboardLayoutInfo {
if (!this._initialized) {
this._setKeyboardData(this._isISOKeyboard, nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
this._setKeyboardData(nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
}
return this._layoutInfo;
}
Expand All @@ -105,27 +100,26 @@ export class KeyboardMapperFactory {

public getRawKeyboardMapping(): nativeKeymap.IKeyboardMapping {
if (!this._initialized) {
this._setKeyboardData(this._isISOKeyboard, nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
this._setKeyboardData(nativeKeymap.getCurrentKeyboardLayout(), nativeKeymap.getKeyMap());
}
return this._rawMapping;
}

private _setKeyboardData(isISOKeyboard: boolean, layoutInfo: nativeKeymap.IKeyboardLayoutInfo, rawMapping: nativeKeymap.IKeyboardMapping): void {
private _setKeyboardData(layoutInfo: nativeKeymap.IKeyboardLayoutInfo, rawMapping: nativeKeymap.IKeyboardMapping): void {
this._layoutInfo = layoutInfo;

if (this._initialized && this._isISOKeyboard === isISOKeyboard && KeyboardMapperFactory._equals(this._rawMapping, rawMapping)) {
if (this._initialized && KeyboardMapperFactory._equals(this._rawMapping, rawMapping)) {
// nothing to do...
return;
}

this._initialized = true;
this._isISOKeyboard = isISOKeyboard;
this._rawMapping = rawMapping;
this._keyboardMapper = KeyboardMapperFactory._createKeyboardMapper(this._isISOKeyboard, this._layoutInfo, this._rawMapping);
this._keyboardMapper = KeyboardMapperFactory._createKeyboardMapper(this._layoutInfo, this._rawMapping);
this._onDidChangeKeyboardMapper.fire();
}

private static _createKeyboardMapper(isISOKeyboard: boolean, layoutInfo: nativeKeymap.IKeyboardLayoutInfo, rawMapping: nativeKeymap.IKeyboardMapping): IKeyboardMapper {
private static _createKeyboardMapper(layoutInfo: nativeKeymap.IKeyboardLayoutInfo, rawMapping: nativeKeymap.IKeyboardMapping): IKeyboardMapper {
const isUSStandard = KeyboardMapperFactory._isUSStandard(layoutInfo);
if (OS === OperatingSystem.Windows) {
return new WindowsKeyboardMapper(isUSStandard, <IWindowsKeyboardMapping>rawMapping);
Expand All @@ -144,7 +138,7 @@ export class KeyboardMapperFactory {
}
}

return new MacLinuxKeyboardMapper(isISOKeyboard, isUSStandard, <IMacLinuxKeyboardMapping>rawMapping, OS);
return new MacLinuxKeyboardMapper(isUSStandard, <IMacLinuxKeyboardMapping>rawMapping, OS);
}

private static _equals(a: nativeKeymap.IKeyboardMapping, b: nativeKeymap.IKeyboardMapping): boolean {
Expand Down
68 changes: 4 additions & 64 deletions src/vs/workbench/services/keybinding/test/macLinuxKeyboardMapper.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const WRITE_FILE_IF_DIFFERENT = false;

function createKeyboardMapper(isUSStandard: boolean, file: string, OS: OperatingSystem): TPromise<MacLinuxKeyboardMapper> {
return readRawMapping<IMacLinuxKeyboardMapping>(file).then((rawMappings) => {
return new MacLinuxKeyboardMapper(false, isUSStandard, rawMappings, OS);
return new MacLinuxKeyboardMapper(isUSStandard, rawMappings, OS);
});
}

Expand Down Expand Up @@ -1202,7 +1202,7 @@ suite('keyboardMapper - LINUX en_us', () => {
suite('keyboardMapper', () => {

test('issue #23706: Linux UK layout: Ctrl + Apostrophe also toggles terminal', () => {
let mapper = new MacLinuxKeyboardMapper(false, false, {
let mapper = new MacLinuxKeyboardMapper(false, {
'Backquote': {
'value': '`',
'withShift': '¬',
Expand Down Expand Up @@ -1234,7 +1234,7 @@ suite('keyboardMapper', () => {
});

test('issue #24064: NumLock/NumPad keys stopped working in 1.11 on Linux', () => {
let mapper = new MacLinuxKeyboardMapper(false, false, {}, OperatingSystem.Linux);
let mapper = new MacLinuxKeyboardMapper(false, {}, OperatingSystem.Linux);

function assertNumpadKeyboardEvent(keyCode: KeyCode, code: string, label: string, electronAccelerator: string, userSettingsLabel: string, dispatch: string): void {
assertResolveKeyboardEvent(
Expand Down Expand Up @@ -1273,7 +1273,7 @@ suite('keyboardMapper', () => {
});

test('issue #24107: Delete, Insert, Home, End, PgUp, PgDn, and arrow keys no longer work editor in 1.11', () => {
let mapper = new MacLinuxKeyboardMapper(false, false, {}, OperatingSystem.Linux);
let mapper = new MacLinuxKeyboardMapper(false, {}, OperatingSystem.Linux);

function assertKeyboardEvent(keyCode: KeyCode, code: string, label: string, electronAccelerator: string, userSettingsLabel: string, dispatch: string): void {
assertResolveKeyboardEvent(
Expand Down Expand Up @@ -1322,66 +1322,6 @@ suite('keyboardMapper', () => {
assertKeyboardEvent(KeyCode.DownArrow, 'NumpadEnter', 'DownArrow', 'Down', 'down', '[ArrowDown]');
assertKeyboardEvent(KeyCode.UpArrow, 'Lang3', 'UpArrow', 'Up', 'up', '[ArrowUp]');
});

test('issue #24153: ISO Keyboards: Backslash and IntlBackslash "swapped"', () => {
let mapper = new MacLinuxKeyboardMapper(true, false, {
'Backquote': {
'value': '`',
'withShift': '~',
'withAltGr': '`',
'withShiftAltGr': '`'
},
'IntlBackslash': {
'value': '§',
'withShift': '°',
'withAltGr': '§',
'withShiftAltGr': '°'
}
}, OperatingSystem.Macintosh);

assertResolveKeyboardEvent(
mapper,
{
ctrlKey: true,
shiftKey: false,
altKey: false,
metaKey: false,
keyCode: -1,
code: 'Backquote'
},
{
label: '⌃§',
ariaLabel: 'Control+§',
electronAccelerator: null,
userSettingsLabel: 'ctrl+[IntlBackslash]',
isWYSIWYG: false,
isChord: false,
dispatchParts: ['ctrl+[IntlBackslash]', null],
}
);

assertResolveKeyboardEvent(
mapper,
{
ctrlKey: true,
shiftKey: false,
altKey: false,
metaKey: false,
keyCode: -1,
code: 'IntlBackslash'
},
{
label: '⌃`',
ariaLabel: 'Control+`',
electronAccelerator: null,
userSettingsLabel: 'ctrl+`',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['ctrl+[Backquote]', null],
}
);
});

});

suite('keyboardMapper - LINUX ru', () => {
Expand Down

0 comments on commit 3a03183

Please sign in to comment.