Reduce usage of BinaryKeybindings

src/vs/base/common/keyCodes.ts

@@ -438,7 +438,7 @@ export function KeyChord(firstPart: number, secondPart: number): number {
 	return (firstPart | chordPart) >>> 0;
 }
 
-export class BinaryKeybindings {
+class BinaryKeybindings {
 
 	public static extractFirstPart(keybinding: number): number {
 		return (keybinding & 0x0000ffff) >>> 0;

src/vs/base/common/keybinding.ts

@@ -8,7 +8,7 @@
 import * as nls from 'vs/nls';
 import * as defaultPlatform from 'vs/base/common/platform';
 import { IHTMLContentElement } from 'vs/base/common/htmlContent';
-import { Keybinding, KeyCode, KeyMod, KeyChord, KeyCodeUtils, BinaryKeybindings, USER_SETTINGS } from 'vs/base/common/keyCodes';
+import { Keybinding, SimpleKeybinding, KeyCode, KeyMod, KeyChord, KeyCodeUtils, USER_SETTINGS } from 'vs/base/common/keyCodes';
 
 export interface ISimplifiedPlatform {
 	isMacintosh: boolean;
@@ -41,8 +41,8 @@ export class KeybindingLabels {
 	 * Format the binding to a format appropiate for the user settings file.
 	 * @internal
 	 */
-	public static toUserSettingsLabel(value: number, Platform: ISimplifiedPlatform = defaultPlatform): string {
-		let result = _asString(value, UserSettingsKeyLabelProvider.INSTANCE, Platform);
+	public static toUserSettingsLabel(keybinding: Keybinding, Platform: ISimplifiedPlatform = defaultPlatform): string {
+		let result = _asString(keybinding, UserSettingsKeyLabelProvider.INSTANCE, Platform);
 		result = result.toLowerCase();
 
 		if (Platform.isMacintosh) {
@@ -146,7 +146,7 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static _toUSLabel(keybinding: Keybinding, Platform: ISimplifiedPlatform = defaultPlatform): string {
-		return _asString(keybinding.value, (Platform.isMacintosh ? MacUIKeyLabelProvider.INSTANCE : ClassicUIKeyLabelProvider.INSTANCE), Platform);
+		return _asString(keybinding, (Platform.isMacintosh ? MacUIKeyLabelProvider.INSTANCE : ClassicUIKeyLabelProvider.INSTANCE), Platform);
 	}
 
 	/**
@@ -154,7 +154,7 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static _toUSAriaLabel(keybinding: Keybinding, Platform: ISimplifiedPlatform = defaultPlatform): string {
-		return _asString(keybinding.value, AriaKeyLabelProvider.INSTANCE, Platform);
+		return _asString(keybinding, AriaKeyLabelProvider.INSTANCE, Platform);
 	}
 
 	/**
@@ -162,7 +162,7 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static _toUSHTMLLabel(keybinding: Keybinding, Platform: ISimplifiedPlatform = defaultPlatform): IHTMLContentElement[] {
-		return _asHTML(keybinding.value, (Platform.isMacintosh ? MacUIKeyLabelProvider.INSTANCE : ClassicUIKeyLabelProvider.INSTANCE), Platform);
+		return _asHTML(keybinding, (Platform.isMacintosh ? MacUIKeyLabelProvider.INSTANCE : ClassicUIKeyLabelProvider.INSTANCE), Platform);
 	}
 
 	/**
@@ -170,7 +170,7 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static toCustomLabel(keybinding: Keybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform = defaultPlatform): string {
-		return _asString(keybinding.value, labelProvider, Platform);
+		return _asString(keybinding, labelProvider, Platform);
 	}
 
 	/**
@@ -178,7 +178,7 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static toCustomHTMLLabel(keybinding: Keybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform = defaultPlatform): IHTMLContentElement[] {
-		return _asHTML(keybinding.value, labelProvider, Platform);
+		return _asHTML(keybinding, labelProvider, Platform);
 	}
 
 	/**
@@ -187,16 +187,16 @@ export class KeybindingLabels {
 	 * @internal
 	 */
 	public static _toElectronAccelerator(keybinding: Keybinding, Platform: ISimplifiedPlatform = defaultPlatform): string {
-		if (BinaryKeybindings.hasChord(keybinding.value)) {
+		if (keybinding.isChord()) {
 			// Electron cannot handle chords
 			return null;
 		}
-		let keyCode = BinaryKeybindings.extractKeyCode(keybinding.value);
+		let keyCode = keybinding.getKeyCode();
 		if (keyCode >= KeyCode.NUMPAD_0 && keyCode <= KeyCode.NUMPAD_DIVIDE) {
 			// Electron cannot handle numpad keys
 			return null;
 		}
-		return _asString(keybinding.value, ElectronAcceleratorLabelProvider.INSTANCE, Platform);
+		return _asString(keybinding, ElectronAcceleratorLabelProvider.INSTANCE, Platform);
 	}
 }
 
@@ -328,13 +328,13 @@ class UserSettingsKeyLabelProvider implements IKeyBindingLabelProvider {
 	}
 }
 
-function _asString(keybinding: number, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform): string {
-	let result: string[] = [],
-		ctrlCmd = BinaryKeybindings.hasCtrlCmd(keybinding),
-		shift = BinaryKeybindings.hasShift(keybinding),
-		alt = BinaryKeybindings.hasAlt(keybinding),
-		winCtrl = BinaryKeybindings.hasWinCtrl(keybinding),
-		keyCode = BinaryKeybindings.extractKeyCode(keybinding);
+function _simpleAsString(keybinding: SimpleKeybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform): string {
+	let result: string[] = [];
+	let ctrlCmd = keybinding.hasCtrlCmd();
+	let shift = keybinding.hasShift();
+	let alt = keybinding.hasAlt();
+	let winCtrl = keybinding.hasWinCtrl();
+	let keyCode = keybinding.getKeyCode();
 
 	let keyLabel = labelProvider.getLabelForKey(keyCode);
 	if (!keyLabel) {
@@ -366,13 +366,17 @@ function _asString(keybinding: number, labelProvider: IKeyBindingLabelProvider,
 	// the actual key
 	result.push(keyLabel);
 
-	var actualResult = result.join(labelProvider.modifierSeparator);
+	return result.join(labelProvider.modifierSeparator);
+}
 
-	if (BinaryKeybindings.hasChord(keybinding)) {
-		return actualResult + ' ' + _asString(BinaryKeybindings.extractChordPart(keybinding), labelProvider, Platform);
+function _asString(keybinding: Keybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform): string {
+	if (keybinding.isChord()) {
+		let firstPart = _simpleAsString(keybinding.extractFirstPart(), labelProvider, Platform);
+		let secondPart = _simpleAsString(keybinding.extractChordPart(), labelProvider, Platform);
+		return firstPart + ' ' + secondPart;
+	} else {
+		return _simpleAsString(keybinding, labelProvider, Platform);
 	}
-
-	return actualResult;
 }
 
 function _pushKey(result: IHTMLContentElement[], str: string): void {
@@ -389,13 +393,13 @@ function _pushKey(result: IHTMLContentElement[], str: string): void {
 	});
 }
 
-function _asHTML(keybinding: number, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform, isChord: boolean = false): IHTMLContentElement[] {
-	let result: IHTMLContentElement[] = [],
-		ctrlCmd = BinaryKeybindings.hasCtrlCmd(keybinding),
-		shift = BinaryKeybindings.hasShift(keybinding),
-		alt = BinaryKeybindings.hasAlt(keybinding),
-		winCtrl = BinaryKeybindings.hasWinCtrl(keybinding),
-		keyCode = BinaryKeybindings.extractKeyCode(keybinding);
+function _simpleAsHTML(keybinding: SimpleKeybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform, isChord: boolean = false): IHTMLContentElement[] {
+	let result: IHTMLContentElement[] = [];
+	let ctrlCmd = keybinding.hasCtrlCmd();
+	let shift = keybinding.hasShift();
+	let alt = keybinding.hasAlt();
+	let winCtrl = keybinding.hasWinCtrl();
+	let keyCode = keybinding.getKeyCode();
 
 	let keyLabel = labelProvider.getLabelForKey(keyCode);
 	if (!keyLabel) {
@@ -427,19 +431,20 @@ function _asHTML(keybinding: number, labelProvider: IKeyBindingLabelProvider, Pl
 	// the actual key
 	_pushKey(result, keyLabel);
 
-	let chordTo: IHTMLContentElement[] = null;
+	return result;
+}
 
-	if (BinaryKeybindings.hasChord(keybinding)) {
-		chordTo = _asHTML(BinaryKeybindings.extractChordPart(keybinding), labelProvider, Platform, true);
+function _asHTML(keybinding: Keybinding, labelProvider: IKeyBindingLabelProvider, Platform: ISimplifiedPlatform): IHTMLContentElement[] {
+	let result: IHTMLContentElement[] = [];
+	if (keybinding.isChord()) {
+		result = result.concat(_simpleAsHTML(keybinding.extractFirstPart(), labelProvider, Platform));
 		result.push({
 			tagName: 'span',
 			text: ' '
 		});
-		result = result.concat(chordTo);
-	}
-
-	if (isChord) {
-		return result;
+		result = result.concat(_simpleAsHTML(keybinding.extractChordPart(), labelProvider, Platform));
+	} else {
+		result = result.concat(_simpleAsHTML(keybinding, labelProvider, Platform));
 	}
 
 	return [{

src/vs/base/test/common/keyCodes.test.ts

@@ -5,41 +5,48 @@
 'use strict';
 
 import * as assert from 'assert';
-import { KeyCode, KeyMod, KeyChord, BinaryKeybindings } from 'vs/base/common/keyCodes';
+import { KeyCode, KeyMod, KeyChord, createKeybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
 
 interface ITestKeybinding {
-	ctrlCmd?: boolean;
-	shift?: boolean;
-	alt?: boolean;
-	winCtrl?: boolean;
+	ctrlCmd: boolean;
+	shift: boolean;
+	alt: boolean;
+	winCtrl: boolean;
 	key: KeyCode;
 	chord?: ITestKeybinding;
 }
 
+function decodeSimpleKeybinding(kb: SimpleKeybinding): ITestKeybinding {
+	return {
+		ctrlCmd: kb.hasCtrlCmd(),
+		shift: kb.hasShift(),
+		alt: kb.hasAlt(),
+		winCtrl: kb.hasWinCtrl(),
+		key: kb.getKeyCode()
+	};
+}
+
+function decodeBinaryKeybinding(k: number): ITestKeybinding {
+	let kb = createKeybinding(k);
+	if (kb.isChord()) {
+		let result = decodeSimpleKeybinding(kb.extractFirstPart());
+		result.chord = decodeSimpleKeybinding(kb.extractChordPart());
+		return result;
+	}
+	return decodeSimpleKeybinding(kb);
+}
+
 suite('keyCodes', () => {
 	test('binary encoding', () => {
 		function test(keybinding: ITestKeybinding, k: number): void {
-			keybinding = keybinding || { key: KeyCode.Unknown };
-			assert.equal(BinaryKeybindings.hasCtrlCmd(k), !!keybinding.ctrlCmd);
-			assert.equal(BinaryKeybindings.hasShift(k), !!keybinding.shift);
-			assert.equal(BinaryKeybindings.hasAlt(k), !!keybinding.alt);
-			assert.equal(BinaryKeybindings.hasWinCtrl(k), !!keybinding.winCtrl);
-			assert.equal(BinaryKeybindings.extractKeyCode(k), keybinding.key);
+			keybinding = keybinding || { ctrlCmd: false, shift: false, alt: false, winCtrl: false, key: KeyCode.Unknown };
 
-			let chord = BinaryKeybindings.extractChordPart(k);
-			assert.equal(BinaryKeybindings.hasChord(k), !!keybinding.chord);
-			if (keybinding.chord) {
-				assert.equal(BinaryKeybindings.hasCtrlCmd(chord), !!keybinding.chord.ctrlCmd);
-				assert.equal(BinaryKeybindings.hasShift(chord), !!keybinding.chord.shift);
-				assert.equal(BinaryKeybindings.hasAlt(chord), !!keybinding.chord.alt);
-				assert.equal(BinaryKeybindings.hasWinCtrl(chord), !!keybinding.chord.winCtrl);
-				assert.equal(BinaryKeybindings.extractKeyCode(chord), keybinding.chord.key);
-			}
+			assert.deepEqual(decodeBinaryKeybinding(k), keybinding);
 		}
 
 		test(null, 0);
-		test({ key: KeyCode.Enter }, KeyCode.Enter);
-		test({ key: KeyCode.Enter, chord: { key: KeyCode.Tab } }, KeyChord(KeyCode.Enter, KeyCode.Tab));
+		test({ ctrlCmd: false, shift: false, alt: false, winCtrl: false, key: KeyCode.Enter }, KeyCode.Enter);
+		test({ ctrlCmd: false, shift: false, alt: false, winCtrl: false, key: KeyCode.Enter, chord: { ctrlCmd: false, shift: false, alt: false, winCtrl: false, key: KeyCode.Tab } }, KeyChord(KeyCode.Enter, KeyCode.Tab));
 		test({ ctrlCmd: false, shift: false, alt: false, winCtrl: false, key: KeyCode.Enter }, KeyCode.Enter);
 		test({ ctrlCmd: false, shift: false, alt: false, winCtrl: true, key: KeyCode.Enter }, KeyMod.WinCtrl | KeyCode.Enter);
 		test({ ctrlCmd: false, shift: false, alt: true, winCtrl: false, key: KeyCode.Enter }, KeyMod.Alt | KeyCode.Enter);
@@ -58,11 +65,14 @@ suite('keyCodes', () => {
 		test({ ctrlCmd: true, shift: true, alt: true, winCtrl: true, key: KeyCode.Enter }, KeyMod.CtrlCmd | KeyMod.Shift | KeyMod.Alt | KeyMod.WinCtrl | KeyCode.Enter);
 
 		let encoded = KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_Y, KeyCode.KEY_Z);
-		let encodedFirstPart = BinaryKeybindings.extractFirstPart(encoded);
-		let encodedSecondPart = BinaryKeybindings.extractChordPart(encoded);
+		let kb = createKeybinding(encoded);
 
-		assert.equal(BinaryKeybindings.hasChord(encoded), true, 'hasChord');
-		assert.equal(encodedFirstPart, KeyMod.CtrlCmd | KeyCode.KEY_Y, 'first part');
-		assert.equal(encodedSecondPart, encodedSecondPart, 'chord part');
+		assert.equal(kb.isChord(), true, 'isCord');
+		if (kb.isChord()) {
+			let firstPart = kb.extractFirstPart();
+			let chordPart = kb.extractChordPart();
+			assert.equal(firstPart.value, KeyMod.CtrlCmd | KeyCode.KEY_Y, 'first part');
+			assert.equal(chordPart.value, KeyCode.KEY_Z, 'chord part');
+		}
 	});
 });

src/vs/editor/contrib/defineKeybinding/browser/defineKeybinding.ts

@@ -361,7 +361,7 @@ class DefineKeybindingWidget implements IOverlayWidget {
 			switch (kb.value) {
 				case KeyCode.Enter:
 					if (this._lastKeybinding) {
-						this._onAccepted(KeybindingLabels.toUserSettingsLabel(this._lastKeybinding.value));
+						this._onAccepted(KeybindingLabels.toUserSettingsLabel(this._lastKeybinding));
 					}
 					this._stop();
 					return;
@@ -373,7 +373,7 @@ class DefineKeybindingWidget implements IOverlayWidget {
 
 			this._lastKeybinding = kb;
 
-			this._inputNode.value = KeybindingLabels.toUserSettingsLabel(this._lastKeybinding.value).toLowerCase();
+			this._inputNode.value = KeybindingLabels.toUserSettingsLabel(this._lastKeybinding).toLowerCase();
 			this._inputNode.title = 'keyCode: ' + keyEvent.browserEvent.keyCode;
 
 			dom.clearNode(this._outputNode);

src/vs/editor/test/common/standalone/standaloneBase.test.ts

@@ -6,7 +6,7 @@
 
 import * as assert from 'assert';
 import { KeyCode as StandaloneKeyCode, Severity as StandaloneSeverity } from 'vs/editor/common/standalone/standaloneBase';
-import { KeyCode as RuntimeKeyCode } from 'vs/base/common/keyCodes';
+import { createKeybinding, KeyCode as RuntimeKeyCode } from 'vs/base/common/keyCodes';
 import { KeybindingLabels } from 'vs/base/common/keybinding';
 import RuntimeSeverity from 'vs/base/common/severity';
 
@@ -157,7 +157,7 @@ suite('KeyCode', () => {
 			if (ignore[keyCode]) {
 				continue;
 			}
-			let userSettings = KeybindingLabels.toUserSettingsLabel(keyCode);
+			let userSettings = KeybindingLabels.toUserSettingsLabel(createKeybinding(keyCode));
 			testIsGood(userSettings, keyCode + ' - ' + StandaloneKeyCode[keyCode] + ' - ' + userSettings);
 		}
 

src/vs/platform/keybinding/common/keybindingResolver.ts

@@ -196,7 +196,7 @@ export class KeybindingResolver {
 			if (KeybindingResolver.whenIsEntirelyIncluded(true, conflict.when, item.when)) {
 				// `item` completely overwrites `conflict`
 				if (this._shouldWarnOnConflict && item.isDefault) {
-					console.warn('Conflict detected, command `' + conflict.commandId + '` cannot be triggered by ' + KeybindingLabels.toUserSettingsLabel(keypress) + ' due to ' + item.command);
+					console.warn('Conflict detected, command `' + conflict.commandId + '` cannot be triggered by ' + KeybindingLabels.toUserSettingsLabel(conflict.keybinding) + ' due to ' + item.command);
 				}
 				KeybindingResolver._push(this._lookupMapUnreachable, conflict.commandId, conflict.keybinding.value);
 			}
@@ -476,7 +476,7 @@ export class IOSupport {
 	}
 
 	public static writeKeybinding(input: number, Platform: ISimplifiedPlatform = platform): string {
-		return KeybindingLabels.toUserSettingsLabel(input, Platform);
+		return KeybindingLabels.toUserSettingsLabel(createKeybinding(input), Platform);
 	}
 
 	public static readKeybinding(input: string, Platform: ISimplifiedPlatform = platform): number {

src/vs/platform/keybinding/common/keybindingsRegistry.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import { BinaryKeybindings, KeyCodeUtils } from 'vs/base/common/keyCodes';
+import { createKeybinding, KeyCodeUtils } from 'vs/base/common/keyCodes';
 import * as platform from 'vs/base/common/platform';
 import { IKeybindingItem, IKeybindings } from 'vs/platform/keybinding/common/keybinding';
 import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
@@ -104,8 +104,10 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry {
 
 	private registerDefaultKeybinding(keybinding: number, commandId: string, weight1: number, weight2: number, when: ContextKeyExpr): void {
 		if (platform.isWindows) {
-			if (BinaryKeybindings.hasCtrlCmd(keybinding) && !BinaryKeybindings.hasShift(keybinding) && BinaryKeybindings.hasAlt(keybinding) && !BinaryKeybindings.hasWinCtrl(keybinding)) {
-				if (/^[A-Z0-9\[\]\|\;\'\,\.\/\`]$/.test(KeyCodeUtils.toString(BinaryKeybindings.extractKeyCode(keybinding)))) {
+			let kb = createKeybinding(keybinding);
+			let simpleKb = kb.isChord() ? kb.extractFirstPart() : kb;
+			if (simpleKb.hasCtrlCmd() && !simpleKb.hasShift() && simpleKb.hasAlt() && !simpleKb.hasWinCtrl()) {
+				if (/^[A-Z0-9\[\]\|\;\'\,\.\/\`]$/.test(KeyCodeUtils.toString(simpleKb.getKeyCode()))) {
 					console.warn('Ctrl+Alt+ keybindings should not be used by default under Windows. Offender: ', keybinding, ' for ', commandId);
 				}
 			}