Skip to content

Commit

Permalink
Changed Modifier to KeyModifier and each enum member to its key
Browse files Browse the repository at this point in the history 
Fixes #458

Signed-off-by: WKnight02 <wknight02@gmail.com>
  • Loading branch information
@paul-marechal @akosyakov
paul-marechal authored and akosyakov committed Feb 23, 2018
1 parent da62d1c commit eb8f3a6
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 65 deletions.
4 changes: 2 additions & 2 deletions doc/Commands_Keybindings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ With the `id`, the only other mandatory parameter is the `keycode`, which is bas

**keys.ts**
```typescript
export declare type Keystroke = { first: Key, modifiers?: Modifier[] };
export declare type Keystroke = { first: Key, modifiers?: KeyModifier[] };
```
Modifiers are platform independent, so `Modifier.M1` is Command on OS X and CTRL on Windows/Linux. Key string constants are defined in `keys.ts`
Modifiers are platform independent, so `KeyModifier.CtrlCmd` is Command on OS X and CTRL on Windows/Linux. Key string constants are defined in `keys.ts`

### Binding the contribution to KeybindingContribution

Expand Down
52 changes: 30 additions & 22 deletions packages/core/src/browser/keybinding.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
KeybindingRegistry, KeybindingContext, KeybindingContextRegistry,
Keybinding, KeybindingContribution, KeybindingScope
} from './keybinding';
import { KeyCode, Key, Modifier, KeySequence, EasyKey } from './keys';
import { KeyCode, Key, KeyModifier, KeySequence, EasyKey } from './keys';
import { CommandRegistry, CommandService, CommandContribution, Command } from '../common/command';
import { LabelParser } from './label-parser';
import { MockLogger } from '../common/test/mock-logger';
Expand Down Expand Up @@ -198,17 +198,17 @@ describe('keybindings', () => {
keybinding: "ctrl+c"
}];

const validKeyCode = KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [Modifier.M1] });
const validKeyCode = KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] });

keybindingRegistry.setKeymap(KeybindingScope.WORKSPACE, keybindingsSpecific);

let bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M1] })]).full;
let bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.CtrlCmd] })]).full;
expect(bindings).to.be.empty;

bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_B, modifiers: [Modifier.M1] })]).full;
bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_B, modifiers: [KeyModifier.CtrlCmd] })]).full;
expect(bindings).to.be.empty;

bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [Modifier.M1] })]).full;
bindings = keybindingRegistry.getKeybindingsForKeySequence([KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] })]).full;
const keyCode = KeyCode.parse(bindings[0].keybinding);
expect(keyCode.key).to.be.equal(validKeyCode.key);
});
Expand All @@ -222,7 +222,7 @@ describe('keybindings', () => {
keybindingRegistry.setKeymap(KeybindingScope.USER, keybindingsUser);

const validKeyCodes = [];
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [Modifier.M1] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_T }));

const bindings = keybindingRegistry.getKeybindingsForKeySequence(KeySequence.parse("ctrlcmd+x"));
Expand Down Expand Up @@ -317,22 +317,22 @@ describe("keys api", () => {

it("it should serialize a keycode properly with BACKQUOTE + M1", () => {
stub = sinon.stub(os, 'isOSX').value(true);
let keyCode = KeyCode.createKeyCode({ first: Key.BACKQUOTE, modifiers: [Modifier.M1] });
let keyCode = KeyCode.createKeyCode({ first: Key.BACKQUOTE, modifiers: [KeyModifier.CtrlCmd] });
let keyCodeString = keyCode.toString();
expect(keyCodeString).to.be.equal("meta+`");
let parsedKeyCode = KeyCode.parse(keyCodeString);
expect(KeyCode.equals(parsedKeyCode, keyCode)).to.be.true;

stub = sinon.stub(os, 'isOSX').value(false);
keyCode = KeyCode.createKeyCode({ first: Key.BACKQUOTE, modifiers: [Modifier.M1] });
keyCode = KeyCode.createKeyCode({ first: Key.BACKQUOTE, modifiers: [KeyModifier.CtrlCmd] });
keyCodeString = keyCode.toString();
expect(keyCodeString).to.be.equal("ctrl+`");
parsedKeyCode = KeyCode.parse(keyCodeString);
expect(KeyCode.equals(parsedKeyCode, keyCode)).to.be.true;
});

it("it should serialize a keycode properly with a + M2 + M3", () => {
const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M2, Modifier.M3] });
const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift, KeyModifier.Alt] });
const keyCodeString = keyCode.toString();
expect(keyCodeString).to.be.equal("shift+alt+a");
const parsedKeyCode = KeyCode.parse(keyCodeString);
Expand All @@ -344,11 +344,19 @@ describe("keys api", () => {
const right = KeySequence.parse("alt+shift+a");
expect(KeySequence.compare(left, right)).to.be.equal(KeySequence.CompareResult.FULL);

expect(KeySequence.compare([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M3, Modifier.M2] })], right)).to.be.equal(KeySequence.CompareResult.FULL);
expect(KeySequence.compare(left, [KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M3, Modifier.M2] })])).to.be.equal(KeySequence.CompareResult.FULL);

