[plugin/tree] fix #4975: don't pass tree item data over JSON-RPC

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
eclipse-theia · May 30, 2019 · f2f77b4 · f2f77b4
1 parent a963e8b
commit f2f77b4
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 62 deletions.
diff --git a/packages/plugin-ext/src/api/plugin-api.ts b/packages/plugin-ext/src/api/plugin-api.ts
@@ -441,11 +441,20 @@ export class TreeViewItem {
 
     collapsibleState?: TreeViewItemCollapsibleState;
 
-    metadata?: any;
     contextValue?: string;
 
 }
 
+export interface TreeViewSelection {
+    treeViewId: string
+    treeItemId: string
+}
+export namespace TreeViewSelection {
+    export function is(arg: Object | any): arg is TreeViewSelection {
+        return !!arg && typeof arg === 'object' && 'treeViewId' in arg && 'treeItemId' in arg;
+    }
+}
+
 /**
  * Collapsible state of the tree item
  */

diff --git a/packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts b/packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts
@@ -23,16 +23,17 @@ import { MenuModelRegistry } from '@theia/core/lib/common';
 import { TabBarToolbarRegistry, TabBarToolbarItem } from '@theia/core/lib/browser/shell/tab-bar-toolbar';
 import { NAVIGATOR_CONTEXT_MENU } from '@theia/navigator/lib/browser/navigator-contribution';
 import { QuickCommandService } from '@theia/core/lib/browser/quick-open/quick-command-service';
-import { VIEW_ITEM_CONTEXT_MENU } from '../view/tree-views-main';
+import { VIEW_ITEM_CONTEXT_MENU, TreeViewWidget } from '../view/tree-views-main';
 import { PluginContribution, Menu } from '../../../common';
 import { DebugStackFramesWidget } from '@theia/debug/lib/browser/view/debug-stack-frames-widget';
 import { DebugThreadsWidget } from '@theia/debug/lib/browser/view/debug-threads-widget';
-import { MetadataSelection } from '../metadata-selection';
 import { TreeViewActions } from '../view/tree-view-actions';
 import { ScmTitleCommandRegistry } from '@theia/scm/lib/browser/scm-title-command-registry';
 import { ScmWidget } from '@theia/scm/lib/browser/scm-widget';
 import { ScmGroupCommandRegistry } from '@theia/scm/lib/browser/scm-group-command-registry';
 import { ScmResourceCommandRegistry } from '@theia/scm/lib/browser/scm-resource-command-registry';
+import { TreeWidgetSelection } from '@theia/core/lib/browser/tree/tree-widget-selection';
+
 import PATH = ScmWidget.ContextMenu.PATH;
 
 @injectable()
@@ -181,7 +182,7 @@ export class MenusContributionPointHandler {
                 group = group.substring(0, group.indexOf(' &&'));
             }
             if (action.group && action.group.startsWith('inline')) {
-                this.scmResourceCommandRegistry.registerItem(group, {command: id, group: 'inline'});
+                this.scmResourceCommandRegistry.registerItem(group, { command: id, group: 'inline' });
             } else {
                 this.menuRegistry.registerMenuAction(['scm-resource-context-menu_' + group], { commandId: id });
             }
