From d7559c7bd0512ab0c348dc6e6973458cf6a479ac Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Tue, 28 Mar 2017 09:04:20 +0200 Subject: [PATCH] :bug: don't fwd menu action args automatically fixes #23162 --- src/vs/base/common/actions.ts | 2 +- .../contextmenu/browser/contextmenu.ts | 2 +- .../actions/browser/menuItemActionItem.ts | 6 ++-- src/vs/platform/actions/common/actions.ts | 29 +++++++++++++------ src/vs/platform/actions/common/menu.ts | 6 ++-- .../browser/parts/editor/titleControl.ts | 4 +-- .../debug/electron-browser/debugViewer.ts | 2 +- .../files/browser/views/explorerViewer.ts | 2 +- .../parts/scm/electron-browser/scmMenus.ts | 2 +- 9 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/vs/base/common/actions.ts b/src/vs/base/common/actions.ts index e6c146d1df3..b51eac17ddb 100644 --- a/src/vs/base/common/actions.ts +++ b/src/vs/base/common/actions.ts @@ -234,6 +234,6 @@ export class ActionRunner extends EventEmitter implements IActionRunner { } protected runAction(action: IAction, context?: any): TPromise { - return TPromise.as(action.run(context)); + return TPromise.as(context ? action.run(context) : action.run()); } } diff --git a/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts b/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts index 99307568943..b6140afd67c 100644 --- a/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts +++ b/src/vs/editor/contrib/contextmenu/browser/contextmenu.ts @@ -126,7 +126,7 @@ export class ContextMenuController implements IEditorContribution { const result: IAction[] = []; let contextMenu = this._menuService.createMenu(MenuId.EditorContext, this._contextKeyService); - const groups = contextMenu.getActions(this._editor.getModel().uri); + const groups = contextMenu.getActions({ arg: 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 e57ed6493ce..778b63ae443 100644 --- a/src/vs/platform/actions/browser/menuItemActionItem.ts +++ b/src/vs/platform/actions/browser/menuItemActionItem.ts @@ -7,7 +7,7 @@ import { localize } from 'vs/nls'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; -import { IMenu, MenuItemAction } from 'vs/platform/actions/common/actions'; +import { IMenu, MenuItemAction, IMenuActionOptions } from 'vs/platform/actions/common/actions'; import { IMessageService } from 'vs/platform/message/common/message'; import Severity from 'vs/base/common/severity'; import { IAction } from 'vs/base/common/actions'; @@ -17,8 +17,8 @@ import { domEvent } from 'vs/base/browser/event'; import { Emitter } from 'vs/base/common/event'; -export function fillInActions(menu: IMenu, context: any, target: IAction[] | { primary: IAction[]; secondary: IAction[]; }, isPrimaryGroup: (group: string) => boolean = group => group === 'navigation'): void { - const groups = menu.getActions(context); +export function fillInActions(menu: IMenu, options: IMenuActionOptions, target: IAction[] | { primary: IAction[]; secondary: IAction[]; }, isPrimaryGroup: (group: string) => boolean = group => group === 'navigation'): void { + const groups = menu.getActions(options); if (groups.length === 0) { return; } diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index 6b06f65e6c5..e031efcb702 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -60,9 +60,14 @@ export class MenuId { } } +export interface IMenuActionOptions { + arg?: any; + shouldForwardArgs?: boolean; +} + export interface IMenu extends IDisposable { onDidChange: Event; - getActions(arg?: any): [string, MenuItemAction[]][]; + getActions(options?: IMenuActionOptions): [string, MenuItemAction[]][]; } export const IMenuService = createDecorator('menuService'); @@ -158,7 +163,7 @@ export class ExecuteCommandAction extends Action { export class MenuItemAction extends ExecuteCommandAction { - private _arg: any; + private _options: IMenuActionOptions; readonly item: ICommandAction; readonly alt: MenuItemAction; @@ -166,24 +171,30 @@ export class MenuItemAction extends ExecuteCommandAction { constructor( item: ICommandAction, alt: ICommandAction, - arg: any, + options: IMenuActionOptions, @ICommandService commandService: ICommandService ) { typeof item.title === 'string' ? super(item.id, item.title, commandService) : super(item.id, item.title.value, commandService); this._cssClass = item.iconClass; this._enabled = true; - this._arg = arg; + this._options = options; this.item = item; - this.alt = alt ? new MenuItemAction(alt, undefined, arg, commandService) : undefined; + this.alt = alt ? new MenuItemAction(alt, undefined, this._options, commandService) : undefined; } run(...args: any[]): TPromise { - if (this._arg) { - return super.run(this._arg, ...args); - } else { - return super.run(...args); + let runArgs = []; + + if (this._options.arg) { + runArgs = [...runArgs, this._options.arg]; } + + if (this._options.shouldForwardArgs) { + runArgs = [...runArgs, ...args]; + } + + return super.run(...runArgs); } } diff --git a/src/vs/platform/actions/common/menu.ts b/src/vs/platform/actions/common/menu.ts index 794a399edd7..1ca92edef17 100644 --- a/src/vs/platform/actions/common/menu.ts +++ b/src/vs/platform/actions/common/menu.ts @@ -9,7 +9,7 @@ import Event, { Emitter } from 'vs/base/common/event'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; import { TPromise } from 'vs/base/common/winjs.base'; import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; -import { MenuId, MenuRegistry, MenuItemAction, IMenu, IMenuItem } from 'vs/platform/actions/common/actions'; +import { MenuId, MenuRegistry, MenuItemAction, IMenu, IMenuItem, IMenuActionOptions } from 'vs/platform/actions/common/actions'; import { ICommandService } from 'vs/platform/commands/common/commands'; type MenuItemGroup = [string, IMenuItem[]]; @@ -69,14 +69,14 @@ export class Menu implements IMenu { return this._onDidChange.event; } - getActions(arg?: any): [string, MenuItemAction[]][] { + getActions(options: IMenuActionOptions): [string, MenuItemAction[]][] { const result: [string, MenuItemAction[]][] = []; for (let group of this._menuGroups) { const [id, items] = group; const activeActions: MenuItemAction[] = []; for (const item of items) { if (this._contextKeyService.contextMatchesRules(item.when)) { - const action = new MenuItemAction(item.command, item.alt, arg, this._commandService); + const action = new MenuItemAction(item.command, item.alt, options, 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/workbench/browser/parts/editor/titleControl.ts b/src/vs/workbench/browser/parts/editor/titleControl.ts index 345d247dc08..dbc94f20749 100644 --- a/src/vs/workbench/browser/parts/editor/titleControl.ts +++ b/src/vs/workbench/browser/parts/editor/titleControl.ts @@ -325,7 +325,7 @@ export abstract class TitleControl extends Themable implements ITitleAreaControl const titleBarMenu = this.menuService.createMenu(MenuId.EditorTitle, scopedContextKeyService); this.disposeOnEditorActions.push(titleBarMenu, titleBarMenu.onDidChange(_ => this.update())); - fillInActions(titleBarMenu, this.resourceContext.get(), { primary, secondary }); + fillInActions(titleBarMenu, { arg: this.resourceContext.get() }, { primary, secondary }); } return { primary, secondary }; @@ -475,7 +475,7 @@ export abstract class TitleControl extends Themable implements ITitleAreaControl } // Fill in contributed actions - fillInActions(this.contextMenu, this.resourceContext.get(), actions); + fillInActions(this.contextMenu, { arg: this.resourceContext.get() }, actions); return actions; } diff --git a/src/vs/workbench/parts/debug/electron-browser/debugViewer.ts b/src/vs/workbench/parts/debug/electron-browser/debugViewer.ts index a55a1dee981..f3de12cbbb8 100644 --- a/src/vs/workbench/parts/debug/electron-browser/debugViewer.ts +++ b/src/vs/workbench/parts/debug/electron-browser/debugViewer.ts @@ -220,7 +220,7 @@ export class BaseDebugController extends DefaultController { this.contextMenuService.showContextMenu({ getAnchor: () => anchor, getActions: () => this.actionProvider.getSecondaryActions(tree, element).then(actions => { - fillInActions(this.contributedContextMenu, this.getContext(element), actions); + fillInActions(this.contributedContextMenu, { arg: this.getContext(element) }, actions); return actions; }), onHide: (wasCancelled?: boolean) => { diff --git a/src/vs/workbench/parts/files/browser/views/explorerViewer.ts b/src/vs/workbench/parts/files/browser/views/explorerViewer.ts index 2efdf693a2a..72661420e33 100644 --- a/src/vs/workbench/parts/files/browser/views/explorerViewer.ts +++ b/src/vs/workbench/parts/files/browser/views/explorerViewer.ts @@ -484,7 +484,7 @@ export class FileController extends DefaultController { getAnchor: () => anchor, getActions: () => { return this.state.actionProvider.getSecondaryActions(tree, stat).then(actions => { - fillInActions(this.contributedContextMenu, stat.resource, actions); + fillInActions(this.contributedContextMenu, { arg: stat.resource }, actions); return actions; }); }, diff --git a/src/vs/workbench/parts/scm/electron-browser/scmMenus.ts b/src/vs/workbench/parts/scm/electron-browser/scmMenus.ts index 342985bd6e3..8a359512d65 100644 --- a/src/vs/workbench/parts/scm/electron-browser/scmMenus.ts +++ b/src/vs/workbench/parts/scm/electron-browser/scmMenus.ts @@ -142,7 +142,7 @@ export class SCMMenus implements IDisposable { const primary = []; const secondary = []; const result = { primary, secondary }; - fillInActions(menu, resource.uri, result, g => g === 'inline'); + fillInActions(menu, { arg: resource.uri, shouldForwardArgs: true }, result, g => g === 'inline'); menu.dispose(); contextKeyService.dispose(); -- GitLab