expect(KeySequence.compare([KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M2, Modifier.M3] })], right)).to.be.equal(KeySequence.CompareResult.FULL);
expect(KeySequence.compare(left, [KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M2, Modifier.M3] })])).to.be.equal(KeySequence.CompareResult.FULL);
expect(KeySequence.compare(
[KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Alt, KeyModifier.Shift] })], right)).to.be.equal(
KeySequence.CompareResult.FULL);
expect(KeySequence.compare(
left, [KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Alt, KeyModifier.Shift] })])).to.be.equal(
KeySequence.CompareResult.FULL);

expect(KeySequence.compare(
[KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift, KeyModifier.Alt] })], right)).to.be.equal(
KeySequence.CompareResult.FULL);
expect(KeySequence.compare(
left, [KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.Shift, KeyModifier.Alt] })])).to.be.equal(
KeySequence.CompareResult.FULL);
});

it("it should parse ctrl key properly on both OS X and other platforms", () => {
Expand All @@ -365,7 +373,7 @@ describe("keys api", () => {

it("it should serialize a keycode properly with a + M4", () => {
stub = sinon.stub(os, 'isOSX').value(true);
const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M4] });
const keyCode = KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.MacCtrl] });
const keyCodeString = keyCode.toString();
expect(keyCodeString).to.be.equal("ctrl+a");
const parsedKeyCode = KeyCode.parse(keyCodeString);
Expand All @@ -374,16 +382,16 @@ describe("keys api", () => {

it("it should parse a multi keycode keybinding", () => {
const validKeyCodes = [];
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M1] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [Modifier.M1, Modifier.M2] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.CtrlCmd] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C, modifiers: [KeyModifier.CtrlCmd, KeyModifier.Shift] }));

const parsedKeyCodes = KeySequence.parse("ctrlcmd+a ctrlcmd+shift+c");
expect(parsedKeyCodes).to.deep.equal(validKeyCodes);
});

it("it should parse a multi keycode keybinding with no modifiers", () => {
const validKeyCodes = [];
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [Modifier.M1] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_A, modifiers: [KeyModifier.CtrlCmd] }));
validKeyCodes.push(KeyCode.createKeyCode({ first: Key.KEY_C }));

const parsedKeyCodes = KeySequence.parse("ctrlcmd+a c");
Expand Down Expand Up @@ -420,15 +428,15 @@ describe("keys api", () => {

it("it should be a modifier only", () => {

const keyCode = KeyCode.createKeyCode({ modifiers: [Modifier.M1] });
expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [Modifier.M1] }));
const keyCode = KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd] });
expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd] }));
expect(keyCode.isModifierOnly()).to.be.true;
});

it("it should be multiple modifiers only", () => {

const keyCode = KeyCode.createKeyCode({ modifiers: [Modifier.M1, Modifier.M3] });
expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [Modifier.M1, Modifier.M3] }));
const keyCode = KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd, KeyModifier.Alt] });
expect(keyCode).to.be.deep.equal(KeyCode.createKeyCode({ modifiers: [KeyModifier.CtrlCmd, KeyModifier.Alt] }));
expect(keyCode.isModifierOnly()).to.be.true;
});
});
Expand Down
66 changes: 33 additions & 33 deletions packages/core/src/browser/keys.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { isOSX } from '../common/os';
* Since `M2+M3+<Key>` (Alt+Shift+<Key>) is reserved on MacOS X for writing special characters, such bindings are commonly
* undefined for platform MacOS X and redefined as `M1+M3+<Key>`. The rule applies on the `M3+M2+<Key>` sequence.
*/
export declare type Keystroke = { first?: Key, modifiers?: Modifier[] };
export declare type Keystroke = { first?: Key, modifiers?: KeyModifier[] };

