Skip to content

Commit

Permalink
Modified unit tests to support multiple chords
Browse files Browse the repository at this point in the history
The major change with the unit tests was no longer requiring
a secondary null field for dispatchparts. As chords can be any length,
it's more forward-thinking to modify the unit tests to accept new behavior.
  • Loading branch information
toumorokoshi committed Jan 17, 2019
1 parent 0c4b29e commit 50670b9
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 92 deletions.
3 changes: 3 additions & 0 deletions src/vs/platform/keybinding/common/resolvedKeybindingItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export class ResolvedKeybindingItem {
if (resolvedKeybinding) {
let [keypressFirstPart, keypressChordPart] = resolvedKeybinding.getDispatchParts();
this.keypressFirstPart = keypressFirstPart;
if (keypressChordPart === undefined) {
keypressChordPart = null;
}
this.keypressChordPart = keypressChordPart;
} else {
this.keypressFirstPart = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export class USLayoutResolvedKeybinding extends ResolvedKeybinding {
}

public getParts(): ResolvedKeybindingPart[] {
return this._parts.map(this._toResolvedKeybindingPart);
return this._parts.map(this._toResolvedKeybindingPart, this);
}

private _toResolvedKeybindingPart(keybinding: SimpleKeybinding): ResolvedKeybindingPart {
Expand Down
21 changes: 16 additions & 5 deletions src/vs/platform/keybinding/test/common/keybindingResolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,23 @@ suite('KeybindingResolver', () => {
let testResolve = (ctx: IContext, _expectedKey: number, commandId: string) => {
const expectedKey = createKeybinding(_expectedKey, OS)!;

for (let i = 0; i < expectedKey.parts.length; i++) {
let previousPart = null;
for (let i = 0, len = expectedKey.parts.length; i < len; i++) {
let part = getDispatchStr(expectedKey.parts[i]);
let result = resolver.resolve(ctx, null, part);
assert.ok(result !== null, `Enters chord for {commandId} at index {i}`);
assert.equal(result.commandId, null, `Enters chord for {commandId} at index {i}`);
assert.equal(result.enterChord, true, `Enters chord for {commandId} at index {i}`);
let result = resolver.resolve(ctx, previousPart, part);
assert.ok(result !== null, `Enters chord for ${commandId} at part ${i}`);
if (i === len - 1) {
// if it's the final part, then we should find a valid command,
// and there should not be a chord.
assert.equal(result.commandId, commandId, `Enters chord for ${commandId} at part ${i}`);
assert.equal(result.enterChord, false, `Enters chord for ${commandId} at part ${i}`);
} else {
// if it's not the final part, then we should not find a valid command,
// and there should be a chord.
assert.equal(result.commandId, null, `Enters chord for ${commandId} at part ${i}`);
assert.equal(result.enterChord, true, `Enters chord for ${commandId} at part ${i}`);
}
previousPart = part;
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,15 @@ export class NativeResolvedKeybinding extends ResolvedKeybinding {
}

public isWYSIWYG(): boolean {
return this._parts.every(this._isWYSIWYG);
return this._parts.every(this._isWYSIWYG, this);
}

public isChord(): boolean {
return this._parts.length > 1;
}

public getParts(): ResolvedKeybindingPart[] {
return this._parts.map(this._toResolvedKeybindingPart);
return this._parts.map(this._toResolvedKeybindingPart, this);
}

private _toResolvedKeybindingPart(binding: ScanCodeBinding): ResolvedKeybindingPart {
Expand All @@ -171,7 +171,11 @@ export class NativeResolvedKeybinding extends ResolvedKeybinding {
}

public getDispatchParts(): (string | null)[] {
return this._parts.map(this._mapper.getDispatchStrForScanCodeBinding);
let dispatchParts = [];
for (let part of this._parts) {
dispatchParts.push(part ? this._mapper.getDispatchStrForScanCodeBinding(part) : null);
}
return dispatchParts;
}
}

Expand Down Expand Up @@ -1026,7 +1030,10 @@ export class MacLinuxKeyboardMapper implements IKeyboardMapper {
}

public resolveKeybinding(keybinding: Keybinding): NativeResolvedKeybinding[] {
let chordParts = keybinding.parts.map(this.simpleKeybindingToScanCodeBinding);
let chordParts: ScanCodeBinding[][] = [];
for (let part of keybinding.parts) {
chordParts.push(this.simpleKeybindingToScanCodeBinding(part));
}
let result: NativeResolvedKeybinding[] = [];
this._resolveKeybindingPart(chordParts, 0, [], result);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface IResolvedKeybinding {
userSettingsLabel: string | null;
isWYSIWYG: boolean;
isChord: boolean;
dispatchParts: string[];
dispatchParts: Array<string | null>;
}

function toIResolvedKeybinding(kb: ResolvedKeybinding): IResolvedKeybinding {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ suite('keyboardMapper - MAC fallback', () => {
userSettingsLabel: 'cmd+z',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['meta+Z', null],
dispatchParts: ['meta+Z'],
}]
);
});
Expand Down Expand Up @@ -65,7 +65,7 @@ suite('keyboardMapper - MAC fallback', () => {
userSettingsLabel: 'cmd+z',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['meta+Z', null],
dispatchParts: ['meta+Z'],
}
);
});
Expand Down Expand Up @@ -105,7 +105,7 @@ suite('keyboardMapper - MAC fallback', () => {
userSettingsLabel: 'cmd+',
isWYSIWYG: true,
isChord: false,
dispatchParts: [null, null],
dispatchParts: [null],
}
);
});
Expand All @@ -129,7 +129,7 @@ suite('keyboardMapper - LINUX fallback', () => {
userSettingsLabel: 'ctrl+z',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['ctrl+Z', null],
dispatchParts: ['ctrl+Z'],
}]
);
});
Expand Down Expand Up @@ -167,7 +167,7 @@ suite('keyboardMapper - LINUX fallback', () => {
userSettingsLabel: 'ctrl+z',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['ctrl+Z', null],
dispatchParts: ['ctrl+Z'],
}
);
});
Expand Down Expand Up @@ -201,7 +201,7 @@ suite('keyboardMapper - LINUX fallback', () => {
userSettingsLabel: 'ctrl+,',
isWYSIWYG: true,
isChord: false,
dispatchParts: ['ctrl+,', null],
dispatchParts: ['ctrl+,'],
}]
);
});
Expand All @@ -224,7 +224,7 @@ suite('keyboardMapper - LINUX fallback', () => {
userSettingsLabel: 'ctrl+',
isWYSIWYG: true,
isChord: false,
dispatchParts: [null, null],
dispatchParts: [null],
}
);
});
Expand Down
Loading

0 comments on commit 50670b9

Please sign in to comment.