From 65d45fd985e6b41fcb5900ad4cbbacb637a5e8ac Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 3 Oct 2017 16:33:51 +0200 Subject: [PATCH] wip - make IWorkbenchActionRegistry not hold any data but make it just a forwarder --- .../browser/parts/statusbar/statusbarPart.ts | 34 ++--- src/vs/workbench/common/actions.ts | 119 +++++++++--------- .../quickopen/browser/commandsHandler.ts | 40 +----- .../test/browser/actionRegistry.test.ts | 25 +--- 4 files changed, 72 insertions(+), 146 deletions(-) diff --git a/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts b/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts index 72bfb28604c..011fb2a1548 100644 --- a/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts +++ b/src/vs/workbench/browser/parts/statusbar/statusbarPart.ts @@ -17,7 +17,6 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService'; import { Part } from 'vs/workbench/browser/part'; -import { IWorkbenchActionRegistry, Extensions as ActionExtensions } from 'vs/workbench/common/actions'; import { StatusbarAlignment, IStatusbarRegistry, Extensions, IStatusbarItem } from 'vs/workbench/browser/parts/statusbar/statusbar'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; @@ -287,29 +286,6 @@ class StatusBarEntryItem implements IStatusbarItem { private executeCommand(id: string, args?: any[]) { args = args || []; - // Lookup built in commands - const builtInActionDescriptor = Registry.as(ActionExtensions.WorkbenchActions).getWorkbenchAction(id); - if (builtInActionDescriptor) { - const action = this.instantiationService.createInstance(builtInActionDescriptor.syncDescriptor); - - if (action.enabled) { - /* __GDPR__ - "workbenchActionExecuted" : { - "id" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, - "from": { "classification": "SystemMetaData", "purpose": "FeatureInsight" } - } - */ - this.telemetryService.publicLog('workbenchActionExecuted', { id: action.id, from: 'status bar' }); - (action.run() || TPromise.as(null)).done(() => { - action.dispose(); - }, (err) => this.messageService.show(Severity.Error, toErrorMessage(err))); - } else { - this.messageService.show(Severity.Warning, nls.localize('canNotRun', "Command '{0}' is currently not enabled and can not be run.", action.label || id)); - } - - return; - } - // Maintain old behaviour of always focusing the editor here const activeEditor = this.editorService.getActiveEditor(); const codeEditor = getCodeEditor(activeEditor); @@ -317,7 +293,13 @@ class StatusBarEntryItem implements IStatusbarItem { codeEditor.focus(); } - // Fallback to the command service for any other case + /* __GDPR__ + "workbenchActionExecuted" : { + "id" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }, + "from": { "classification": "SystemMetaData", "purpose": "FeatureInsight" } + } + */ + this.telemetryService.publicLog('workbenchActionExecuted', { id, from: 'status bar' }); this.commandService.executeCommand(id, ...args).done(undefined, err => this.messageService.show(Severity.Error, toErrorMessage(err))); } } @@ -355,4 +337,4 @@ registerThemingParticipant((theme: ITheme, collector: ICssStyleCollector) => { if (statusBarProminentItemHoverBackground) { collector.addRule(`.monaco-workbench > .part.statusbar > .statusbar-item a.status-bar-info:hover:not([disabled]):not(.disabled) { background-color: ${statusBarProminentItemHoverBackground}; }`); } -}); \ No newline at end of file +}); diff --git a/src/vs/workbench/common/actions.ts b/src/vs/workbench/common/actions.ts index 1bd3ba72aa6..03f1aab7ce8 100644 --- a/src/vs/workbench/common/actions.ts +++ b/src/vs/workbench/common/actions.ts @@ -5,13 +5,12 @@ 'use strict'; import { TPromise } from 'vs/base/common/winjs.base'; -import collections = require('vs/base/common/collections'); import { Registry } from 'vs/platform/registry/common/platform'; import { IAction } from 'vs/base/common/actions'; -import { KeybindingsRegistry, ICommandAndKeybindingRule } from 'vs/platform/keybinding/common/keybindingsRegistry'; +import { KeybindingsRegistry } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { IPartService } from 'vs/workbench/services/part/common/partService'; -import { ICommandHandler } from 'vs/platform/commands/common/commands'; -import { SyncActionDescriptor } from 'vs/platform/actions/common/actions'; +import { ICommandHandler, CommandsRegistry } from 'vs/platform/commands/common/commands'; +import { SyncActionDescriptor, MenuRegistry, MenuId } from 'vs/platform/actions/common/actions'; import { IMessageService } from 'vs/platform/message/common/message'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -65,83 +64,87 @@ interface IActionMeta { } class WorkbenchActionRegistry implements IWorkbenchActionRegistry { - private workbenchActions: collections.IStringDictionary; - private mapActionIdToMeta: { [id: string]: IActionMeta; }; - - constructor() { - this.workbenchActions = Object.create(null); - this.mapActionIdToMeta = Object.create(null); - } public registerWorkbenchAction(descriptor: SyncActionDescriptor, alias: string, category?: string): void { - if (!this.workbenchActions[descriptor.id]) { - this.workbenchActions[descriptor.id] = descriptor; - registerWorkbenchCommandFromAction(descriptor); - - const meta: IActionMeta = { alias }; - if (typeof category === 'string') { - meta.category = category; - } - - this.mapActionIdToMeta[descriptor.id] = meta; - } + registerWorkbenchCommandFromAction(descriptor, alias, category); } public unregisterWorkbenchAction(id: string): boolean { - if (!this.workbenchActions[id]) { - return false; - } - - delete this.workbenchActions[id]; - delete this.mapActionIdToMeta[id]; - return true; } public getWorkbenchAction(id: string): SyncActionDescriptor { - return this.workbenchActions[id] || null; + return null; } public getCategory(id: string): string { - return (this.mapActionIdToMeta[id] && this.mapActionIdToMeta[id].category) || null; + const commandAction = MenuRegistry.getCommand(id); + if (!commandAction || !commandAction.category) { + return null; + } + const { category } = commandAction; + if (typeof category === 'string') { + return category; + } else { + return category.value; + } } public getAlias(id: string): string { - return (this.mapActionIdToMeta[id] && this.mapActionIdToMeta[id].alias) || null; + const commandAction = MenuRegistry.getCommand(id); + if (!commandAction) { + return null; + } + const { title } = commandAction; + if (typeof title === 'string') { + return null; + } else { + return title.original; + } } public getWorkbenchActions(): SyncActionDescriptor[] { - return collections.values(this.workbenchActions); - } - - public setWorkbenchActions(actions: SyncActionDescriptor[]): void { - this.workbenchActions = Object.create(null); - this.mapActionIdToMeta = Object.create(null); - - actions.forEach(action => this.registerWorkbenchAction(action, ''), this); + return []; } } Registry.add(Extensions.WorkbenchActions, new WorkbenchActionRegistry()); -function registerWorkbenchCommandFromAction(descriptor: SyncActionDescriptor): void { - const when = descriptor.keybindingContext; - const weight = (typeof descriptor.keybindingWeight === 'undefined' ? KeybindingsRegistry.WEIGHT.workbenchContrib() : descriptor.keybindingWeight); - const keybindings = descriptor.keybindings; - - const desc: ICommandAndKeybindingRule = { - id: descriptor.id, - handler: createCommandHandler(descriptor), - weight: weight, - when: when, - primary: keybindings && keybindings.primary, - secondary: keybindings && keybindings.secondary, - win: keybindings && keybindings.win, - mac: keybindings && keybindings.mac, - linux: keybindings && keybindings.linux - }; +function registerWorkbenchCommandFromAction(descriptor: SyncActionDescriptor, alias: string, category?: string): void { + + CommandsRegistry.registerCommand(descriptor.id, createCommandHandler(descriptor)); + + { + // register keybinding + const when = descriptor.keybindingContext; + const weight = (typeof descriptor.keybindingWeight === 'undefined' ? KeybindingsRegistry.WEIGHT.workbenchContrib() : descriptor.keybindingWeight); + const keybindings = descriptor.keybindings; + KeybindingsRegistry.registerKeybindingRule({ + id: descriptor.id, + weight: weight, + when: when, + primary: keybindings && keybindings.primary, + secondary: keybindings && keybindings.secondary, + win: keybindings && keybindings.win, + mac: keybindings && keybindings.mac, + linux: keybindings && keybindings.linux + }); + } - KeybindingsRegistry.registerCommandAndKeybindingRule(desc); + { + // register menu item + if (descriptor.label) { + // slightly weird if-check required because of + // https://github.com/Microsoft/vscode/blob/d28ace31aa147596e35adf101a27768a048c79ec/src/vs/workbench/parts/files/browser/fileActions.contribution.ts#L194 + MenuRegistry.appendMenuItem(MenuId.CommandPalette, { + command: { + id: descriptor.id, + title: { value: descriptor.label, original: alias }, + category + } + }); + } + } } function createCommandHandler(descriptor: SyncActionDescriptor): ICommandHandler { @@ -193,4 +196,4 @@ function triggerAndDisposeAction(instantitationService: IInstantiationService, t return TPromise.wrapError(err); } }); -} \ No newline at end of file +} diff --git a/src/vs/workbench/parts/quickopen/browser/commandsHandler.ts b/src/vs/workbench/parts/quickopen/browser/commandsHandler.ts index d366f705447..f5e99434096 100644 --- a/src/vs/workbench/parts/quickopen/browser/commandsHandler.ts +++ b/src/vs/workbench/parts/quickopen/browser/commandsHandler.ts @@ -16,8 +16,6 @@ import { Mode, IEntryRunContext, IAutoFocus, IModel, IQuickNavigateConfiguration import { QuickOpenEntryGroup, IHighlight, QuickOpenModel, QuickOpenEntry } from 'vs/base/parts/quickopen/browser/quickOpenModel'; import { SyncActionDescriptor, IMenuService, MenuId, MenuItemAction } from 'vs/platform/actions/common/actions'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; -import { IWorkbenchActionRegistry, Extensions as ActionExtensions } from 'vs/workbench/common/actions'; -import { Registry } from 'vs/platform/registry/common/platform'; import { QuickOpenHandler, IWorkbenchQuickOpenConfiguration } from 'vs/workbench/browser/quickopen'; import { IEditorAction, IEditor, ICommonCodeEditor } from 'vs/editor/common/editorCommon'; import { matchesWords, matchesPrefix, matchesContiguousSubString, or } from 'vs/base/common/filters'; @@ -418,11 +416,6 @@ export class CommandsHandler extends QuickOpenHandler { searchValue = searchValue.trim(); this.lastSearchValue = searchValue; - // Workbench Actions - let workbenchEntries: CommandEntry[] = []; - const workbenchActions = Registry.as(ActionExtensions.WorkbenchActions).getWorkbenchActions(); - workbenchEntries = this.actionDescriptorsToEntries(workbenchActions, searchValue); - // Editor Actions const activeEditor = this.editorService.getActiveEditor(); const activeEditorControl = activeEditor ? activeEditor.getControl() : null; @@ -443,7 +436,7 @@ export class CommandsHandler extends QuickOpenHandler { const commandEntries = this.menuItemActionsToEntries(menuActions, searchValue); // Concat - let entries = [...workbenchEntries, ...editorEntries, ...commandEntries]; + let entries = [...editorEntries, ...commandEntries]; // Remove duplicates entries = arrays.distinct(entries, entry => `${entry.getLabel()}${entry.getGroupLabel()}${entry.getCommandId()}`); @@ -498,35 +491,6 @@ export class CommandsHandler extends QuickOpenHandler { return TPromise.as(new QuickOpenModel(entries)); } - private actionDescriptorsToEntries(actionDescriptors: SyncActionDescriptor[], searchValue: string): CommandEntry[] { - const entries: CommandEntry[] = []; - const registry = Registry.as(ActionExtensions.WorkbenchActions); - - for (let i = 0; i < actionDescriptors.length; i++) { - const actionDescriptor = actionDescriptors[i]; - if (actionDescriptor.label) { - - // Label (with optional category) - let label = actionDescriptor.label; - const category = registry.getCategory(actionDescriptor.id); - if (category) { - label = nls.localize('commandLabel', "{0}: {1}", category, label); - } - - // Alias for non default languages - const alias = (language !== LANGUAGE_DEFAULT) ? registry.getAlias(actionDescriptor.id) : null; - const labelHighlights = wordFilter(searchValue, label); - const aliasHighlights = alias ? wordFilter(searchValue, alias) : null; - - if (labelHighlights || aliasHighlights) { - entries.push(this.instantiationService.createInstance(CommandEntry, actionDescriptor.id, this.keybindingService.lookupKeybinding(actionDescriptor.id), label, alias, { label: labelHighlights, alias: aliasHighlights }, actionDescriptor, (id: string) => this.onBeforeRunCommand(id))); - } - } - } - - return entries; - } - private editorActionsToEntries(actions: IEditorAction[], searchValue: string): EditorActionCommandEntry[] { const entries: EditorActionCommandEntry[] = []; @@ -621,4 +585,4 @@ export class CommandsHandler extends QuickOpenHandler { lastCommandPaletteInput = void 0; // clear last input when user canceled quick open } } -} \ No newline at end of file +} diff --git a/src/vs/workbench/test/browser/actionRegistry.test.ts b/src/vs/workbench/test/browser/actionRegistry.test.ts index b990f175019..97a62c5cc7f 100644 --- a/src/vs/workbench/test/browser/actionRegistry.test.ts +++ b/src/vs/workbench/test/browser/actionRegistry.test.ts @@ -6,10 +6,7 @@ 'use strict'; import * as assert from 'assert'; -import * as Platform from 'vs/platform/registry/common/platform'; -import { SyncActionDescriptor } from 'vs/platform/actions/common/actions'; import { Separator } from 'vs/base/browser/ui/actionbar/actionbar'; -import { Extensions, IWorkbenchActionRegistry } from 'vs/workbench/common/actions'; import { prepareActions } from 'vs/workbench/browser/actions'; import { Action } from 'vs/base/common/actions'; @@ -22,26 +19,6 @@ class MyClass extends Action { suite('Workbench Action Registry', () => { - test('Workbench Action Registration', function () { - let Registry = Platform.Registry.as(Extensions.WorkbenchActions); - - let d = new SyncActionDescriptor(MyClass, 'id', 'name'); - - let oldActions = Registry.getWorkbenchActions().slice(0); - let oldCount = Registry.getWorkbenchActions().length; - - Registry.registerWorkbenchAction(d, 'My Alias', 'category'); - Registry.registerWorkbenchAction(d, null); - - assert.equal(Registry.getWorkbenchActions().length, 1 + oldCount); - assert.strictEqual(d, Registry.getWorkbenchAction('id')); - - assert.deepEqual(Registry.getAlias(d.id), 'My Alias'); - assert.equal(Registry.getCategory(d.id), 'category'); - - (Registry).setWorkbenchActions(oldActions); - }); - test('Workbench Action Bar prepareActions()', function () { let a1 = new Separator(); let a2 = new Separator(); @@ -57,4 +34,4 @@ suite('Workbench Action Registry', () => { assert(actions[1] === a5); assert(actions[2] === a6); }); -}); \ No newline at end of file +}); -- GitLab