@@ -228,9 +229,8 @@ export class MenusContributionPointHandler {
         const selectedResource = () => {
             const selection = this.selectionService.selection;
 
-            const metadata = MetadataSelection.getMetadata(selection);
-            if (metadata) {
-                return metadata;
+            if (TreeWidgetSelection.is(selection) && selection.source instanceof TreeViewWidget && selection[0]) {
+                return selection.source.toTreeViewSelection(selection[0]);
             }
 
             const uri = UriSelection.getUri(selection);

diff --git a/packages/plugin-ext/src/main/browser/metadata-selection.ts b/packages/plugin-ext/src/main/browser/metadata-selection.ts
diff --git a/packages/plugin-ext/src/main/browser/view/tree-views-main.tsx b/packages/plugin-ext/src/main/browser/view/tree-views-main.tsx
@@ -15,7 +15,7 @@
  ********************************************************************************/
 
 import { interfaces, injectable, inject, Container } from 'inversify';
-import { MAIN_RPC_CONTEXT, TreeViewsMain, TreeViewsExt } from '../../../api/plugin-api';
+import { MAIN_RPC_CONTEXT, TreeViewsMain, TreeViewsExt, TreeViewSelection } from '../../../api/plugin-api';
 import { RPCProtocol } from '../../../api/rpc-protocol';
 import { ViewRegistry } from './view-registry';
 import { Message } from '@phosphor/messaging';
@@ -147,12 +147,7 @@ export interface SelectionEventHandler {
     readonly contextSelection: boolean;
 }
 
-export interface DescriptiveMetadata {
-    // tslint:disable-next-line:no-any
-    readonly metadata?: any
-}
-
-export interface TreeViewNode extends SelectableTreeNode, DescriptiveMetadata {
+export interface TreeViewNode extends SelectableTreeNode {
     contextValue?: string
 }
 
@@ -180,7 +175,6 @@ export class TreeViewDataProviderMain {
             selected: false,
             expanded,
             children: [],
-            metadata: item.metadata,
             contextValue: item.contextValue
         };
     }
@@ -195,7 +189,6 @@ export class TreeViewDataProviderMain {
             parent: undefined,
             visible: true,
             selected: false,
-            metadata: item.metadata,
             contextValue: item.contextValue
         };
     }
@@ -360,7 +353,7 @@ export class TreeViewWidget extends TreeWidget {
         this.contextKeys.view.set(this.id);
         this.contextKeys.viewItem.set(node.contextValue);
         try {
-            const arg = node.metadata;
+            const arg = this.toTreeViewSelection(node);
             return <React.Fragment>
                 {this.actions.getInlineCommands(arg).map(command => this.renderInlineCommand(command, arg))}
             </React.Fragment>;
@@ -370,6 +363,10 @@ export class TreeViewWidget extends TreeWidget {
         }
     }
 
+    toTreeViewSelection(node: TreeNode): TreeViewSelection {
+        return { treeViewId: this.id, treeItemId: node.id };
+    }
+
     // tslint:disable-next-line:no-any
     protected renderInlineCommand(command: Command, arg: any): React.ReactNode {
         if (!command.iconClass) {

diff --git a/packages/plugin-ext/src/plugin/tree/tree-views.ts b/packages/plugin-ext/src/plugin/tree/tree-views.ts
@@ -24,7 +24,7 @@ import { Disposable, ThemeIcon } from '../types-impl';
 import { Plugin, PLUGIN_RPC_CONTEXT, TreeViewsExt, TreeViewsMain, TreeViewItem } from '../../api/plugin-api';
 import { RPCProtocol } from '../../api/rpc-protocol';
 import { CommandRegistryImpl } from '../command-registry';
-import { PluginPackage } from '../../common';
+import { PluginPackage, TreeViewSelection } from '../../common';
 import { SelectionServiceExt } from '../selection-provider-ext';
 
 export class TreeViewsExtImpl implements TreeViewsExt {
@@ -35,6 +35,16 @@ export class TreeViewsExtImpl implements TreeViewsExt {
 
     constructor(rpc: RPCProtocol, private commandRegistry: CommandRegistryImpl, private selectionService: SelectionServiceExt) {
         this.proxy = rpc.getProxy(PLUGIN_RPC_CONTEXT.TREE_VIEWS_MAIN);
+        commandRegistry.registerArgumentProcessor({
+            processArgument: arg => {
+                if (!TreeViewSelection.is(arg)) {
+                    return arg;
+                }
+                const { treeViewId, treeItemId } = arg;
+                const treeView = this.treeViews.get(treeViewId);
+                return treeView && treeView.getTreeItem(treeItemId);
+            }
+        });
     }
 
     registerTreeDataProvider<T>(plugin: Plugin, treeViewId: string, treeDataProvider: TreeDataProvider<T>): Disposable {
@@ -167,9 +177,13 @@ class TreeViewExtImpl<T> extends Disposable {
         return `item-${this.idCounter++}`;
     }
 
+    getTreeItem(treeItemId: string): T | undefined {
+        return this.cache.get(treeItemId);
+    }
+
     async getChildren(treeItemId: string): Promise<TreeViewItem[] | undefined> {
         // get element from a cache
-        const cachedElement: T | undefined = this.cache.get(treeItemId);
+        const cachedElement = this.getTreeItem(treeItemId);
 
         // ask data provider for children for cached element
         const result = await this.treeDataProvider.getChildren(cachedElement);
@@ -264,7 +278,7 @@ class TreeViewExtImpl<T> extends Disposable {
 
     async onExpanded(treeItemId: string): Promise<any> {
         // get element from a cache
-        const cachedElement: T | undefined = this.cache.get(treeItemId);
+        const cachedElement = this.getTreeItem(treeItemId);
 
         // fire an event
         if (cachedElement) {
@@ -276,7 +290,7 @@ class TreeViewExtImpl<T> extends Disposable {
 
     async onCollapsed(treeItemId: string): Promise<any> {
         // get element from a cache
-        const cachedElement: T | undefined = this.cache.get(treeItemId);
+        const cachedElement = this.getTreeItem(treeItemId);
 
         // fire an event
         if (cachedElement) {
@@ -288,7 +302,7 @@ class TreeViewExtImpl<T> extends Disposable {
 
     async onSelectionChanged(treeItemId: string, contextSelection: boolean): Promise<any> {
         // get element from a cache
-        const cachedElement: T | undefined = this.cache.get(treeItemId);
+        const cachedElement = this.getTreeItem(treeItemId);
 
         this.selectionService.selection = undefined;