diff --git a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts index 73aec9dca84..afaa490c84f 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts @@ -31,10 +31,10 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie ViewsRegistry.registerTreeViewDataProvider(treeViewId, this._register(new TreeViewDataProvider(treeViewId, this._proxy, this.messageService))); } - $refresh(treeViewId: string, treeItemHandles: string[]): void { + $refresh(treeViewId: string, itemsToRefresh: { [treeItemHandle: string]: ITreeItem }): void { const treeViewDataProvider: TreeViewDataProvider = ViewsRegistry.getTreeViewDataProvider(treeViewId); if (treeViewDataProvider) { - treeViewDataProvider.refresh(treeItemHandles); + treeViewDataProvider.refresh(itemsToRefresh); } } @@ -54,7 +54,6 @@ class TreeViewDataProvider implements ITreeViewDataProvider { private _onDispose: Emitter = new Emitter(); readonly onDispose: Event = this._onDispose.event; - private childrenMap: Map = new Map(); private itemsMap: Map = new Map(); constructor(private treeViewId: string, @@ -66,8 +65,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider { getElements(): TPromise { return this._proxy.$getElements(this.treeViewId) .then(elements => { - this.postGetElements(null, elements); - return elements; + return this.postGetElements(elements); }, err => { this.messageService.show(Severity.Error, err); return null; @@ -80,28 +78,35 @@ class TreeViewDataProvider implements ITreeViewDataProvider { } return this._proxy.$getChildren(this.treeViewId, treeItem.handle) .then(children => { - this.postGetElements(treeItem.handle, children); - return children; + return this.postGetElements(children); }, err => { this.messageService.show(Severity.Error, err); return null; }); } - refresh(treeItemHandles: string[]) { - if (treeItemHandles && treeItemHandles.length) { - let treeItems = treeItemHandles.map(treeItemHandle => this.itemsMap.get(treeItemHandle)) - .filter(treeItem => !!treeItem); - if (treeItems.length) { - this._onDidChange.fire(treeItems); - } - /*this._proxy.$resolveHandles(this.treeViewId, treeItemHandles) - .then(treeItems => { - treeItems = coalesce(treeItems.map(treeItem => this.updateTreeItem(treeItem))); - if (treeItems.length) { - this._onDidChange.fire(treeItems); + refresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }) { + if (itemsToRefreshByHandle) { + const itemsToRefresh: ITreeItem[] = []; + for (const treeItemHandle of Object.keys(itemsToRefreshByHandle)) { + const currentTreeItem = this.itemsMap.get(treeItemHandle); + if (currentTreeItem) { // Refresh only if the item exists + const treeItem = itemsToRefreshByHandle[treeItemHandle]; + // Update the current item with refreshed item + this.updateTreeItem(currentTreeItem, treeItem); + if (treeItemHandle === treeItem.handle) { + itemsToRefresh.push(currentTreeItem); + } else { + // Update maps when handle is changed and refresh parent + this.itemsMap.delete(treeItemHandle); + this.itemsMap.set(currentTreeItem.handle, currentTreeItem); + itemsToRefresh.push(this.itemsMap.get(treeItem.parentHandle)); } - }); */ + } + if (itemsToRefresh.length) { + this._onDidChange.fire(itemsToRefresh); + } + } } else { this._onDidChange.fire(); } @@ -111,33 +116,28 @@ class TreeViewDataProvider implements ITreeViewDataProvider { this._onDispose.fire(); } - private postGetElements(parent: TreeItemHandle, children: ITreeItem[]) { - this.setElements(parent, children); - } - - private setElements(parent: TreeItemHandle, children: ITreeItem[]) { - if (children && children.length) { - for (const child of children) { - if (!this.updateTreeItem(child)) { - this.itemsMap.set(child.handle, child); + private postGetElements(elements: ITreeItem[]): ITreeItem[] { + const result = []; + if (elements) { + for (const element of elements) { + const currentTreeItem = this.itemsMap.get(element.handle); + if (currentTreeItem) { + // Update the current item with new item + this.updateTreeItem(currentTreeItem, element); + } else { + this.itemsMap.set(element.handle, element); } - if (child.children && child.children.length) { - this.setElements(child.handle, child.children); - } - } - if (parent) { - this.childrenMap.set(parent, children.map(child => child.handle)); + // Always use the existing items + result.push(this.itemsMap.get(element.handle)); } } + return result; } - private updateTreeItem(treeItem: ITreeItem): ITreeItem { - const current = this.itemsMap.get(treeItem.handle); + private updateTreeItem(current: ITreeItem, treeItem: ITreeItem): void { treeItem.children = treeItem.children ? treeItem.children : null; if (current) { assign(current, treeItem); - return current; } - return null; } } \ No newline at end of file diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 23d68755c05..e29ed5ad072 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: string[]): void; + $refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void; } export interface MainThreadErrorsShape extends IDisposable { @@ -495,7 +495,6 @@ export interface ExtHostDocumentsAndEditorsShape { export interface ExtHostTreeViewsShape { $getElements(treeViewId: string): TPromise; $getChildren(treeViewId: string, treeItemHandle: string): TPromise; - // $resolveHandles(treeViewId: string, treeItemHandles: string[]): TPromise; } export interface ExtHostWorkspaceShape { diff --git a/src/vs/workbench/api/node/extHostTreeViews.ts b/src/vs/workbench/api/node/extHostTreeViews.ts index fd1ffd55c84..785f9a67adb 100644 --- a/src/vs/workbench/api/node/extHostTreeViews.ts +++ b/src/vs/workbench/api/node/extHostTreeViews.ts @@ -62,14 +62,6 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape { return treeView.getChildren(treeItemHandle); } - /* $resolveHandles(treeViewId: string, treeItemHandles: 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))); - } - return treeView.resolveHandles(treeItemHandles); - } */ - private convertArgument(arg: TreeViewItemHandleArg): any { const treeView = this.treeViews.get(arg.$treeViewId); return treeView ? treeView.getExtensionElement(arg.$treeItemHandle) : null; @@ -77,6 +69,7 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape { } interface TreeNode { + index: number; handle: TreeItemHandle; parent: TreeItemHandle; children: TreeItemHandle[]; @@ -84,7 +77,8 @@ interface TreeNode { class ExtHostTreeView extends Disposable { - private extElementsMap: Map = new Map(); + private static ROOT_HANDLE = '0'; + private elements: Map = new Map(); private nodes: Map = new Map(); constructor(private viewId: string, private dataProvider: vscode.TreeDataProvider, private proxy: MainThreadTreeViewsShape, private commands: CommandsConverter) { @@ -98,7 +92,7 @@ class ExtHostTreeView extends Disposable { getTreeItems(): TPromise { this.clearAll(); return asWinJsPromise(() => this.dataProvider.getChildren()) - .then(elements => this.resolveElements(elements, '0')); + .then(elements => this.resolveElements(elements)); } getChildren(treeItemHandle: TreeItemHandle): TPromise { @@ -117,34 +111,23 @@ class ExtHostTreeView extends Disposable { }); } - /* resolveHandles(treeItemHandles: TreeItemHandle[]): TPromise { - return TPromise.join(treeItemHandles.map(treeItemHandle => { - let extElement = this.getExtensionElement(treeItemHandle); - if (!extElement) { - return TPromise.wrapError(new Error(localize('treeItem.notFound', 'No tree item with id \'{0}\' found.', treeItemHandle))); - } - const node = this.nodes.get(extElement); - return this.resolveElement(extElement, node.handle); - })).then(treeItems => coalesce(treeItems)); - } */ - getExtensionElement(treeItemHandle: TreeItemHandle): T { - return this.extElementsMap.get(treeItemHandle); + return this.elements.get(treeItemHandle); } private _refresh(elements: T[]): void { const hasRoot = elements.some(element => !element); if (hasRoot) { - this.proxy.$refresh(this.viewId, []); + this.proxy.$refresh(this.viewId); } else { - const handlesToUpdate = this.getElementsToUpdate(elements); + const handlesToUpdate = this.getHandlesToUpdate(elements); if (handlesToUpdate.length) { - this.proxy.$refresh(this.viewId, handlesToUpdate); + this._refreshHandles(handlesToUpdate); } } } - private resolveElements(elements: T[], parentHandle: TreeItemHandle): TPromise { + private resolveElements(elements: T[], parentHandle?: TreeItemHandle): TPromise { if (elements && elements.length) { return TPromise.join( elements.filter(element => !!element) @@ -153,11 +136,12 @@ class ExtHostTreeView extends Disposable { .then(treeItem => { if (treeItem) { this.nodes.set(element, { + index, handle: treeItem.handle, parent: parentHandle, children: void 0 }); - this.extElementsMap.set(treeItem.handle, element); + this.elements.set(treeItem.handle, element); } return treeItem; }); @@ -167,7 +151,7 @@ class ExtHostTreeView extends Disposable { return TPromise.as([]); } - private resolveElement(element: T, index: number, parentHandle: TreeItemHandle): TPromise { + private resolveElement(element: T, index: number, parentHandle?: TreeItemHandle): TPromise { return asWinJsPromise(() => this.dataProvider.getTreeItem(element)) .then(extTreeItem => this.massageTreeItem(extTreeItem, index, parentHandle)); } @@ -178,7 +162,8 @@ class ExtHostTreeView extends Disposable { } const icon = this.getLightIconPath(extensionTreeItem); return { - handle: `${parentHandle}/${index}:${extensionTreeItem.label}`, + handle: `${parentHandle ? parentHandle : ExtHostTreeView.ROOT_HANDLE}/${index}:${extensionTreeItem.label}`, + parentHandle, label: extensionTreeItem.label, command: extensionTreeItem.command ? this.commands.toInternal(extensionTreeItem.command) : void 0, contextValue: extensionTreeItem.contextValue, @@ -212,7 +197,7 @@ class ExtHostTreeView extends Disposable { return URI.file(iconPath).toString(); } - private getElementsToUpdate(elements: T[]): TreeItemHandle[] { + private getHandlesToUpdate(elements: T[]): TreeItemHandle[] { const elementsToUpdate = new Set(); for (const element of elements) { let elementNode = this.nodes.get(element); @@ -220,10 +205,10 @@ class ExtHostTreeView extends Disposable { // check if an ancestor of extElement is already in the elements to update list let currentNode = elementNode; while (currentNode && currentNode.parent && !elementsToUpdate.has(currentNode.parent)) { - const parentElement = this.extElementsMap.get(currentNode.parent); + const parentElement = this.elements.get(currentNode.parent); currentNode = this.nodes.get(parentElement); } - if (!currentNode) { + if (!currentNode.parent) { elementsToUpdate.add(elementNode.handle); } } @@ -232,7 +217,7 @@ class ExtHostTreeView extends Disposable { const handlesToUpdate: TreeItemHandle[] = []; // Take only top level elements elementsToUpdate.forEach((handle) => { - const element = this.extElementsMap.get(handle); + const element = this.elements.get(handle); let node = this.nodes.get(element); if (node && !elementsToUpdate.has(node.parent)) { handlesToUpdate.push(handle); @@ -242,11 +227,49 @@ class ExtHostTreeView extends Disposable { return handlesToUpdate; } + private _refreshHandles(itemHandles: TreeItemHandle[]): TPromise { + const itemsToRefresh: { [handle: string]: ITreeItem } = {}; + const promises: TPromise[] = []; + itemHandles.forEach(treeItemHandle => { + const extElement = this.getExtensionElement(treeItemHandle); + const node = this.nodes.get(extElement); + const promise = this.resolveElement(extElement, node.index, node.parent) + .then(treeItem => { + if (treeItemHandle !== treeItem.handle) { + // Update caches if handle changes + this.updateCaches(node, treeItem, extElement); + } + itemsToRefresh[treeItemHandle] = treeItem; + }); + promises.push(promise); + }); + return TPromise.join(promises) + .then(treeItems => { + this.proxy.$refresh(this.viewId, itemsToRefresh); + }); + } + + private updateCaches(node: TreeNode, treeItem: ITreeItem, element: T): void { + if (node.parent) { + // Update parent's children handles + const parentElement = this.getExtensionElement(node.parent); + const parentNode = this.nodes.get(parentElement); + parentNode.children[node.index] = treeItem.handle; + } + + // Update elements map + this.elements.delete(node.handle); + this.elements.set(treeItem.handle, element); + + // Update node + node.handle = treeItem.handle; + } + private clearChildren(element: T): void { let node = this.nodes.get(element); if (node.children) { for (const childHandle of node.children) { - const childEleement = this.extElementsMap.get(childHandle); + const childEleement = this.elements.get(childHandle); if (childEleement) { this.clear(childEleement); } @@ -259,18 +282,18 @@ class ExtHostTreeView extends Disposable { let node = this.nodes.get(element); if (node.children) { for (const childHandle of node.children) { - const childEleement = this.extElementsMap.get(childHandle); + const childEleement = this.elements.get(childHandle); if (childEleement) { this.clear(childEleement); } } } this.nodes.delete(element); - this.extElementsMap.delete(node.handle); + this.elements.delete(node.handle); } private clearAll(): void { - this.extElementsMap.clear(); + this.elements.clear(); this.nodes.clear(); } diff --git a/src/vs/workbench/browser/parts/views/treeView.ts b/src/vs/workbench/browser/parts/views/treeView.ts index d4eee28fead..c5b67f40ad1 100644 --- a/src/vs/workbench/browser/parts/views/treeView.ts +++ b/src/vs/workbench/browser/parts/views/treeView.ts @@ -173,10 +173,14 @@ export class TreeView extends TreeViewsViewletPanel { } private refresh(elements: ITreeItem[]): void { - elements = elements ? elements : [this.tree.getInput()]; - for (const element of elements) { - element.children = null; - this.tree.refresh(element); + if (elements) { + for (const element of elements) { + this.tree.refresh(element); + } + } else { + const root: ITreeItem = this.tree.getInput(); + root.children = null; // reset children + this.tree.refresh(root); } } @@ -193,6 +197,7 @@ export class TreeView extends TreeViewsViewletPanel { class Root implements ITreeItem { label = 'root'; handle = '0'; + parentHandle = null; collapsibleState = TreeItemCollapsibleState.Expanded; } diff --git a/src/vs/workbench/common/views.ts b/src/vs/workbench/common/views.ts index 8435ceb6953..00b5ddbc939 100644 --- a/src/vs/workbench/common/views.ts +++ b/src/vs/workbench/common/views.ts @@ -22,6 +22,8 @@ export interface ITreeItem { handle: string; + parentHandle: string; + label: string; icon?: 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 815a5832edb..317c058be4c 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts @@ -19,38 +19,43 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti 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'; +import { TreeItemCollapsibleState, ITreeItem } from 'vs/workbench/common/views'; suite('ExtHostTreeView', function () { class RecordingShape extends mock() { - onRefresh = new Emitter(); + onRefresh = new Emitter<{ [treeItemHandle: string]: ITreeItem }>(); $registerView(treeViewId: string): void { } - $refresh(viewId: string, itemHandles: string[]): void { - this.onRefresh.fire(itemHandles); + $refresh(viewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void { + this.onRefresh.fire(itemsToRefresh); } } let testObject: ExtHostTreeViews; let target: RecordingShape; let onDidChangeTreeData: Emitter; - let tree = { - 'a': { - 'aa': {}, - 'ab': {} - }, - 'b': { - 'ba': {}, - 'bb': {} - } - }; + let tree; + let labels; setup(() => { + tree = { + 'a': { + 'aa': {}, + 'ab': {} + }, + 'b': { + 'ba': {}, + 'bb': {} + } + }; + + labels = {}; + let threadService = new TestThreadService(); // Use IInstantiationService to get typechecking when instantiating let inst: IInstantiationService; @@ -102,23 +107,34 @@ suite('ExtHostTreeView', function () { test('refresh root', function (done) { target.onRefresh.event(actuals => { - assert.equal(0, actuals.length); + assert.equal(undefined, actuals); done(); }); onDidChangeTreeData.fire(); }); - test('refresh a parent node', function (done) { - target.onRefresh.event(actuals => { - assert.deepEqual(['0/1:b'], actuals); - done(); + test('refresh a parent node', () => { + return new TPromise((c, e) => { + target.onRefresh.event(actuals => { + assert.deepEqual(['0/1:b'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/1:b']), { + handle: '0/1:b', + label: 'b', + }); + c(null); + }); + onDidChangeTreeData.fire('b'); }); - onDidChangeTreeData.fire('b'); }); test('refresh a leaf node', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual(['0/1:b/1:bb'], actuals); + assert.deepEqual(['0/1:b/1:bb'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/1:b/1:bb']), { + handle: '0/1:b/1:bb', + parentHandle: '0/1:b', + label: 'bb' + }); done(); }); onDidChangeTreeData.fire('bb'); @@ -126,7 +142,16 @@ suite('ExtHostTreeView', function () { test('refresh parent and child node trigger refresh only on parent - scenario 1', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual(['0/1:b', '0/0:a/0:aa'], actuals); + assert.deepEqual(['0/1:b', '0/0:a/0:aa'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/1:b']), { + handle: '0/1:b', + label: 'b', + }); + assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), { + handle: '0/0:a/0:aa', + parentHandle: '0/0:a', + label: 'aa', + }); done(); }); onDidChangeTreeData.fire('b'); @@ -136,7 +161,16 @@ suite('ExtHostTreeView', function () { test('refresh parent and child node trigger refresh only on parent - scenario 2', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual(['0/0:a/0:aa', '0/1:b'], actuals); + assert.deepEqual(['0/0:a/0:aa', '0/1:b'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/1:b']), { + handle: '0/1:b', + label: 'b', + }); + assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), { + handle: '0/0:a/0:aa', + parentHandle: '0/0:a', + label: 'aa', + }); done(); }); onDidChangeTreeData.fire('bb'); @@ -144,9 +178,22 @@ suite('ExtHostTreeView', function () { onDidChangeTreeData.fire('b'); }); + test('refresh an element for label change', function (done) { + labels['a'] = 'aa'; + target.onRefresh.event(actuals => { + assert.deepEqual(['0/0:a'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/0:a']), { + handle: '0/0:aa', + label: 'aa', + }); + done(); + }); + onDidChangeTreeData.fire('a'); + }); + test('refresh calls are throttled on roots', function (done) { target.onRefresh.event(actuals => { - assert.equal(0, actuals.length); + assert.equal(undefined, actuals); done(); }); onDidChangeTreeData.fire(); @@ -157,7 +204,7 @@ suite('ExtHostTreeView', function () { test('refresh calls are throttled on elements', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual(['0/0:a', '0/1:b'], actuals); + assert.deepEqual(['0/0:a', '0/1:b'], Object.keys(actuals)); done(); }); @@ -169,7 +216,7 @@ suite('ExtHostTreeView', function () { test('refresh calls are throttled on unknown elements', function (done) { target.onRefresh.event(actuals => { - assert.deepEqual(['0/0:a', '0/1:b'], actuals); + assert.deepEqual(['0/0:a', '0/1:b'], Object.keys(actuals)); done(); }); @@ -181,7 +228,7 @@ suite('ExtHostTreeView', function () { test('refresh calls are throttled on unknown elements and root', function (done) { target.onRefresh.event(actuals => { - assert.equal(0, actuals.length); + assert.equal(undefined, actuals); done(); }); @@ -193,7 +240,7 @@ suite('ExtHostTreeView', function () { test('refresh calls are throttled on elements and root', function (done) { target.onRefresh.event(actuals => { - assert.equal(0, actuals.length); + assert.equal(undefined, actuals); done(); }); @@ -203,6 +250,16 @@ suite('ExtHostTreeView', function () { onDidChangeTreeData.fire('a'); }); + function removeUnsetKeys(obj: any): any { + const result = {}; + for (const key of Object.keys(obj)) { + if (obj[key] !== void 0) { + result[key] = obj[key]; + } + } + return result; + } + function aTreeDataProvider(): TreeDataProvider { const getTreeElement = (element) => { let parent = tree; @@ -233,7 +290,7 @@ suite('ExtHostTreeView', function () { getTreeItem: (element: string): TreeItem => { const treeElement = getTreeElement(element); return { - label: element, + label: labels[element] || element, collapsibleState: treeElement ? treeElement['collapsibleState'] : TreeItemCollapsibleState.Collapsed }; },