Fix keybinding configuration and menu service (microsoft#209970)

Configure Keybinding: fix when and modify label

Author: benibenj

src/vs/platform/actions/common/menuService.ts

@@ -14,6 +14,7 @@ import { IAction, Separator, toAction } from 'vs/base/common/actions';
 import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
 import { removeFastWithoutKeepingOrder } from 'vs/base/common/arrays';
 import { localize } from 'vs/nls';
+import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 
 export class MenuService implements IMenuService {
 
@@ -23,13 +24,14 @@ export class MenuService implements IMenuService {
 
 	constructor(
 		@ICommandService private readonly _commandService: ICommandService,
+		@IKeybindingService private readonly _keybindingService: IKeybindingService,
 		@IStorageService storageService: IStorageService,
 	) {
 		this._hiddenStates = new PersistedMenuHideState(storageService);
 	}
 
 	createMenu(id: MenuId, contextKeyService: IContextKeyService, options?: IMenuCreateOptions): IMenu {
-		return new MenuImpl(id, this._hiddenStates, { emitEventsForSubmenuChanges: false, eventDebounceDelay: 50, ...options }, this._commandService, contextKeyService);
+		return new MenuImpl(id, this._hiddenStates, { emitEventsForSubmenuChanges: false, eventDebounceDelay: 50, ...options }, this._commandService, this._keybindingService, contextKeyService);
 	}
 
 	resetHiddenStates(ids?: MenuId[]): void {
@@ -162,6 +164,7 @@ class MenuInfo {
 		private readonly _hiddenStates: PersistedMenuHideState,
 		private readonly _collectContextKeysForSubmenus: boolean,
 		@ICommandService private readonly _commandService: ICommandService,
+		@IKeybindingService private readonly _keybindingService: IKeybindingService,
 		@IContextKeyService private readonly _contextKeyService: IContextKeyService
 	) {
 		this.refresh();
@@ -245,11 +248,11 @@ class MenuInfo {
 					const menuHide = createMenuHide(this._id, isMenuItem ? item.command : item, this._hiddenStates);
 					if (isMenuItem) {
 						// MenuItemAction
-						const menuKeybinding = createMenuKeybindingAction(this._id, item.command, this._commandService);
+						const menuKeybinding = createMenuKeybindingAction(this._id, item.command, item.when, this._commandService, this._keybindingService);
 						activeActions.push(new MenuItemAction(item.command, item.alt, options, menuHide, menuKeybinding, this._contextKeyService, this._commandService));
 					} else {
 						// SubmenuItemAction
-						const groups = new MenuInfo(item.submenu, this._hiddenStates, this._collectContextKeysForSubmenus, this._commandService, this._contextKeyService).createActionGroups(options);
+						const groups = new MenuInfo(item.submenu, this._hiddenStates, this._collectContextKeysForSubmenus, this._commandService, this._keybindingService, this._contextKeyService).createActionGroups(options);
 						const submenuActions = Separator.join(...groups.map(g => g[1]));
 						if (submenuActions.length > 0) {
 							activeActions.push(new SubmenuItemAction(item, menuHide, submenuActions));
@@ -336,9 +339,10 @@ class MenuImpl implements IMenu {
 		hiddenStates: PersistedMenuHideState,
 		options: Required<IMenuCreateOptions>,
 		@ICommandService commandService: ICommandService,
+		@IKeybindingService keybindingService: IKeybindingService,
 		@IContextKeyService contextKeyService: IContextKeyService
 	) {
-		this._menuInfo = new MenuInfo(id, hiddenStates, options.emitEventsForSubmenuChanges, commandService, contextKeyService);
+		this._menuInfo = new MenuInfo(id, hiddenStates, options.emitEventsForSubmenuChanges, commandService, keybindingService, contextKeyService);
 
 		// Rebuild this menu whenever the menu registry reports an event for this MenuId.
 		// This usually happen while code and extensions are loaded and affects the over
@@ -438,17 +442,17 @@ function createMenuHide(menu: MenuId, command: ICommandAction | ISubmenuItem, st
 	};
 }
 
-function createMenuKeybindingAction(menu: MenuId, command: ICommandAction | ISubmenuItem, commandService: ICommandService): IAction | undefined {
+function createMenuKeybindingAction(menu: MenuId, command: ICommandAction | ISubmenuItem, when: ContextKeyExpression | undefined = undefined, commandService: ICommandService, keybindingService: IKeybindingService): IAction | undefined {
 	if (isISubmenuItem(command)) {
 		return undefined;
 	}
 
 	const configureKeybindingAction = toAction({
 		id: `configureKeybinding/${menu.id}/${command.id}`,
-		label: localize('configure keybinding', "Configure Keybinding"),
+		label: keybindingService.lookupKeybinding(command.id) ? localize('change keybinding', "Change Keybinding") : localize('configure keybinding', "Configure Keybinding"),
 		run() {
-			const when = command.precondition?.serialize();
-			commandService.executeCommand('workbench.action.openGlobalKeybindings', `@command:${command.id}` + (when ? ` +when:${when}` : ''));
+			const whenValue = when ? when.serialize() : undefined;
+			commandService.executeCommand('workbench.action.openGlobalKeybindings', `@command:${command.id}` + (whenValue ? ` +when:${whenValue}` : ''));
 		}
 	});
 

src/vs/platform/actions/test/common/menuService.test.ts

@@ -10,7 +10,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/uti
 import { isIMenuItem, MenuId, MenuRegistry } from 'vs/platform/actions/common/actions';
 import { MenuService } from 'vs/platform/actions/common/menuService';
 import { NullCommandService } from 'vs/platform/commands/test/common/nullCommandService';
-import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { MockContextKeyService, MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { InMemoryStorageService } from 'vs/platform/storage/common/storage';
 
 // --- service instances
@@ -30,7 +30,7 @@ suite('MenuService', function () {
 	let testMenuId: MenuId;
 
 	setup(function () {
-		menuService = new MenuService(NullCommandService, new InMemoryStorageService());
+		menuService = new MenuService(NullCommandService, new MockKeybindingService(), new InMemoryStorageService());
 		testMenuId = new MenuId(`testo/${generateUuid()}`);
 		disposables.clear();
 	});