From ed5e512a2f855598edb19c7c2a6ceabe66477bbd Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Fri, 29 May 2020 17:19:05 +0200 Subject: [PATCH 1/3] Do not push an empty string to the result --- src/vs/base/common/keybindingLabels.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/vs/base/common/keybindingLabels.ts b/src/vs/base/common/keybindingLabels.ts index 671816830bdf7..1cdbc8d8e9e39 100644 --- a/src/vs/base/common/keybindingLabels.ts +++ b/src/vs/base/common/keybindingLabels.ts @@ -182,7 +182,9 @@ function _simpleAsString(modifiers: Modifiers, key: string, labels: ModifierLabe } // the actual key - result.push(key); + if (key !== '') { + result.push(key); + } return result.join(labels.separator); } From c68ab245e898706bbfa646d9158274ef31539baf Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 1 Jun 2020 11:13:36 +0200 Subject: [PATCH 2/3] Fix broken tests --- .../macLinuxFallbackKeyboardMapper.test.ts | 10 ++--- .../macLinuxKeyboardMapper.test.ts | 40 +++++++++---------- .../windowsKeyboardMapper.test.ts | 12 +++--- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxFallbackKeyboardMapper.test.ts b/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxFallbackKeyboardMapper.test.ts index c61fee376c9da..171dea63f671c 100644 --- a/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxFallbackKeyboardMapper.test.ts +++ b/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxFallbackKeyboardMapper.test.ts @@ -107,9 +107,9 @@ suite('keyboardMapper - MAC fallback', () => { }, { label: '⌘', - ariaLabel: 'Command+', + ariaLabel: 'Command', electronAccelerator: null, - userSettingsLabel: 'cmd+', + userSettingsLabel: 'cmd', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -228,10 +228,10 @@ suite('keyboardMapper - LINUX fallback', () => { code: null! }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], diff --git a/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxKeyboardMapper.test.ts b/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxKeyboardMapper.test.ts index f4088abc61e45..4576fec3d3063 100644 --- a/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxKeyboardMapper.test.ts +++ b/src/vs/workbench/services/keybinding/test/electron-browser/macLinuxKeyboardMapper.test.ts @@ -344,9 +344,9 @@ suite('keyboardMapper - MAC de_ch', () => { }, { label: '⌘', - ariaLabel: 'Command+', + ariaLabel: 'Command', electronAccelerator: null, - userSettingsLabel: 'cmd+', + userSettingsLabel: 'cmd', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -368,9 +368,9 @@ suite('keyboardMapper - MAC de_ch', () => { }, { label: '⌘', - ariaLabel: 'Command+', + ariaLabel: 'Command', electronAccelerator: null, - userSettingsLabel: 'cmd+', + userSettingsLabel: 'cmd', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -425,9 +425,9 @@ suite('keyboardMapper - MAC en_us', () => { }, { label: '⌘', - ariaLabel: 'Command+', + ariaLabel: 'Command', electronAccelerator: null, - userSettingsLabel: 'cmd+', + userSettingsLabel: 'cmd', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -449,9 +449,9 @@ suite('keyboardMapper - MAC en_us', () => { }, { label: '⌘', - ariaLabel: 'Command+', + ariaLabel: 'Command', electronAccelerator: null, - userSettingsLabel: 'cmd+', + userSettingsLabel: 'cmd', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -780,10 +780,10 @@ suite('keyboardMapper - LINUX de_ch', () => { code: 'ControlLeft' }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -804,10 +804,10 @@ suite('keyboardMapper - LINUX de_ch', () => { code: 'ControlRight' }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -1180,10 +1180,10 @@ suite('keyboardMapper - LINUX en_us', () => { code: 'ControlLeft' }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -1204,10 +1204,10 @@ suite('keyboardMapper - LINUX en_us', () => { code: 'ControlRight' }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], diff --git a/src/vs/workbench/services/keybinding/test/electron-browser/windowsKeyboardMapper.test.ts b/src/vs/workbench/services/keybinding/test/electron-browser/windowsKeyboardMapper.test.ts index 5babd01552e53..8087a1c799254 100644 --- a/src/vs/workbench/services/keybinding/test/electron-browser/windowsKeyboardMapper.test.ts +++ b/src/vs/workbench/services/keybinding/test/electron-browser/windowsKeyboardMapper.test.ts @@ -308,10 +308,10 @@ suite('keyboardMapper - WINDOWS de_ch', () => { code: null! }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], @@ -396,10 +396,10 @@ suite('keyboardMapper - WINDOWS en_us', () => { code: null! }, { - label: 'Ctrl+', - ariaLabel: 'Control+', + label: 'Ctrl', + ariaLabel: 'Control', electronAccelerator: null, - userSettingsLabel: 'ctrl+', + userSettingsLabel: 'ctrl', isWYSIWYG: true, isChord: false, dispatchParts: [null], From 918bf38bb89e79cf110b0ec15b6dc284d4dcf805 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 1 Jun 2020 11:13:47 +0200 Subject: [PATCH 3/3] Add test for #91235 --- .../platform/keybinding/test/common/keybindingLabels.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vs/platform/keybinding/test/common/keybindingLabels.test.ts b/src/vs/platform/keybinding/test/common/keybindingLabels.test.ts index 16e096e564bb4..a321d9b38bb00 100644 --- a/src/vs/platform/keybinding/test/common/keybindingLabels.test.ts +++ b/src/vs/platform/keybinding/test/common/keybindingLabels.test.ts @@ -167,4 +167,7 @@ suite('KeybindingLabels', () => { assertElectronAcceleratorLabel(OperatingSystem.Macintosh, KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_A, KeyMod.CtrlCmd | KeyCode.KEY_B), 'cmd+a cmd+b'); }); + test('issue #91235: Do not end with a +', () => { + assertUSLabel(OperatingSystem.Windows, KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.Alt, 'Ctrl+Alt'); + }); });