From 66f4bf35961f4ee1c4d46810c0c76f3df9dbeb15 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 28 Nov 2016 19:04:21 +0100 Subject: [PATCH] debt - manually set command arg when getting menu actions --- .../contextmenu/browser/contextmenu.ts | 2 +- .../actions/browser/menuItemActionItem.ts | 30 ++++----- src/vs/platform/actions/common/actions.ts | 67 +++++++------------ src/vs/platform/actions/common/menuService.ts | 17 ++--- .../actions/test/common/menuService.test.ts | 2 +- .../browser/parts/editor/titleControl.ts | 4 +- .../files/browser/views/explorerViewer.ts | 4 +- .../markers/browser/markersTreeController.ts | 2 +- 8 files changed, 55 insertions(+), 73 deletions(-) diff --git a/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts b/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts index 45779c0ad24..a356d54f71d 100644 --- a/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts +++ b/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts @@ -127,7 +127,7 @@ export class ContextMenuController implements IEditorContribution { const result: IAction[] = []; let contextMenu = this._menuService.createMenu(MenuId.EditorContext, this._contextKeyService); - const groups = contextMenu.getActions(); + const groups = contextMenu.getActions(this._editor.getModel().uri); contextMenu.dispose(); for (let group of groups) { diff --git a/src/vs/platform/actions/browser/menuItemActionItem.ts b/src/vs/platform/actions/browser/menuItemActionItem.ts index b3222b7822e..64152cc193b 100644 --- a/src/vs/platform/actions/browser/menuItemActionItem.ts +++ b/src/vs/platform/actions/browser/menuItemActionItem.ts @@ -17,8 +17,8 @@ import { domEvent } from 'vs/base/browser/event'; import { Emitter } from 'vs/base/common/event'; -export function fillInActions(menu: IMenu, target: IAction[] | { primary: IAction[]; secondary: IAction[]; }): void { - const groups = menu.getActions(); +export function fillInActions(menu: IMenu, context: any, target: IAction[] | { primary: IAction[]; secondary: IAction[]; }): void { + const groups = menu.getActions(context); if (groups.length === 0) { return; } @@ -98,21 +98,18 @@ class MenuItemActionItem extends ActionItem { @IKeybindingService private _keybindingService: IKeybindingService, @IMessageService private _messageService: IMessageService ) { - super(undefined, action, { icon: !!action.command.iconClass, label: !action.command.iconClass }); + super(undefined, action, { icon: !!action.class, label: !action.class }); } private get _command() { - const {command, altCommand} = this._action; - return this._wantsAltCommand && altCommand || command; + return this._wantsAltCommand && (this._action).alt || this._action; } onClick(event: MouseEvent): void { event.preventDefault(); event.stopPropagation(); - (this._action).run(this._wantsAltCommand).done(undefined, err => { - this._messageService.show(Severity.Error, err); - }); + this._command.run().done(undefined, err => this._messageService.show(Severity.Error, err)); } render(container: HTMLElement): void { @@ -149,7 +146,7 @@ class MenuItemActionItem extends ActionItem { _updateLabel(): void { if (this.options.label) { - this.$e.text(this._command.title); + this.$e.text(this._command.label); } } @@ -159,20 +156,19 @@ class MenuItemActionItem extends ActionItem { const keybindingLabel = keybinding && this._keybindingService.getLabelFor(keybinding); element.title = keybindingLabel - ? localize('titleAndKb', "{0} ({1})", this._command.title, keybindingLabel) - : this._command.title; + ? localize('titleAndKb', "{0} ({1})", this._command.label, keybindingLabel) + : this._command.label; } _updateClass(): void { if (this.options.icon) { const element = this.$e.getHTMLElement(); - const {command, altCommand} = (this._action); - if (this._command !== command) { - element.classList.remove(command.iconClass); - } else if (altCommand) { - element.classList.remove(altCommand.iconClass); + if (this._command !== this._action) { + element.classList.remove(this._action.class); + } else if ((this._action).alt) { + element.classList.remove((this._action).alt.class); } - element.classList.add('icon', this._command.iconClass); + element.classList.add('icon', this._command.class); } } } diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index b6f363a3f67..7c870752389 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import URI from 'vs/base/common/uri'; import { IAction, Action } from 'vs/base/common/actions'; import { Promise, TPromise } from 'vs/base/common/winjs.base'; import { SyncDescriptor0, createSyncDescriptor, AsyncDescriptor0 } from 'vs/platform/instantiation/common/descriptors'; @@ -24,7 +23,7 @@ export interface ICommandAction { export interface IMenu extends IDisposable { onDidChange: Event; - getActions(): [string, IAction[]][]; + getActions(arg?: any): [string, MenuItemAction[]][]; } export interface IMenuItem { @@ -100,59 +99,45 @@ export const MenuRegistry: IMenuRegistry = new class { } }; - -export class MenuItemAction extends Action { - - private static _getMenuItemId(item: IMenuItem): string { - let result = item.command.id; - if (item.alt) { - result += `||${item.alt.id}`; - } - return result; - } +export class ExecuteCommandAction extends Action { constructor( - private _item: IMenuItem, - @ICommandService private _commandService: ICommandService, - @IContextKeyService private _contextKeyService: IContextKeyService - ) { - super(MenuItemAction._getMenuItemId(_item), _item.command.title); - - this.order = this._item.order; //TODO@Ben order is menu item property, not an action property - } - - get item(): IMenuItem { - return this._item; - } - - get command() { - return this._item.command; - } + id: string, + label: string, + @ICommandService private _commandService: ICommandService) { - get altCommand() { - return this._item.alt; + super(id, label); } - run(alt: boolean) { - const {id} = alt === true && this._item.alt || this._item.command; - const resource = this._contextKeyService.getContextKeyValue('resource'); - return this._commandService.executeCommand(id, resource); + run(...args: any[]): TPromise { + return this._commandService.executeCommand(this.id, ...args); } } +export class MenuItemAction extends ExecuteCommandAction { -export class ExecuteCommandAction extends Action { + private _arg: any; + + readonly item: ICommandAction; + readonly alt: MenuItemAction; constructor( - id: string, - label: string, - @ICommandService private _commandService: ICommandService) { + item: ICommandAction, + alt: ICommandAction, + arg: any, + @ICommandService commandService: ICommandService + ) { + super(item.id, item.title, commandService); + this._cssClass = item.iconClass; + this._enabled = true; + this._arg = arg; - super(id, label); + this.item = item; + this.alt = alt ? new MenuItemAction(alt, undefined, arg, commandService) : undefined; } - run(...args: any[]): TPromise { - return this._commandService.executeCommand(this.id, ...args); + run(): TPromise { + return super.run(this._arg); } } diff --git a/src/vs/platform/actions/common/menuService.ts b/src/vs/platform/actions/common/menuService.ts index 74d096dc3aa..575df57988c 100644 --- a/src/vs/platform/actions/common/menuService.ts +++ b/src/vs/platform/actions/common/menuService.ts @@ -7,7 +7,6 @@ import Event, { Emitter } from 'vs/base/common/event'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { IAction } from 'vs/base/common/actions'; import { values } from 'vs/base/common/collections'; import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { MenuId, MenuRegistry, ICommandAction, MenuItemAction, IMenu, IMenuItem, IMenuService } from 'vs/platform/actions/common/actions'; @@ -34,7 +33,7 @@ export class MenuService implements IMenuService { } } -type MenuItemGroup = [string, MenuItemAction[]]; +type MenuItemGroup = [string, IMenuItem[]]; class Menu implements IMenu { @@ -63,7 +62,7 @@ class Menu implements IMenu { group = [groupName, []]; this._menuGroups.push(group); } - group[1].push(new MenuItemAction(item, this._commandService, this._contextKeyService)); + group[1].push(item); // keep keys for eventing Menu._fillInKbExprKeys(item.when, keysFilter); @@ -92,13 +91,15 @@ class Menu implements IMenu { return this._onDidChange.event; } - getActions(): [string, IAction[]][] { - const result: MenuItemGroup[] = []; + getActions(arg?: any): [string, MenuItemAction[]][] { + const result: [string, MenuItemAction[]][] = []; for (let group of this._menuGroups) { - const [id, actions] = group; + const [id, items] = group; const activeActions: MenuItemAction[] = []; - for (let action of actions) { - if (this._contextKeyService.contextMatchesRules(action.item.when)) { + for (const item of items) { + if (this._contextKeyService.contextMatchesRules(item.when)) { + const action = new MenuItemAction(item.command, item.alt, arg, this._commandService); + action.order = item.order; //TODO@Ben order is menu item property, not an action property activeActions.push(action); } } diff --git a/src/vs/platform/actions/test/common/menuService.test.ts b/src/vs/platform/actions/test/common/menuService.test.ts index e35df331861..4cc8cc02d73 100644 --- a/src/vs/platform/actions/test/common/menuService.test.ts +++ b/src/vs/platform/actions/test/common/menuService.test.ts @@ -186,4 +186,4 @@ suite('MenuService', function () { assert.equal(two.id, 'b'); assert.equal(three.id, 'a'); }); -}); \ No newline at end of file +}); diff --git a/src/vs/workbench/browser/parts/editor/titleControl.ts b/src/vs/workbench/browser/parts/editor/titleControl.ts index 3e6817ec209..26b01ca287f 100644 --- a/src/vs/workbench/browser/parts/editor/titleControl.ts +++ b/src/vs/workbench/browser/parts/editor/titleControl.ts @@ -332,7 +332,7 @@ export abstract class TitleControl implements ITitleAreaControl { const titleBarMenu = this.menuService.createMenu(MenuId.EditorTitle, scopedContextKeyService); this.disposeOnEditorActions.push(titleBarMenu, titleBarMenu.onDidChange(_ => this.update())); - fillInActions(titleBarMenu, { primary, secondary }); + fillInActions(titleBarMenu, this.resourceContext.get(), { primary, secondary }); } return { primary, secondary }; @@ -480,7 +480,7 @@ export abstract class TitleControl implements ITitleAreaControl { } // Fill in contributed actions - fillInActions(this.contextMenu, actions); + fillInActions(this.contextMenu, this.resourceContext.get(), actions); return actions; } diff --git a/src/vs/workbench/parts/files/browser/views/explorerViewer.ts b/src/vs/workbench/parts/files/browser/views/explorerViewer.ts index de3bd799aeb..a3c357841b8 100644 --- a/src/vs/workbench/parts/files/browser/views/explorerViewer.ts +++ b/src/vs/workbench/parts/files/browser/views/explorerViewer.ts @@ -507,7 +507,7 @@ export class FileController extends DefaultController { getAnchor: () => anchor, getActions: () => { return this.state.actionProvider.getSecondaryActions(tree, stat).then(actions => { - fillInActions(this.contributedContextMenu, actions); + fillInActions(this.contributedContextMenu, stat.resource, actions); return actions; }); }, @@ -959,4 +959,4 @@ export class FileDragAndDrop implements IDragAndDrop { promise.done(null, errors.onUnexpectedError); } -} \ No newline at end of file +} diff --git a/src/vs/workbench/parts/markers/browser/markersTreeController.ts b/src/vs/workbench/parts/markers/browser/markersTreeController.ts index 6b246e37528..eb1ef5ce954 100644 --- a/src/vs/workbench/parts/markers/browser/markersTreeController.ts +++ b/src/vs/workbench/parts/markers/browser/markersTreeController.ts @@ -156,4 +156,4 @@ export class Controller extends treedefaults.DefaultController { } return null; } -} \ No newline at end of file +} -- GitLab