Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-05-16 13:21:04 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #134890 from microsoft/alex/electron-accelerators

Browse Source 
Revisit Electron accelerators
This commit is contained in:
Alexandru Dima
2021-10-13 07:48:32 +02:00
committed by GitHub
parent 793dc93890 779360bdd6
commit d0cdfb9a2d
12 changed files with 87 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vs/base/common/keyCodes.ts
+39 -1
View File
@@ -378,6 +378,33 @@ export namespace KeyCodeUtils {
export function fromUserSettings(key: string): KeyCode {
return userSettingsUSMap.strToKeyCode(key) || userSettingsGeneralMap.strToKeyCode(key);
}
export function toElectronAccelerator(keyCode: KeyCode): string | null {
if (keyCode >= KeyCode.NUMPAD_0 && keyCode <= KeyCode.NUMPAD_DIVIDE) {
// [Electron Accelerators] Electron is able to parse numpad keys, but unfortunately it
// renders them just as regular keys in menus. For example, num0 is rendered as "0",
// numdiv is rendered as "/", numsub is rendered as "-".
//
// This can lead to incredible confusion, as it makes numpad based keybindings indistinguishable
// from keybindings based on regular keys.
//
// We therefore need to fall back to custom rendering for numpad keys.
return null;
}
switch (keyCode) {
case KeyCode.UpArrow:
return 'Up';
case KeyCode.DownArrow:
return 'Down';
case KeyCode.LeftArrow:
return 'Left';
case KeyCode.RightArrow:
return 'Right';
}
return uiMap.keyCodeToStr(keyCode);
}
}
/**
@@ -442,7 +469,18 @@ export function createSimpleKeybinding(keybinding: number, OS: OperatingSystem):
return new SimpleKeybinding(ctrlKey, shiftKey, altKey, metaKey, keyCode);
}
export class SimpleKeybinding {
export interface Modifiers {
readonly ctrlKey: boolean;
readonly shiftKey: boolean;
readonly altKey: boolean;
readonly metaKey: boolean;
}
export interface IBaseKeybinding extends Modifiers {
isDuplicateModifierCase(): boolean;
}
export class SimpleKeybinding implements IBaseKeybinding {
public readonly ctrlKey: boolean;
public readonly shiftKey: boolean;
public readonly altKey: boolean;
src/vs/base/common/keybindingLabels.ts
+1 -7
View File
@@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Modifiers } from 'vs/base/common/keyCodes';
import { OperatingSystem } from 'vs/base/common/platform';
import * as nls from 'vs/nls';
@@ -14,13 +15,6 @@ export interface ModifierLabels {
readonly separator: string;
}
export interface Modifiers {
readonly ctrlKey: boolean;
readonly shiftKey: boolean;
readonly altKey: boolean;
readonly metaKey: boolean;
}
export interface KeyLabelProvider<T extends Modifiers> {
(keybinding: T): string | null;
}
src/vs/base/common/scanCode.ts
+2 -2
View File
@@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { KeyCode } from 'vs/base/common/keyCodes';
import { IBaseKeybinding, KeyCode } from 'vs/base/common/keyCodes';
/**
* keyboardEvent.code
@@ -228,7 +228,7 @@ export const IMMUTABLE_CODE_TO_KEY_CODE: KeyCode[] = [];
*/
export const IMMUTABLE_KEY_CODE_TO_CODE: ScanCode[] = [];
export class ScanCodeBinding {
export class ScanCodeBinding implements IBaseKeybinding {
public readonly ctrlKey: boolean;
public readonly shiftKey: boolean;
public readonly altKey: boolean;
src/vs/platform/keybinding/common/abstractKeybindingService.ts
+4 -1
View File
@@ -173,8 +173,11 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
}
public dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void {
this._log(`/ Dispatching keybinding triggered via menu entry accelerator - ${userSettingsLabel}`);
const keybindings = this.resolveUserBinding(userSettingsLabel);
if (keybindings.length >= 1) {
if (keybindings.length === 0) {
this._log(`\\ Could not resolve - ${userSettingsLabel}`);
} else {
this._doDispatch(keybindings[0], target, /*isSingleModiferChord*/false);
}
}
src/vs/platform/keybinding/common/baseResolvedKeybinding.ts
+9 -4
View File
@@ -4,11 +4,11 @@
*--------------------------------------------------------------------------------------------*/
import { illegalArgument } from 'vs/base/common/errors';
import { AriaLabelProvider, ElectronAcceleratorLabelProvider, Modifiers, UILabelProvider, UserSettingsLabelProvider } from 'vs/base/common/keybindingLabels';
import { KeybindingModifier, ResolvedKeybinding, ResolvedKeybindingPart } from 'vs/base/common/keyCodes';
import { AriaLabelProvider, ElectronAcceleratorLabelProvider, UILabelProvider, UserSettingsLabelProvider } from 'vs/base/common/keybindingLabels';
import { IBaseKeybinding, KeybindingModifier, ResolvedKeybinding, ResolvedKeybindingPart } from 'vs/base/common/keyCodes';
import { OperatingSystem } from 'vs/base/common/platform';
export abstract class BaseResolvedKeybinding<T extends Modifiers> extends ResolvedKeybinding {
export abstract class BaseResolvedKeybinding<T extends IBaseKeybinding> extends ResolvedKeybinding {
protected readonly _os: OperatingSystem;
protected readonly _parts: T[];
@@ -32,7 +32,12 @@ export abstract class BaseResolvedKeybinding<T extends Modifiers> extends Resolv
public getElectronAccelerator(): string | null {
if (this._parts.length > 1) {
// Electron cannot handle chords
// [Electron Accelerators] Electron cannot handle chords
return null;
}
if (this._parts[0].isDuplicateModifierCase()) {
// [Electron Accelerators] Electron cannot handle modifier only keybindings
// e.g. "shift shift"
return null;
}
return ElectronAcceleratorLabelProvider.toLabel(this._os, this._parts, (keybinding) => this._getElectronAccelerator(keybinding));
src/vs/platform/keybinding/common/usLayoutResolvedKeybinding.ts
+1 -24
View File
@@ -46,31 +46,8 @@ export class USLayoutResolvedKeybinding extends BaseResolvedKeybinding<SimpleKey
return KeyCodeUtils.toString(keybinding.keyCode);
}
private _keyCodeToElectronAccelerator(keyCode: KeyCode): string | null {
if (keyCode >= KeyCode.NUMPAD_0 && keyCode <= KeyCode.NUMPAD_DIVIDE) {
// Electron cannot handle numpad keys
return null;
}
switch (keyCode) {
case KeyCode.UpArrow:
return 'Up';
case KeyCode.DownArrow:
return 'Down';
case KeyCode.LeftArrow:
return 'Left';
case KeyCode.RightArrow:
return 'Right';
}
return KeyCodeUtils.toString(keyCode);
}
protected _getElectronAccelerator(keybinding: SimpleKeybinding): string | null {
if (keybinding.isDuplicateModifierCase()) {
return null;
}
return this._keyCodeToElectronAccelerator(keybinding.keyCode);
return KeyCodeUtils.toElectronAccelerator(keybinding.keyCode);
}
protected _getUserSettingsLabel(keybinding: SimpleKeybinding): string | null {
src/vs/workbench/services/keybinding/common/macLinuxKeyboardMapper.ts
+7 -34
View File
@@ -872,45 +872,24 @@ export class MacLinuxKeyboardMapper implements IKeyboardMapper {
return this._scanCodeToDispatch[binding.scanCode];
}
private _getElectronLabelForKeyCode(keyCode: KeyCode): string | null {
if (keyCode >= KeyCode.NUMPAD_0 && keyCode <= KeyCode.NUMPAD_DIVIDE) {
// Electron cannot handle numpad keys
return null;
}
switch (keyCode) {
case KeyCode.UpArrow:
return 'Up';
case KeyCode.DownArrow:
return 'Down';
case KeyCode.LeftArrow:
return 'Left';
case KeyCode.RightArrow:
return 'Right';
}
// electron menus always do the correct rendering on Windows
return KeyCodeUtils.toString(keyCode);
}
public getElectronAcceleratorLabelForScanCodeBinding(binding: ScanCodeBinding | null): string | null {
if (!binding) {
return null;
}
if (binding.isDuplicateModifierCase()) {
return null;
}
const immutableKeyCode = IMMUTABLE_CODE_TO_KEY_CODE[binding.scanCode];
if (immutableKeyCode !== KeyCode.DependsOnKbLayout) {
return this._getElectronLabelForKeyCode(immutableKeyCode);
return KeyCodeUtils.toElectronAccelerator(immutableKeyCode);
}
// Check if this scanCode always maps to the same keyCode and back
const constantKeyCode: KeyCode = this._scanCodeKeyCodeMapper.guessStableKeyCode(binding.scanCode);
if (!this._isUSStandard) {
// Electron cannot handle these key codes on anything else than standard US
if (this._OS === OperatingSystem.Linux && !this._isUSStandard) {
// [Electron Accelerators] On Linux, Electron does not handle correctly OEM keys.
// when using a different keyboard layout than US Standard.
// See https://github.com/microsoft/vscode/issues/23706
// See https://github.com/microsoft/vscode/pull/134890#issuecomment-941671791
const isOEMKey = (
constantKeyCode === KeyCode.US_SEMICOLON
|| constantKeyCode === KeyCode.US_EQUAL
@@ -929,14 +908,8 @@ export class MacLinuxKeyboardMapper implements IKeyboardMapper {
}
}
// See https://github.com/microsoft/vscode/issues/108880
if (this._OS === OperatingSystem.Macintosh && binding.ctrlKey && !binding.metaKey && !binding.altKey && constantKeyCode === KeyCode.US_MINUS) {
// ctrl+- and ctrl+shift+- render very similarly in native macOS menus, leading to confusion
return null;
}
if (constantKeyCode !== KeyCode.DependsOnKbLayout) {
return this._getElectronLabelForKeyCode(constantKeyCode);
return KeyCodeUtils.toElectronAccelerator(constantKeyCode);
}
return null;
src/vs/workbench/services/keybinding/common/windowsKeyboardMapper.ts
+1 -47
View File
@@ -67,9 +67,6 @@ export class WindowsNativeResolvedKeybinding extends BaseResolvedKeybinding<Simp
}
protected _getElectronAccelerator(keybinding: SimpleKeybinding): string | null {
if (keybinding.isDuplicateModifierCase()) {
return null;
}
return this._mapper.getElectronAcceleratorForKeyBinding(keybinding);
}
@@ -405,50 +402,7 @@ export class WindowsKeyboardMapper implements IKeyboardMapper {
}
public getElectronAcceleratorForKeyBinding(keybinding: SimpleKeybinding): string | null {
if (!this.isUSStandard) {
// See https://github.com/electron/electron/issues/26888
// Electron does not render accelerators respecting the current keyboard layout since 3.x
const keyCode = keybinding.keyCode;
const isOEMKey = (
keyCode === KeyCode.US_SEMICOLON
|| keyCode === KeyCode.US_EQUAL
|| keyCode === KeyCode.US_COMMA
|| keyCode === KeyCode.US_MINUS
|| keyCode === KeyCode.US_DOT
|| keyCode === KeyCode.US_SLASH
|| keyCode === KeyCode.US_BACKTICK
|| keyCode === KeyCode.US_OPEN_SQUARE_BRACKET
|| keyCode === KeyCode.US_BACKSLASH
|| keyCode === KeyCode.US_CLOSE_SQUARE_BRACKET
|| keyCode === KeyCode.OEM_8
|| keyCode === KeyCode.OEM_102
);
if (isOEMKey) {
return null;
}
}
return this._keyCodeToElectronAccelerator(keybinding.keyCode);
}
private _keyCodeToElectronAccelerator(keyCode: KeyCode): string | null {
if (keyCode >= KeyCode.NUMPAD_0 && keyCode <= KeyCode.NUMPAD_DIVIDE) {
// Electron cannot handle numpad keys
return null;
}
switch (keyCode) {
case KeyCode.UpArrow:
return 'Up';
case KeyCode.DownArrow:
return 'Down';
case KeyCode.LeftArrow:
return 'Left';
case KeyCode.RightArrow:
return 'Right';
}
// electron menus always do the correct rendering on Windows
return KeyCodeUtils.toString(keyCode);
return KeyCodeUtils.toElectronAccelerator(keybinding.keyCode);
}
private _getLabelForKeyCode(keyCode: KeyCode): string {
src/vs/workbench/services/keybinding/test/electron-browser/mac_de_ch.txt
+8 -8
View File
@@ -469,14 +469,14 @@ isUSStandard: false
| Shift+Alt+Period | : | Shift+Alt+; | | Shift+Alt+. | shift+alt+[Period] | null | shift+alt+[Period] | NO |
| Ctrl+Shift+Alt+Period | ÷ | Ctrl+Shift+Alt+; | | Ctrl+Shift+Alt+. | ctrl+shift+alt+[Period] | null | ctrl+shift+alt+[Period] | NO |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| Slash | - | - | | - | - | null | [Slash] | |
| Ctrl+Slash | - | Ctrl+- | | Ctrl+- | ctrl+- | null | ctrl+[Slash] | |
| Shift+Slash | _ | Shift+- | | Shift+- | shift+- | null | shift+[Slash] | |
| Ctrl+Shift+Slash | _ | Ctrl+Shift+- | | Ctrl+Shift+- | ctrl+shift+- | null | ctrl+shift+[Slash] | |
| Alt+Slash | - | Alt+- | | Alt+- | alt+- | null | alt+[Slash] | |
| Ctrl+Alt+Slash | – | Ctrl+Alt+- | | Ctrl+Alt+- | ctrl+alt+- | null | ctrl+alt+[Slash] | |
| Shift+Alt+Slash | _ | Shift+Alt+- | | Shift+Alt+- | shift+alt+- | null | shift+alt+[Slash] | |
| Ctrl+Shift+Alt+Slash | — | Ctrl+Shift+Alt+- | | Ctrl+Shift+Alt+- | ctrl+shift+alt+- | null | ctrl+shift+alt+[Slash] | |
| Slash | - | - | | - | - | - | [Slash] | |
| Ctrl+Slash | - | Ctrl+- | | Ctrl+- | ctrl+- | Ctrl+- | ctrl+[Slash] | |
| Shift+Slash | _ | Shift+- | | Shift+- | shift+- | Shift+- | shift+[Slash] | |
| Ctrl+Shift+Slash | _ | Ctrl+Shift+- | | Ctrl+Shift+- | ctrl+shift+- | Ctrl+Shift+- | ctrl+shift+[Slash] | |
| Alt+Slash | - | Alt+- | | Alt+- | alt+- | Alt+- | alt+[Slash] | |
| Ctrl+Alt+Slash | – | Ctrl+Alt+- | | Ctrl+Alt+- | ctrl+alt+- | Ctrl+Alt+- | ctrl+alt+[Slash] | |
| Shift+Alt+Slash | _ | Shift+Alt+- | | Shift+Alt+- | shift+alt+- | Shift+Alt+- | shift+alt+[Slash] | |
| Ctrl+Shift+Alt+Slash | — | Ctrl+Shift+Alt+- | | Ctrl+Shift+Alt+- | ctrl+shift+alt+- | Ctrl+Shift+Alt+- | ctrl+shift+alt+[Slash] | |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| HW Code combination | Key | KeyCode combination | Pri | UI label | User settings | Electron accelerator | Dispatching string | WYSIWYG |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src/vs/workbench/services/keybinding/test/electron-browser/mac_en_us.txt
+2 -2
View File
@@ -345,9 +345,9 @@ isUSStandard: true
| HW Code combination | Key | KeyCode combination | Pri | UI label | User settings | Electron accelerator | Dispatching string | WYSIWYG |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| Minus | - | - | | - | - | - | [Minus] | |
| Ctrl+Minus | - | Ctrl+- | | Ctrl+- | ctrl+- | null | ctrl+[Minus] | |
| Ctrl+Minus | - | Ctrl+- | | Ctrl+- | ctrl+- | Ctrl+- | ctrl+[Minus] | |
| Shift+Minus | _ | Shift+- | | Shift+- | shift+- | Shift+- | shift+[Minus] | |
| Ctrl+Shift+Minus | _ | Ctrl+Shift+- | | Ctrl+Shift+- | ctrl+shift+- | null | ctrl+shift+[Minus] | |
| Ctrl+Shift+Minus | _ | Ctrl+Shift+- | | Ctrl+Shift+- | ctrl+shift+- | Ctrl+Shift+- | ctrl+shift+[Minus] | |
| Alt+Minus | - | Alt+- | | Alt+- | alt+- | Alt+- | alt+[Minus] | |
| Ctrl+Alt+Minus | – | Ctrl+Alt+- | | Ctrl+Alt+- | ctrl+alt+- | Ctrl+Alt+- | ctrl+alt+[Minus] | |
| Shift+Alt+Minus | _ | Shift+Alt+- | | Shift+Alt+- | shift+alt+- | Shift+Alt+- | shift+alt+[Minus] | |
src/vs/workbench/services/keybinding/test/electron-browser/mac_zh_hant.txt
+8 -8
View File
@@ -353,14 +353,14 @@ isUSStandard: false
| Shift+Alt+Minus | _ | Shift+Alt+- | | Shift+Alt+ㄦ | shift+alt+[Minus] | null | shift+alt+[Minus] | NO |
| Ctrl+Shift+Alt+Minus | _ | Ctrl+Shift+Alt+- | | Ctrl+Shift+Alt+ㄦ | ctrl+shift+alt+[Minus] | null | ctrl+shift+alt+[Minus] | NO |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| Equal | = | = | | = | = | null | [Equal] | |
| Ctrl+Equal | = | Ctrl+= | | Ctrl+= | ctrl+= | null | ctrl+[Equal] | |
| Shift+Equal | + | Shift+= | | Shift+= | shift+= | null | shift+[Equal] | |
| Ctrl+Shift+Equal | + | Ctrl+Shift+= | | Ctrl+Shift+= | ctrl+shift+= | null | ctrl+shift+[Equal] | |
| Alt+Equal | = | Alt+= | | Alt+= | alt+= | null | alt+[Equal] | |
| Ctrl+Alt+Equal | = | Ctrl+Alt+= | | Ctrl+Alt+= | ctrl+alt+= | null | ctrl+alt+[Equal] | |
| Shift+Alt+Equal | + | Shift+Alt+= | | Shift+Alt+= | shift+alt+= | null | shift+alt+[Equal] | |
| Ctrl+Shift+Alt+Equal | + | Ctrl+Shift+Alt+= | | Ctrl+Shift+Alt+= | ctrl+shift+alt+= | null | ctrl+shift+alt+[Equal] | |
| Equal | = | = | | = | = | = | [Equal] | |
| Ctrl+Equal | = | Ctrl+= | | Ctrl+= | ctrl+= | Ctrl+= | ctrl+[Equal] | |
| Shift+Equal | + | Shift+= | | Shift+= | shift+= | Shift+= | shift+[Equal] | |
| Ctrl+Shift+Equal | + | Ctrl+Shift+= | | Ctrl+Shift+= | ctrl+shift+= | Ctrl+Shift+= | ctrl+shift+[Equal] | |
| Alt+Equal | = | Alt+= | | Alt+= | alt+= | Alt+= | alt+[Equal] | |
| Ctrl+Alt+Equal | = | Ctrl+Alt+= | | Ctrl+Alt+= | ctrl+alt+= | Ctrl+Alt+= | ctrl+alt+[Equal] | |
| Shift+Alt+Equal | + | Shift+Alt+= | | Shift+Alt+= | shift+alt+= | Shift+Alt+= | shift+alt+[Equal] | |
| Ctrl+Shift+Alt+Equal | + | Ctrl+Shift+Alt+= | | Ctrl+Shift+Alt+= | ctrl+shift+alt+= | Ctrl+Shift+Alt+= | ctrl+shift+alt+[Equal] | |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
| BracketLeft | 「 | | | 「 | [BracketLeft] | null | [BracketLeft] | NO |
| Ctrl+BracketLeft | 「 | | | Ctrl+「 | ctrl+[BracketLeft] | null | ctrl+[BracketLeft] | NO |
src/vs/workbench/services/keybinding/test/electron-browser/windowsKeyboardMapper.test.ts
+5 -5
View File
@@ -100,7 +100,7 @@ suite('keyboardMapper - WINDOWS de_ch', () => {
[{
label: 'Ctrl+^',
ariaLabel: 'Control+^',
electronAccelerator: null,
electronAccelerator: 'Ctrl+]',
userSettingsLabel: 'ctrl+oem_6',
isWYSIWYG: false,
isChord: false,
@@ -125,7 +125,7 @@ suite('keyboardMapper - WINDOWS de_ch', () => {
{
label: 'Ctrl+^',
ariaLabel: 'Control+^',
electronAccelerator: null,
electronAccelerator: 'Ctrl+]',
userSettingsLabel: 'ctrl+oem_6',
isWYSIWYG: false,
isChord: false,
@@ -142,7 +142,7 @@ suite('keyboardMapper - WINDOWS de_ch', () => {
[{
label: 'Shift+^',
ariaLabel: 'Shift+^',
electronAccelerator: null,
electronAccelerator: 'Shift+]',
userSettingsLabel: 'shift+oem_6',
isWYSIWYG: false,
isChord: false,
@@ -159,7 +159,7 @@ suite('keyboardMapper - WINDOWS de_ch', () => {
[{
label: 'Ctrl+§',
ariaLabel: 'Control+§',
electronAccelerator: null,
electronAccelerator: 'Ctrl+/',
userSettingsLabel: 'ctrl+oem_2',
isWYSIWYG: false,
isChord: false,
@@ -176,7 +176,7 @@ suite('keyboardMapper - WINDOWS de_ch', () => {
[{
label: 'Ctrl+Shift+§',
ariaLabel: 'Control+Shift+§',
electronAccelerator: null,
electronAccelerator: 'Ctrl+Shift+/',
userSettingsLabel: 'ctrl+shift+oem_2',
isWYSIWYG: false,
isChord: false,