Merge pull request #98815 from microsoft/lszomoru/screencast-label

Keybinding - Do not push an empty string to the result
microsoft · Jun 1, 2020 · 8a6c086 · 8a6c086
2 parents 1cfe31d + 918bf38
commit 8a6c086
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 32 deletions.
diff --git 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);
 }
diff --git 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');
+	});
 });
diff --git a/...orkbench/services/keybinding/test/electron-browser/macLinuxFallbackKeyboardMapper.test.ts b/...orkbench/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
@@ -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
@@ -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],