Skip to content

Commit

Permalink
Add support for three or more chord keyboard shortcuts (#175253)
Browse files Browse the repository at this point in the history 
* Add support for three or more chords in shortcuts

* Add tests and fix bug that was uncovered

* Clean up TODO comments

* Make multi chords more explicit in quickInput.ts

* Address review comments

* keybindings: resolver: align terminology with rest of code

* Allow input of >2 chords in keybinding settings

* Keep the limit at 2 chords in the widget for now

* Revert "Keep the limit at 2 chords in the widget for now"

This reverts commit 55c32ae.

* keybindings: UI: search widget: limit chords to 2 for now

* Fix single modifier chords

* Remove unused import

---------

Co-authored-by: Ulugbek Abdullaev <ulugbekna@gmail.com>
Co-authored-by: Alex Dima <alexdima@microsoft.com>
  • Loading branch information
@dyedgreen @ulugbekna @alexdima
3 people authored Mar 20, 2023
1 parent faaaa3c commit a0a30a5
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 119 deletions.
10 changes: 5 additions & 5 deletions src/vs/base/browser/ui/keybindingLabel/keybindingLabel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ export class KeybindingLabel {
this.clear();

if (this.keybinding) {
const [firstChord, secondChord] = this.keybinding.getChords();// TODO@chords
if (firstChord) {
this.renderChord(this.domNode, firstChord, this.matches ? this.matches.firstPart : null);
const chords = this.keybinding.getChords();
if (chords[0]) {
this.renderChord(this.domNode, chords[0], this.matches ? this.matches.firstPart : null);
}
if (secondChord) {
for (let i = 1; i < chords.length; i++) {
dom.append(this.domNode, $('span.monaco-keybinding-key-chord-separator', undefined, ' '));
this.renderChord(this.domNode, secondChord, this.matches ? this.matches.chordPart : null);
this.renderChord(this.domNode, chords[i], this.matches ? this.matches.chordPart : null);
}
const title = (this.options.disableTitle ?? false) ? undefined : this.keybinding.getAriaLabel() || undefined;
if (title !== undefined) {
Expand Down
32 changes: 20 additions & 12 deletions src/vs/base/common/keybindings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,27 @@ const enum BinaryKeybindingsMask {
KeyCode = 0x000000FF
}

export function decodeKeybinding(keybinding: number, OS: OperatingSystem): Keybinding | null {
if (keybinding === 0) {
return null;
}
const firstChord = (keybinding & 0x0000FFFF) >>> 0;
const secondChord = (keybinding & 0xFFFF0000) >>> 16;
if (secondChord !== 0) {
return new Keybinding([
createSimpleKeybinding(firstChord, OS),
createSimpleKeybinding(secondChord, OS)
]);
export function decodeKeybinding(keybinding: number | number[], OS: OperatingSystem): Keybinding | null {
if (typeof keybinding === 'number') {
if (keybinding === 0) {
return null;
}
const firstChord = (keybinding & 0x0000FFFF) >>> 0;
const secondChord = (keybinding & 0xFFFF0000) >>> 16;
if (secondChord !== 0) {
return new Keybinding([
createSimpleKeybinding(firstChord, OS),
createSimpleKeybinding(secondChord, OS)
]);
}
return new Keybinding([createSimpleKeybinding(firstChord, OS)]);
} else {
const chords = [];
for (let i = 0; i < keybinding.length; i++) {
chords.push(createSimpleKeybinding(keybinding[i], OS));
}
return new Keybinding(chords);
}
return new Keybinding([createSimpleKeybinding(firstChord, OS)]);
}

export function createSimpleKeybinding(keybinding: number, OS: OperatingSystem): KeyCodeChord {
Expand Down
56 changes: 40 additions & 16 deletions src/vs/platform/keybinding/common/abstractKeybindingService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
return this._onDidUpdateKeybindings ? this._onDidUpdateKeybindings.event : Event.None; // Sinon stubbing walks properties on prototype
}

private _currentChord: CurrentChord | null;
private _currentChord: CurrentChord[] | null;
private _currentChordChecker: IntervalTimer;
private _currentChordStatusMessage: IDisposable | null;
private _ignoreSingleModifiers: KeybindingModifierSet;
Expand Down Expand Up @@ -140,17 +140,12 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
}

const contextValue = this._contextKeyService.getContext(target);
const currentChord = this._currentChord ? this._currentChord.keypress : null;
const currentChord = this._currentChord ? this._currentChord.map((({ keypress }) => keypress)) : null;
return this._getResolver().resolve(contextValue, currentChord, firstChord);
}

private _enterMultiChordMode(firstChord: string, keypressLabel: string | null): void {
this._currentChord = {
keypress: firstChord,
label: keypressLabel
};
this._currentChordStatusMessage = this._notificationService.status(nls.localize('first.chord', "({0}) was pressed. Waiting for second key of chord...", keypressLabel));
const chordEnterTime = Date.now();
private _scheduleLeaveChordMode(): void {
const chordLastInteractedTime = Date.now();
this._currentChordChecker.cancelAndSet(() => {

if (!this._documentHasFocus()) {
Expand All @@ -159,15 +154,35 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
return;
}

if (Date.now() - chordEnterTime > 5000) {
if (Date.now() - chordLastInteractedTime > 5000) {
// 5 seconds elapsed => leave chord mode
this._leaveChordMode();
}

}, 500);
}

private _enterMultiChordMode(firstChord: string, keypressLabel: string | null): void {
this._currentChord = [{
keypress: firstChord,
label: keypressLabel
}];
this._currentChordStatusMessage = this._notificationService.status(nls.localize('first.chord', "({0}) was pressed. Waiting for second key of chord...", keypressLabel));
this._scheduleLeaveChordMode();
IME.disable();
}

private _continueMultiChordMode(nextChord: string, keypressLabel: string | null): void {
this._currentChord = this._currentChord ? this._currentChord : [];
this._currentChord.push({
keypress: nextChord,
label: keypressLabel
});
const fullKeypressLabel = this._currentChord.map(({ label }) => label).join(', ');
this._currentChordStatusMessage = this._notificationService.status(nls.localize('next.chord', "({0}) was pressed. Waiting for next key of chord...", fullKeypressLabel));
this._scheduleLeaveChordMode();
}

private _leaveChordMode(): void {
if (this._currentChordStatusMessage) {
this._currentChordStatusMessage.dispose();
Expand Down Expand Up @@ -255,15 +270,18 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
}

let firstChord: string | null = null; // the first keybinding i.e. Ctrl+K
let currentChord: string | null = null;// the "second" keybinding i.e. Ctrl+K "Ctrl+D"
let currentChord: string[] | null = null;// the "second" keybinding i.e. Ctrl+K "Ctrl+D"

if (isSingleModiferChord) {
// The keybinding is the second keypress of a single modifier chord, e.g. "shift shift".
// A single modifier can only occur when the same modifier is pressed in short sequence,
// hence we disregard `_currentChord` and use the same modifier instead.
const [dispatchKeyname,] = keybinding.getSingleModifierDispatchChords();
firstChord = dispatchKeyname;
currentChord = dispatchKeyname;
currentChord = dispatchKeyname ? [dispatchKeyname] : [];
} else {
[firstChord,] = keybinding.getDispatchChords();
currentChord = this._currentChord ? this._currentChord.keypress : null;
currentChord = this._currentChord ? this._currentChord.map(({ keypress }) => keypress) : null;
}

if (firstChord === null) {
Expand All @@ -286,9 +304,15 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
}

if (this._currentChord) {
if (!resolveResult || !resolveResult.commandId) {
this._log(`+ Leaving chord mode: Nothing bound to "${this._currentChord.label} ${keypressLabel}".`);
this._notificationService.status(nls.localize('missing.chord', "The key combination ({0}, {1}) is not a command.", this._currentChord.label, keypressLabel), { hideAfter: 10 * 1000 /* 10s */ });
if (resolveResult && !resolveResult.leaveMultiChord) {
shouldPreventDefault = true;
this._continueMultiChordMode(firstChord, keypressLabel);
this._log(`+ Continuing chord mode...`);
return shouldPreventDefault;
} else if (!resolveResult || !resolveResult.commandId) {
const currentChordLabel = this._currentChord.map(({ label }) => label).join(', ');
this._log(`+ Leaving chord mode: Nothing bound to "${currentChordLabel}, ${keypressLabel}".`);
this._notificationService.status(nls.localize('missing.chord', "The key combination ({0}, {1}) is not a command.", currentChordLabel, keypressLabel), { hideAfter: 10 * 1000 /* 10s */ });
shouldPreventDefault = true;
}
}
Expand Down
67 changes: 42 additions & 25 deletions src/vs/platform/keybinding/common/keybindingResolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,17 @@ export class KeybindingResolver {
continue;
}

// TODO@chords
this._addKeyPress(k.chords[0], k);
}
}

private static _isTargetedForRemoval(defaultKb: ResolvedKeybindingItem, keypressFirstPart: string | null, keypressChordPart: string | null, when: ContextKeyExpression | undefined): boolean {
// TODO@chords
if (keypressFirstPart && defaultKb.chords[0] !== keypressFirstPart) {
return false;
}
// TODO@chords
if (keypressChordPart && defaultKb.chords[1] !== keypressChordPart) {
return false;
private static _isTargetedForRemoval(defaultKb: ResolvedKeybindingItem, keypress: string[] | null, when: ContextKeyExpression | undefined): boolean {
if (keypress) {
for (let i = 0; i < keypress.length; i++) {
if (keypress[i] !== defaultKb.chords[i]) {
return false;
}
}
}

// `true` means always, as does `undefined`
Expand Down Expand Up @@ -132,11 +130,8 @@ export class KeybindingResolver {
}
let isRemoved = false;
for (const commandRemoval of commandRemovals) {
// TODO@chords
const keypressFirstChord = commandRemoval.chords[0];
const keypressSecondChord = commandRemoval.chords[1];
const when = commandRemoval.when;
if (this._isTargetedForRemoval(rule, keypressFirstChord, keypressSecondChord, when)) {
if (this._isTargetedForRemoval(rule, commandRemoval.chords, when)) {
isRemoved = true;
break;
}
Expand Down Expand Up @@ -167,12 +162,17 @@ export class KeybindingResolver {
continue;
}

const conflictHasMultipleChords = (conflict.chords.length > 1);
const itemHasMultipleChords = (item.chords.length > 1);

// TODO@chords
if (conflictHasMultipleChords && itemHasMultipleChords && conflict.chords[1] !== item.chords[1]) {
// The conflict only shares the first chord with this command
// Test if the shorter keybinding is a prefix of the longer one.
// If the shorter keybinding is a prefix, it effectively will shadow the longer one and is considered a conflict.
let isShorterKbPrefix = true;
for (let i = 1; i < conflict.chords.length && i < item.chords.length; i++) {
if (conflict.chords[i] !== item.chords[i]) {
// The ith step does not conflict
isShorterKbPrefix = false;
break;
}
}
if (!isShorterKbPrefix) {
continue;
}

Expand Down Expand Up @@ -277,14 +277,13 @@ export class KeybindingResolver {
return items[items.length - 1];
}

public resolve(context: IContext, currentChord: string | null, keypress: string): IResolveResult | null {
public resolve(context: IContext, currentChord: string[] | null, keypress: string): IResolveResult | null {
this._log(`| Resolving ${keypress}${currentChord ? ` chorded from ${currentChord}` : ``}`);
let lookupMap: ResolvedKeybindingItem[] | null = null;

if (currentChord !== null) {
// Fetch all chord bindings for `currentChord`

const candidates = this._map.get(currentChord);
const candidates = this._map.get(currentChord[0]);
if (typeof candidates === 'undefined') {
// No chords starting with `currentChord`
this._log(`\\ No keybinding entries.`);
Expand All @@ -294,8 +293,18 @@ export class KeybindingResolver {
lookupMap = [];
for (let i = 0, len = candidates.length; i < len; i++) {
const candidate = candidates[i];
// TODO@chords
if (candidate.chords[1] === keypress) {
if (candidate.chords.length <= currentChord.length) {
continue;
}

let prefixMatches = true;
for (let i = 1; i < currentChord.length; i++) {
if (candidate.chords[i] !== currentChord[i]) {
prefixMatches = false;
break;
}
}
if (prefixMatches && candidate.chords[currentChord.length] === keypress) {
lookupMap.push(candidate);
}
}
Expand All @@ -316,7 +325,6 @@ export class KeybindingResolver {
return null;
}

// TODO@chords
if (currentChord === null && result.chords.length > 1 && result.chords[1] !== null) {
this._log(`\\ From ${lookupMap.length} keybinding entries, matched chord, when: ${printWhenExplanation(result.when)}, source: ${printSourceExplanation(result)}.`);
return {
Expand All @@ -326,6 +334,15 @@ export class KeybindingResolver {
commandArgs: null,
bubble: false
};
} else if (currentChord !== null && currentChord.length + 1 < result.chords.length) {
this._log(`\\ From ${lookupMap.length} keybinding entries, continued chord, when: ${printWhenExplanation(result.when)}, source: ${printSourceExplanation(result)}.`);
return {
enterMultiChord: false,
leaveMultiChord: false,
commandId: null,
commandArgs: null,
bubble: false
};
}

this._log(`\\ From ${lookupMap.length} keybinding entries, matched ${result.command}, when: ${printWhenExplanation(result.when)}, source: ${printSourceExplanation(result)}.`);
Expand Down
36 changes: 27 additions & 9 deletions src/vs/platform/keybinding/test/common/keybindingResolver.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function createContext(ctx: any) {

suite('KeybindingResolver', () => {

function kbItem(keybinding: number, command: string, commandArgs: any, when: ContextKeyExpression | undefined, isDefault: boolean): ResolvedKeybindingItem {
function kbItem(keybinding: number | number[], command: string, commandArgs: any, when: ContextKeyExpression | undefined, isDefault: boolean): ResolvedKeybindingItem {
const resolvedKeybinding = createUSLayoutResolvedKeybinding(keybinding, OS);
return new ResolvedKeybindingItem(
resolvedKeybinding,
Expand Down Expand Up @@ -304,7 +304,7 @@ suite('KeybindingResolver', () => {

test('resolve command', () => {

function _kbItem(keybinding: number, command: string, when: ContextKeyExpression | undefined): ResolvedKeybindingItem {
function _kbItem(keybinding: number | number[], command: string, when: ContextKeyExpression | undefined): ResolvedKeybindingItem {
return kbItem(keybinding, command, null, when, true);
}

Expand Down Expand Up @@ -383,12 +383,17 @@ suite('KeybindingResolver', () => {
KeyMod.CtrlCmd | KeyCode.KeyG,
'eleven',
undefined
)
),
_kbItem(
[KeyMod.CtrlCmd | KeyCode.KeyK, KeyCode.KeyA, KeyCode.KeyB],
'long multi chord',
undefined
),
];

const resolver = new KeybindingResolver(items, [], () => { });

const testKey = (commandId: string, expectedKeys: number[]) => {
const testKey = (commandId: string, expectedKeys: number[] | number[][]) => {
// Test lookup
const lookupResult = resolver.lookupKeybindings(commandId);
assert.strictEqual(lookupResult.length, expectedKeys.length, 'Length mismatch @ commandId ' + commandId);
Expand All @@ -399,10 +404,10 @@ suite('KeybindingResolver', () => {
}
};

const testResolve = (ctx: IContext, _expectedKey: number, commandId: string) => {
const testResolve = (ctx: IContext, _expectedKey: number | number[], commandId: string) => {
const expectedKeybinding = decodeKeybinding(_expectedKey, OS)!;

let previousChord: (string | null) = null;
let previousChord: string[] | null = null;
for (let i = 0, len = expectedKeybinding.chords.length; i < len; i++) {
const chord = getDispatchStr(<KeyCodeChord>expectedKeybinding.chords[i]);
const result = resolver.resolve(ctx, previousChord, chord);
Expand All @@ -412,14 +417,24 @@ suite('KeybindingResolver', () => {
assert.ok(result !== null, `Enters multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.commandId, commandId, `Enters multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.enterMultiChord, false, `Enters multi chord for ${commandId} at chord ${i}`);
} else if (i > 0) {
// if this is an intermediate chord, we should not find a valid command,
// and there should be an open chord we continue.
assert.ok(result !== null, `Continues multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.commandId, null, `Continues multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.enterMultiChord, false, `Is already in multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.leaveMultiChord, false, `Does not leave multi chord for ${commandId} at chord ${i}`);
} else {
// if it's not the final chord, then we should not find a valid command,
// and there should be a chord.
// if it's not the final chord and not an intermediate, then we should not
// find a valid command, and we should enter a chord.
assert.ok(result !== null, `Enters multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.commandId, null, `Enters multi chord for ${commandId} at chord ${i}`);
assert.strictEqual(result!.enterMultiChord, true, `Enters multi chord for ${commandId} at chord ${i}`);
}
previousChord = chord;
if (previousChord === null) {
previousChord = [];
}
previousChord.push(chord);
}
};

Expand Down Expand Up @@ -452,5 +467,8 @@ suite('KeybindingResolver', () => {
testResolve(createContext({}), KeyMod.CtrlCmd | KeyCode.KeyG, 'eleven');

testKey('sixth', []);

testKey('long multi chord', [[KeyMod.CtrlCmd | KeyCode.KeyK, KeyCode.KeyA, KeyCode.KeyB]]);
testResolve(createContext({}), [KeyMod.CtrlCmd | KeyCode.KeyK, KeyCode.KeyA, KeyCode.KeyB], 'long multi chord');
});
});
2 changes: 1 addition & 1 deletion src/vs/platform/keybinding/test/common/keybindingsTestUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { decodeKeybinding, ResolvedKeybinding } from 'vs/base/common/keybindings
import { OperatingSystem } from 'vs/base/common/platform';
import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';

export function createUSLayoutResolvedKeybinding(encodedKeybinding: number, OS: OperatingSystem): ResolvedKeybinding | undefined {
export function createUSLayoutResolvedKeybinding(encodedKeybinding: number | number[], OS: OperatingSystem): ResolvedKeybinding | undefined {
if (encodedKeybinding === 0) {
return undefined;
}
Expand Down
Loading

0 comments on commit a0a30a5

Please sign in to comment.