Fixes #1369: Do not give Electron accelerators that change with keybo…

…ard layout under Mac & Linux

src/vs/base/common/keyCodes.ts

@@ -527,7 +527,7 @@ export class Keybinding {
 	 * This prints the binding in a format suitable for electron's accelerators.
 	 * See https://github.com/atom/electron/blob/master/docs/api/accelerator.md
 	 */
-	public toElectronAccelerator(): string {
+	public _toElectronAccelerator(): string {
 		return Keybinding._toElectronAccelerator(this.value);
 	}
 
@@ -553,7 +553,7 @@ export interface IKeyBindingLabelProvider {
 /**
  * Print for Electron
  */
-class ElectronAcceleratorLabelProvider implements IKeyBindingLabelProvider {
+export class ElectronAcceleratorLabelProvider implements IKeyBindingLabelProvider {
 	public static INSTANCE = new ElectronAcceleratorLabelProvider();
 
 	public ctrlKeyLabel = 'Ctrl';

src/vs/platform/keybinding/browser/keybindingServiceImpl.ts

@@ -123,6 +123,7 @@ export abstract class AbstractKeybindingService {
 
 	public abstract getLabelFor(keybinding:Keybinding): string;
 	public abstract getHTMLLabelFor(keybinding:Keybinding): IHTMLContentElement[];
+	public abstract getElectronAcceleratorFor(keybinding:Keybinding): string;
 	public abstract customKeybindingsCount(): number;
 	public abstract getContext(contextId: number): KeybindingContext;
 	public abstract createChildContext(parentContextId?: number): number;
@@ -174,6 +175,10 @@ export class KeybindingService extends AbstractKeybindingService implements IKey
 		return keybinding._toUSHTMLLabel();
 	}
 
+	public getElectronAcceleratorFor(keybinding:Keybinding): string {
+		return keybinding._toElectronAccelerator();
+	}
+
 	protected updateResolver(): void {
 		this._createOrUpdateResolver(false);
 	}
@@ -334,6 +339,10 @@ class ScopedKeybindingService extends AbstractKeybindingService {
 		return this._parent.getHTMLLabelFor(keybinding);
 	}
 
+	public getElectronAcceleratorFor(keybinding:Keybinding): string {
+		return this._parent.getElectronAcceleratorFor(keybinding);
+	}
+
 	public getDefaultKeybindings(): string {
 		return this._parent.getDefaultKeybindings();
 	}

src/vs/platform/keybinding/common/keybindingService.ts

@@ -94,6 +94,7 @@ export interface IKeybindingService {
 
 	getLabelFor(keybinding:Keybinding): string;
 	getHTMLLabelFor(keybinding:Keybinding): IHTMLContentElement[];
+	getElectronAcceleratorFor(keybinding:Keybinding): string;
 
 	executeCommand<T>(commandId: string, args?: any): TPromise<T>;
 	executeCommand(commandId: string, args?: any): TPromise<any>;

src/vs/workbench/electron-browser/integration.ts

@@ -121,11 +121,17 @@ export class ElectronIntegration {
 		return this.partService.joinCreation().then(() => {
 			return arrays.coalesce(actionIds.map((id) => {
 				let bindings = this.keybindingService.lookupKeybindings(id);
-				if (bindings.length) {
-					return {
-						id: id,
-						binding: bindings[0].value	// take first user configured binding
-					};
+
+				// return the first binding that can be represented by electron
+				for (let i = 0; i < bindings.length; i++) {
+					let binding = bindings[i];
+					let electronAccelerator = this.keybindingService.getElectronAcceleratorFor(binding);
+					if (electronAccelerator) {
+						return {
+							id: id,
+							binding: binding.value
+						};
+					}
 				}
 
 				return null;

src/vs/workbench/electron-browser/shell.ts

@@ -326,7 +326,7 @@ export class WorkbenchShell {
 		result.addSingleton(IRequestService, requestService);
 		result.addSingleton(IWorkspaceContextService, this.contextService);
 		result.addSingleton(IContextViewService, this.contextViewService);
-		result.addSingleton(IContextMenuService, new ContextMenuService(this.messageService, this.telemetryService));
+		result.addSingleton(IContextMenuService, new ContextMenuService(this.messageService, this.telemetryService, this.keybindingService));
 		result.addSingleton(IMessageService, this.messageService);
 		result.addSingleton(IStorageService, this.storageService);
 		result.addSingleton(ILifecycleService, lifecycleService);

src/vs/workbench/electron-main/menus.ts

@@ -92,7 +92,7 @@ export class VSCodeMenu {
 
 			let needsMenuUpdate = false;
 			keybindings.forEach((keybinding) => {
-				let accelerator = new Keybinding(keybinding.binding).toElectronAccelerator();
+				let accelerator = new Keybinding(keybinding.binding)._toElectronAccelerator();
 				if (accelerator) {
 					this.mapResolvedKeybindingToActionId[keybinding.id] = accelerator;
 					if (this.mapLastKnownKeybindingToActionId[keybinding.id] !== accelerator) {

src/vs/workbench/services/contextview/electron-browser/contextmenuService.ts

@@ -15,6 +15,7 @@ import {KeybindingsUtils} from 'vs/platform/keybinding/common/keybindingsUtils';
 import {IContextMenuService, IContextMenuDelegate} from 'vs/platform/contextview/browser/contextView';
 import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
 import {IMessageService} from 'vs/platform/message/common/message';
+import {IKeybindingService} from 'vs/platform/keybinding/common/keybindingService';
 
 import remote = require('remote');
 
@@ -25,10 +26,12 @@ export class ContextMenuService implements IContextMenuService {
 	public serviceId = IContextMenuService;
 	private telemetryService: ITelemetryService;
 	private messageService: IMessageService;
+	private keybindingService: IKeybindingService;
 
-	constructor(messageService: IMessageService, telemetryService: ITelemetryService) {
+	constructor(messageService: IMessageService, telemetryService: ITelemetryService, keybindingService: IKeybindingService) {
 		this.messageService = messageService;
 		this.telemetryService = telemetryService;
+		this.keybindingService = keybindingService;
 	}
 
 	public showContextMenu(delegate: IContextMenuDelegate): void {
@@ -45,7 +48,7 @@ export class ContextMenuService implements IContextMenuService {
 					menu.append(new MenuItem({ type: 'separator' }));
 				} else {
 					const keybinding = !!delegate.getKeyBinding ? delegate.getKeyBinding(a) : undefined;
-					const accelerator = keybinding && keybinding.toElectronAccelerator();
+					const accelerator = keybinding && this.keybindingService.getElectronAcceleratorFor(keybinding);
 
 					const item = new MenuItem({
 						label: a.label,

src/vs/workbench/services/keybinding/electron-browser/pluginKeybindingService.ts

@@ -467,6 +467,23 @@ export default class PluginWorkbenchKeybindingService extends WorkbenchKeybindin
 		return keybinding.toCustomHTMLLabel(this._nativeLabelProvider);
 	}
 
+	public getElectronAcceleratorFor(keybinding:Keybinding): string {
+		if (Platform.isWindows) {
+			// electron menus always do the correct rendering on Windows
+			return super.getElectronAcceleratorFor(keybinding);
+		}
+
+		let usLabel = keybinding._toUSLabel();
+		let label = this.getLabelFor(keybinding);
+		if (usLabel !== label) {
+			// electron menus are incorrect in rendering (linux) and in rendering and interpreting (mac)
+			// for non US standard keyboard layouts
+			return null;
+		}
+
+		return super.getElectronAcceleratorFor(keybinding);
+	}
+
 	private _handleKeybindingsExtensionPointUser(isBuiltin: boolean, keybindings:ContributedKeyBinding | ContributedKeyBinding[], collector:IMessageCollector): boolean {
 		if (isContributedKeyBindingsArray(keybindings)) {
 			let commandAdded = false;

src/vs/workbench/test/browser/servicesTestUtils.ts

@@ -173,6 +173,10 @@ export class TestKeybindingService implements IKeybindingService {
 		return keybinding._toUSHTMLLabel();
 	}
 
+	public getElectronAcceleratorFor(keybinding:Keybinding): string {
+		return keybinding._toElectronAccelerator();
+	}
+
 	public createScoped(domNode: HTMLElement): IKeybindingService {
 		return this;
 	}