From cb926d58e4c12223f2e4ccb4751787c8326bc9f6 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Fri, 1 Dec 2017 02:11:46 +0100 Subject: [PATCH] Tree views: Use paths as ids --- .../electron-browser/mainThreadTreeViews.ts | 6 +- src/vs/workbench/api/node/extHost.protocol.ts | 4 +- src/vs/workbench/api/node/extHostTreeViews.ts | 24 ++-- .../workbench/browser/parts/views/treeView.ts | 4 +- src/vs/workbench/common/views.ts | 4 +- .../api/extHostTreeViews.test.ts | 128 ++++++++++++++++-- 6 files changed, 136 insertions(+), 34 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts index aa70391c538..d055e61652b 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts @@ -30,7 +30,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie ViewsRegistry.registerTreeViewDataProvider(treeViewId, this._register(new TreeViewDataProvider(treeViewId, this._proxy, this.messageService))); } - $refresh(treeViewId: string, treeItemHandles: number[]): void { + $refresh(treeViewId: string, treeItemHandles: string[]): void { const treeViewDataProvider: TreeViewDataProvider = ViewsRegistry.getTreeViewDataProvider(treeViewId); if (treeViewDataProvider) { treeViewDataProvider.refresh(treeItemHandles); @@ -43,7 +43,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie } } -type TreeItemHandle = number; +type TreeItemHandle = string; class TreeViewDataProvider implements ITreeViewDataProvider { @@ -87,7 +87,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider { }); } - refresh(treeItemHandles: number[]) { + refresh(treeItemHandles: string[]) { if (treeItemHandles && treeItemHandles.length) { let treeItems = treeItemHandles.map(treeItemHandle => this.itemsMap.get(treeItemHandle)) .filter(treeItem => !!treeItem); diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 0e352960a6c..23849bca3b1 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -238,7 +238,7 @@ export interface MainThreadEditorsShape extends IDisposable { export interface MainThreadTreeViewsShape extends IDisposable { $registerView(treeViewId: string): void; - $refresh(treeViewId: string, treeItemHandles: number[]): void; + $refresh(treeViewId: string, treeItemHandles: string[]): void; } export interface MainThreadErrorsShape extends IDisposable { @@ -493,7 +493,7 @@ export interface ExtHostDocumentsAndEditorsShape { export interface ExtHostTreeViewsShape { $getElements(treeViewId: string): TPromise; - $getChildren(treeViewId: string, treeItemHandle: number): TPromise; + $getChildren(treeViewId: string, treeItemHandle: string): TPromise; } export interface ExtHostWorkspaceShape { diff --git a/src/vs/workbench/api/node/extHostTreeViews.ts b/src/vs/workbench/api/node/extHostTreeViews.ts index 71789d1563c..9a169b7a524 100644 --- a/src/vs/workbench/api/node/extHostTreeViews.ts +++ b/src/vs/workbench/api/node/extHostTreeViews.ts @@ -17,7 +17,7 @@ import { TreeItemCollapsibleState } from './extHostTypes'; import { ExtHostCommands, CommandsConverter } from 'vs/workbench/api/node/extHostCommands'; import { asWinJsPromise } from 'vs/base/common/async'; -type TreeItemHandle = number; +type TreeItemHandle = string; export class ExtHostTreeViews implements ExtHostTreeViewsShape { @@ -56,7 +56,7 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape { return treeView.getTreeItems(); } - $getChildren(treeViewId: string, treeItemHandle?: number): TPromise { + $getChildren(treeViewId: string, treeItemHandle?: string): TPromise { const treeView = this.treeViews.get(treeViewId); if (!treeView) { return TPromise.wrapError(new Error(localize('treeView.notRegistered', 'No tree view with id \'{0}\' registered.', treeViewId))); @@ -72,8 +72,6 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape { class ExtHostTreeView extends Disposable { - private _itemHandlePool = 0; - private extElementsMap: Map = new Map(); private itemHandlesMap: Map = new Map(); private extChildrenElementsMap: Map = new Map(); @@ -92,7 +90,7 @@ class ExtHostTreeView extends Disposable { this.itemHandlesMap.clear(); return asWinJsPromise(() => this.dataProvider.getChildren()) - .then(elements => this.processAndMapElements(elements)); + .then(elements => this.processAndMapElements(elements, '0')); } getChildren(treeItemHandle: TreeItemHandle): TPromise { @@ -104,7 +102,7 @@ class ExtHostTreeView extends Disposable { } return asWinJsPromise(() => this.dataProvider.getChildren(extElement)) - .then(childrenElements => this.processAndMapElements(childrenElements)); + .then(childrenElements => this.processAndMapElements(childrenElements, treeItemHandle)); } getExtensionElement(treeItemHandle: TreeItemHandle): T { @@ -124,25 +122,25 @@ class ExtHostTreeView extends Disposable { } } - private processAndMapElements(elements: T[]): TPromise { + private processAndMapElements(elements: T[], parentHandle: TreeItemHandle): TPromise { if (elements && elements.length) { return TPromise.join( elements.filter(element => !!element) - .map(element => { + .map((element, index) => { if (this.extChildrenElementsMap.has(element)) { return TPromise.wrapError(new Error(localize('treeView.duplicateElement', 'Element {0} is already registered', element))); } - return this.resolveElement(element); + return this.resolveElement(element, index, parentHandle); })) .then(treeItems => treeItems.filter(treeItem => !!treeItem)); } return TPromise.as([]); } - private resolveElement(element: T): TPromise { + private resolveElement(element: T, index: number, parentHandle: TreeItemHandle): TPromise { return asWinJsPromise(() => this.dataProvider.getTreeItem(element)) .then(extTreeItem => { - const treeItem = this.massageTreeItem(extTreeItem); + const treeItem = this.massageTreeItem(extTreeItem, index, parentHandle); if (treeItem) { this.itemHandlesMap.set(element, treeItem.handle); this.extElementsMap.set(treeItem.handle, element); @@ -159,13 +157,13 @@ class ExtHostTreeView extends Disposable { }); } - private massageTreeItem(extensionTreeItem: vscode.TreeItem): ITreeItem { + private massageTreeItem(extensionTreeItem: vscode.TreeItem, index: number, parentHandle: TreeItemHandle): ITreeItem { if (!extensionTreeItem) { return null; } const icon = this.getLightIconPath(extensionTreeItem); return { - handle: ++this._itemHandlePool, + handle: `${parentHandle}/${index}:${extensionTreeItem.label}`, label: extensionTreeItem.label, command: extensionTreeItem.command ? this.commands.toInternal(extensionTreeItem.command) : void 0, contextValue: extensionTreeItem.contextValue, diff --git a/src/vs/workbench/browser/parts/views/treeView.ts b/src/vs/workbench/browser/parts/views/treeView.ts index 829eb3fa8ed..d4eee28fead 100644 --- a/src/vs/workbench/browser/parts/views/treeView.ts +++ b/src/vs/workbench/browser/parts/views/treeView.ts @@ -192,7 +192,7 @@ export class TreeView extends TreeViewsViewletPanel { class Root implements ITreeItem { label = 'root'; - handle = -1; + handle = '0'; collapsibleState = TreeItemCollapsibleState.Expanded; } @@ -205,7 +205,7 @@ class TreeDataSource implements IDataSource { } public getId(tree: ITree, node: ITreeItem): string { - return '' + node.handle; + return node.handle; } public hasChildren(tree: ITree, node: ITreeItem): boolean { diff --git a/src/vs/workbench/common/views.ts b/src/vs/workbench/common/views.ts index 599a1cfdde4..8435ceb6953 100644 --- a/src/vs/workbench/common/views.ts +++ b/src/vs/workbench/common/views.ts @@ -9,7 +9,7 @@ import { Command } from 'vs/editor/common/modes'; export type TreeViewItemHandleArg = { $treeViewId: string, - $treeItemHandle: number + $treeItemHandle: string }; export enum TreeItemCollapsibleState { @@ -20,7 +20,7 @@ export enum TreeItemCollapsibleState { export interface ITreeItem { - handle: number; + handle: string; label: string; diff --git a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts index 85c50190733..5e06e83c94d 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts @@ -18,18 +18,20 @@ import { MainThreadCommands } from 'vs/workbench/api/electron-browser/mainThread import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { mock } from 'vs/workbench/test/electron-browser/api/mock'; import { NoopLogService } from 'vs/platform/log/common/log'; +import { TPromise } from 'vs/base/common/winjs.base'; +import { TreeItemCollapsibleState } from 'vs/workbench/common/views'; -suite('ExtHostConfiguration', function () { +suite('ExtHostTreeView', function () { class RecordingShape extends mock() { - onRefresh = new Emitter(); + onRefresh = new Emitter(); $registerView(treeViewId: string): void { } - $refresh(viewId: string, itemHandles: number[]): void { + $refresh(viewId: string, itemHandles: string[]): void { this.onRefresh.fire(itemHandles); } } @@ -37,6 +39,16 @@ suite('ExtHostConfiguration', function () { let testObject: ExtHostTreeViews; let target: RecordingShape; let onDidChangeTreeData: Emitter; + let tree = { + 'a': { + 'aa': {}, + 'ab': {} + }, + 'b': { + 'ba': {}, + 'bb': {} + } + }; setup(() => { let threadService = new TestThreadService(); @@ -60,6 +72,83 @@ suite('ExtHostConfiguration', function () { }); }); + test('construct tree', () => { + return testObject.$getElements('testDataProvider') + .then(elements => { + const actuals = elements.map(e => e.handle); + assert.deepEqual(actuals, ['0/0:a', '0/1:b']); + return TPromise.join([ + testObject.$getChildren('testDataProvider', '0/0:a') + .then(children => { + const actuals = children.map(e => e.handle); + assert.deepEqual(actuals, ['0/0:a/0:aa', '0/0:a/1:ab']); + return TPromise.join([ + testObject.$getChildren('testDataProvider', '0/0:a/0:aa').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testDataProvider', '0/0:a/1:ab').then(children => assert.equal(children.length, 0)) + ]); + }), + testObject.$getChildren('testDataProvider', '0/1:b') + .then(children => { + const actuals = children.map(e => e.handle); + assert.deepEqual(actuals, ['0/1:b/0:ba', '0/1:b/1:bb']); + return TPromise.join([ + testObject.$getChildren('testDataProvider', '0/1:b/0:ba').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testDataProvider', '0/1:b/1:bb').then(children => assert.equal(children.length, 0)) + ]); + }) + ]); + }); + }); + + test('children are fetched immediately if state is expanded', () => { + tree['c'] = { + 'ca': { + 'caa': { + 'collapsibleState': TreeItemCollapsibleState.None + }, + 'collapsibleState': TreeItemCollapsibleState.Expanded, + }, + 'cb': { + 'cba': {}, + 'collapsibleState': TreeItemCollapsibleState.Collapsed, + }, + 'collapsibleState': TreeItemCollapsibleState.Expanded, + }; + return testObject.$getElements('testDataProvider') + .then(elements => { + const actuals = elements.map(e => e.handle); + assert.deepEqual(actuals, ['0/0:a', '0/1:b', '0/2:c']); + assert.deepEqual(elements[2].children.map(e => e.handle), ['0/2:c/0:ca', '0/2:c/1:cb']); + assert.deepEqual(elements[2].children[0].children.map(e => e.handle), ['0/2:c/0:ca/0:caa']); + assert.deepEqual(elements[2].children[0].children[0].children, undefined); + assert.deepEqual(elements[2].children[1].children, undefined); + }); + }); + + test('refresh root', function (done) { + target.onRefresh.event(actuals => { + assert.equal(0, actuals.length); + done(); + }); + onDidChangeTreeData.fire(); + }); + + test('refresh a parent node', function (done) { + target.onRefresh.event(actuals => { + assert.deepEqual(['0/1:b'], actuals); + done(); + }); + onDidChangeTreeData.fire('b'); + }); + + test('refresh a leaf node', function (done) { + target.onRefresh.event(actuals => { + assert.deepEqual(['0/1:b/1:bb'], actuals); + done(); + }); + onDidChangeTreeData.fire('bb'); + }); + test('refresh calls are throttled on roots', function (done) { target.onRefresh.event(actuals => { assert.equal(0, actuals.length); @@ -73,7 +162,7 @@ suite('ExtHostConfiguration', function () { test('refresh calls are throttled on elements', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual([1, 2], actuals); + assert.deepEqual(['0/0:a', '0/1:b'], actuals); done(); }); @@ -85,7 +174,7 @@ suite('ExtHostConfiguration', function () { test('refresh calls are throttled on unknown elements', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual([1, 2], actuals); + assert.deepEqual(['0/0:a', '0/1:b'], actuals); done(); }); @@ -120,22 +209,37 @@ suite('ExtHostConfiguration', function () { }); function aTreeDataProvider(): TreeDataProvider { + const getTreeElement = (element) => { + let parent = tree; + for (let i = 0; i < element.length; i++) { + parent = parent[element.substring(0, i + 1)]; + if (!parent) { + return null; + } + } + return parent; + }; return { getChildren: (element: string): string[] => { if (!element) { - return ['a', 'b']; - } - if (element === 'a') { - return ['aa', 'ab']; + return Object.keys(tree); } - if (element === 'b') { - return ['ba', 'bb']; + let treeElement = getTreeElement(element); + if (treeElement) { + const children = Object.keys(treeElement); + const collapsibleStateIndex = children.indexOf('collapsibleState'); + if (collapsibleStateIndex !== -1) { + children.splice(collapsibleStateIndex, 1); + } + return children; } return []; }, getTreeItem: (element: string): TreeItem => { + const treeElement = getTreeElement(element); return { - label: element + label: element, + collapsibleState: treeElement ? treeElement['collapsibleState'] : TreeItemCollapsibleState.Collapsed }; }, onDidChangeTreeData: onDidChangeTreeData.event -- GitLab