export type KeySequence = KeyCode[];
export namespace KeySequence {
Expand Down Expand Up @@ -152,24 +152,24 @@ export class KeyCode {
}

if (isOSX) {
this.meta = parts.some(part => part === Modifier.M1);
this.shift = parts.some(part => part === Modifier.M2);
this.alt = parts.some(part => part === Modifier.M3);
this.ctrl = parts.some(part => part === Modifier.M4);
this.meta = parts.some(part => part === KeyModifier.CtrlCmd);
this.shift = parts.some(part => part === KeyModifier.Shift);
this.alt = parts.some(part => part === KeyModifier.Alt);
this.ctrl = parts.some(part => part === KeyModifier.MacCtrl);
} else {
this.meta = false;
this.ctrl = parts.some(part => part === Modifier.M1);
this.shift = parts.some(part => part === Modifier.M2);
this.alt = parts.some(part => part === Modifier.M3);
this.ctrl = parts.some(part => part === KeyModifier.CtrlCmd);
this.shift = parts.some(part => part === KeyModifier.Shift);
this.alt = parts.some(part => part === KeyModifier.Alt);
}
}

/* Return true of string is a modifier M1 to M4 */
public static isModifierString(key: string) {
if (key === Modifier.M1
|| key === Modifier.M2
|| key === Modifier.M3
|| key === Modifier.M4) {
if (key === KeyModifier.CtrlCmd
|| key === KeyModifier.Shift
|| key === KeyModifier.Alt
|| key === KeyModifier.MacCtrl) {
return true;
}
return false;
Expand Down Expand Up @@ -212,26 +212,26 @@ export class KeyCode {
/* meta only works on macOS */
if (keyString === SpecialCases.META) {
if (isOSX) {
sequence.push(`${Modifier.M1}`);
sequence.push(`${KeyModifier.CtrlCmd}`);
} else {
throw new Error(`Can't parse keybinding ${keybinding} meta is for OSX only`);
}
/* ctrlcmd for M1 keybindings that work on both macOS and other platforms */
} else if (keyString === SpecialCases.CTRLCMD) {
sequence.push(`${Modifier.M1}`);
sequence.push(`${KeyModifier.CtrlCmd}`);
} else if (Key.isKey(key)) {
if (Key.isModifier(key.code)) {
if (key.keyCode === EasyKey.CONTROL.keyCode) {
// CTRL on MacOS X (M4)
if (isOSX) {
sequence.push(`${Modifier.M4}`);
sequence.push(`${KeyModifier.MacCtrl}`);
} else {
sequence.push(`${Modifier.M1}`);
sequence.push(`${KeyModifier.CtrlCmd}`);
}
} else if (key.keyCode === EasyKey.SHIFT.keyCode) {
sequence.push(`${Modifier.M2}`);
sequence.push(`${KeyModifier.Shift}`);
} else if (key.keyCode === EasyKey.ALT.keyCode) {
sequence.push(`${Modifier.M3}`);
sequence.push(`${KeyModifier.Alt}`);
}
} else {
sequence.unshift(key.code);
Expand All @@ -243,7 +243,7 @@ export class KeyCode {

// We need to sort the modifier keys, but on the modifiers, so it always keeps the M1 less than M2, M2 less than M3 and so on order.
// We intentionally ignore other cases.
sequence.sort((left: string, right: string) => Modifier.isModifier(left) && Modifier.isModifier(right) ? left.localeCompare(right) : 0);
sequence.sort((left: string, right: string) => KeyModifier.isModifier(left) && KeyModifier.isModifier(right) ? left.localeCompare(right) : 0);
KeyCode.keybindings[keybinding] = new KeyCode(sequence.join('+'));
return KeyCode.keybindings[keybinding];
}
Expand All @@ -259,22 +259,22 @@ export class KeyCode {

// CTRL + COMMAND (M1)
if ((isOSX && event.metaKey) || (!isOSX && event.ctrlKey)) {
sequence.push(`${Modifier.M1}`);
sequence.push(`${KeyModifier.CtrlCmd}`);
}

// SHIFT (M2)
if (event.shiftKey) {
sequence.push(`${Modifier.M2}`);
sequence.push(`${KeyModifier.Shift}`);
}

// ALT (M3)
if (event.altKey) {
sequence.push(`${Modifier.M3}`);
sequence.push(`${KeyModifier.Alt}`);
}

// CTRL on MacOS X (M4)
if (isOSX && !event.metaKey && event.ctrlKey) {
sequence.push(`${Modifier.M4}`);
sequence.push(`${KeyModifier.MacCtrl}`);
}

return new KeyCode(sequence.join('+'));
Expand Down Expand Up @@ -365,35 +365,35 @@ export class KeyCode {
}
}

export enum Modifier {
export enum KeyModifier {
/**
* M1 is the COMMAND key on MacOS X, and the CTRL key on most other platforms.
*/
M1 = "M1",
CtrlCmd = "M1",
/**
* M2 is the SHIFT key.
*/
M2 = "M2",
Shift = "M2",
/**
* M3 is the Option key on MacOS X, and the ALT key on most other platforms.
*/
M3 = "M3",
Alt = "M3",
/**
* M4 is the CTRL key on MacOS X, and is undefined on other platforms.
*/
M4 = "M4"
MacCtrl = "M4"
}

export namespace Modifier {
export namespace KeyModifier {
/**
* The CTRL key, independently of the platform.
* _Note:_ In general `Modifier.M1` should be preferred over this constant.
* _Note:_ In general `KeyModifier.CtrlCmd` should be preferred over this constant.
*/
export const CTRL = isOSX ? Modifier.M4 : Modifier.M1;
export const CTRL = isOSX ? KeyModifier.MacCtrl : KeyModifier.CtrlCmd;
/**
* An alias for the SHIFT key (`Modifier.M2`).
* An alias for the SHIFT key (`KeyModifier.Shift`).
*/
export const SHIFT = Modifier.M2;
export const SHIFT = KeyModifier.Shift;

/**
* `true` if the argument represents a modifier. Otherwise, `false`.
Expand Down
10 changes: 5 additions & 5 deletions packages/monaco/src/browser/monaco-keybinding.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { injectable, inject } from 'inversify';
import { KeybindingContribution, KeybindingRegistry, Key, KeyCode, Keystroke, Modifier } from '@theia/core/lib/browser';
import { KeybindingContribution, KeybindingRegistry, Key, KeyCode, Keystroke, KeyModifier } from '@theia/core/lib/browser';
import { MonacoCommands } from './monaco-command';
import { MonacoCommandRegistry } from './monaco-command-registry';
import { KEY_CODE_MAP } from './monaco-keycode-map';
Expand Down Expand Up @@ -62,16 +62,16 @@ export class MonacoKeybindingContribution implements KeybindingContribution {
modifiers: []
};
if (keybinding.ctrlKey) {
sequence.modifiers!.push(Modifier.M1);
sequence.modifiers!.push(KeyModifier.CtrlCmd);
}
if (keybinding.shiftKey) {
sequence.modifiers!.push(Modifier.M2);
sequence.modifiers!.push(KeyModifier.Shift);
}
if (keybinding.altKey) {
sequence.modifiers!.push(Modifier.M3);
sequence.modifiers!.push(KeyModifier.Alt);
}
if (keybinding.metaKey) {
sequence.modifiers!.push(Modifier.M4);
sequence.modifiers!.push(KeyModifier.MacCtrl);
}
return KeyCode.createKeyCode(sequence);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/terminal/src/browser/terminal-frontend-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '@theia/core/lib/common';
import {
CommonMenus, ApplicationShell, KeybindingContribution, KeyCode, Key,
Modifier, KeybindingRegistry, Keybinding, KeybindingContextRegistry,
KeyModifier, KeybindingRegistry, Keybinding, KeybindingContextRegistry,
} from '@theia/core/lib/browser';
import { TERMINAL_WIDGET_FACTORY_ID, TerminalWidgetFactoryOptions, TerminalWidget } from './terminal-widget';
import { WidgetManager } from '@theia/core/lib/browser/widget-manager';
Expand Down Expand Up @@ -82,7 +82,7 @@ export class TerminalFrontendContribution implements CommandContribution, MenuCo
const regCtrl = (k: Key) => {
keybindings.registerKeybindings({
command: KeybindingRegistry.PASSTHROUGH_PSEUDO_COMMAND,
keybinding: KeyCode.createKeyCode({ first: k, modifiers: [Modifier.CTRL] }).toString(),
keybinding: KeyCode.createKeyCode({ first: k, modifiers: [KeyModifier.CTRL] }).toString(),
context: TERMINAL_ACTIVE_CONTEXT,
});
};
Expand All @@ -92,7 +92,7 @@ export class TerminalFrontendContribution implements CommandContribution, MenuCo
const regAlt = (k: Key) => {
keybindings.registerKeybinding({
command: KeybindingRegistry.PASSTHROUGH_PSEUDO_COMMAND,
keybinding: KeyCode.createKeyCode({ first: k, modifiers: [Modifier.M3] }).toString(),
keybinding: KeyCode.createKeyCode({ first: k, modifiers: [KeyModifier.Alt] }).toString(),
context: TERMINAL_ACTIVE_CONTEXT,
});
};
Expand Down

0 comments on commit eb8f3a6

Please sign in to comment.