diff --git a/src/vs/workbench/api/node/extHostTreeViews.ts b/src/vs/workbench/api/node/extHostTreeViews.ts index 5ebf180db38c1c6296afee0847a48931ddf2e855..a96594f0e3a8753c1273c78c1b1f67be2f9da355 100644 --- a/src/vs/workbench/api/node/extHostTreeViews.ts +++ b/src/vs/workbench/api/node/extHostTreeViews.ts @@ -77,7 +77,9 @@ interface TreeNode { class ExtHostTreeView extends Disposable { - private static ROOT_HANDLE = '0'; + private static LABEL_HANDLE_PREFIX = '0'; + + private rootHandles: TreeItemHandle[] = []; private elements: Map = new Map(); private nodes: Map = new Map(); @@ -85,49 +87,36 @@ class ExtHostTreeView extends Disposable { super(); this.proxy.$registerView(viewId); if (dataProvider.onDidChangeTreeData) { - this._register(debounceEvent(dataProvider.onDidChangeTreeData, (last, current) => last ? [...last, current] : [current], 200)(elements => this._refresh(elements))); + this._register(debounceEvent(dataProvider.onDidChangeTreeData, (last, current) => last ? [...last, current] : [current], 200)(elements => this.refresh(elements))); } } getChildren(parentHandle?: TreeItemHandle): TPromise { - let parentElement; - if (parentHandle) { - parentElement = this.getExtensionElement(parentHandle); - if (!parentElement) { - return TPromise.wrapError(new Error(localize('treeItem.notFound', 'No tree item with id \'{0}\' found.', parentHandle))); - } + const parentElement = parentHandle ? this.getExtensionElement(parentHandle) : void 0; + if (parentHandle && !parentElement) { + console.error(`No tree item with id \'${parentHandle}\' found.`); + return TPromise.as([]); } + this.clearChildren(parentElement); return asWinJsPromise(() => this.dataProvider.getChildren(parentElement)) - .then(elements => { - elements = coalesce(elements || []); - return TPromise.join(elements.map((element, index) => { - const node = this.nodes.get(element); - const currentHandle = node && node.parentHandle === parentHandle ? node.handle : void 0; - return this.resolveElement(element, currentHandle ? currentHandle : index, parentHandle) - .then(treeItem => { - if (treeItem) { - if (!currentHandle) { - // update the caches if current handle is not used - this.nodes.set(element, { - handle: treeItem.handle, - parentHandle, - childrenHandles: void 0 - }); - this.elements.set(treeItem.handle, element); - } + .then(elements => TPromise.join( + coalesce(elements || []).map(element => + asWinJsPromise(() => this.dataProvider.getTreeItem(element)) + .then(extTreeItem => { + if (extTreeItem) { + return { element, extTreeItem }; } - return treeItem; - }); - })).then(treeItems => this.updateChildren(coalesce(treeItems), parentElement)); - }); + return null; + }) + ))).then(extTreeItems => extTreeItems.map((({ element, extTreeItem }) => this.createTreeItem(element, extTreeItem, parentHandle)))); } getExtensionElement(treeItemHandle: TreeItemHandle): T { return this.elements.get(treeItemHandle); } - private _refresh(elements: T[]): void { + private refresh(elements: T[]): void { const hasRoot = elements.some(element => !element); if (hasRoot) { this.proxy.$refresh(this.viewId); @@ -139,26 +128,63 @@ class ExtHostTreeView extends Disposable { } } - private resolveElement(element: T, handleOrIndex: TreeItemHandle | number, parentHandle: TreeItemHandle): TPromise { - return asWinJsPromise(() => this.dataProvider.getTreeItem(element)) - .then(extTreeItem => this.massageTreeItem(element, extTreeItem, handleOrIndex, parentHandle)); + private getHandlesToRefresh(elements: T[]): TreeItemHandle[] { + const elementsToUpdate = new Set(); + for (const element of elements) { + let elementNode = this.nodes.get(element); + if (elementNode && !elementsToUpdate.has(elementNode.handle)) { + // check if an ancestor of extElement is already in the elements to update list + let currentNode = elementNode; + while (currentNode && currentNode.parentHandle && !elementsToUpdate.has(currentNode.parentHandle)) { + const parentElement = this.elements.get(currentNode.parentHandle); + currentNode = this.nodes.get(parentElement); + } + if (!currentNode.parentHandle) { + elementsToUpdate.add(elementNode.handle); + } + } + } + + const handlesToUpdate: TreeItemHandle[] = []; + // Take only top level elements + elementsToUpdate.forEach((handle) => { + const element = this.elements.get(handle); + let node = this.nodes.get(element); + if (node && !elementsToUpdate.has(node.parentHandle)) { + handlesToUpdate.push(handle); + } + }); + + 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); + promises.push(asWinJsPromise(() => this.dataProvider.getTreeItem(extElement)) + .then(extTreeItem => { + if (extTreeItem) { + itemsToRefresh[treeItemHandle] = this.createTreeItem(extElement, extTreeItem, node.parentHandle); + } + })); + }); + return TPromise.join(promises) + .then(treeItems => this.proxy.$refresh(this.viewId, itemsToRefresh)); } - private massageTreeItem(element: T, extensionTreeItem: vscode.TreeItem, handleOrIndex: TreeItemHandle | number, parentHandle: TreeItemHandle): ITreeItem { - if (!extensionTreeItem) { - return null; - } + private createTreeItem(element: T, extensionTreeItem: vscode.TreeItem, parentHandle: TreeItemHandle): ITreeItem { + const handle = this.createHandle(element, extensionTreeItem, parentHandle); const icon = this.getLightIconPath(extensionTreeItem); - const label = extensionTreeItem.label; - const handle = typeof handleOrIndex === 'number' ? - this.generateHandle(label, handleOrIndex, parentHandle) // create the handle - : handleOrIndex; // reuse the passed handle + this.update(element, handle, parentHandle); return { handle, parentHandle, - label, + label: extensionTreeItem.label, command: extensionTreeItem.command ? this.commands.toInternal(extensionTreeItem.command) : void 0, contextValue: extensionTreeItem.contextValue, icon, @@ -167,10 +193,19 @@ class ExtHostTreeView extends Disposable { }; } - private generateHandle(label: string, index: number, parentHandle: TreeItemHandle): TreeItemHandle { - parentHandle = parentHandle ? parentHandle : ExtHostTreeView.ROOT_HANDLE; + private createHandle(element: T, { label }: vscode.TreeItem, parentHandle?: TreeItemHandle): TreeItemHandle { + const prefix = parentHandle ? parentHandle : ExtHostTreeView.LABEL_HANDLE_PREFIX; label = label.indexOf('/') !== -1 ? label.replace('/', '//') : label; - return `${parentHandle}/${index}:${label}`; + const existingHandle = this.nodes.has(element) ? this.nodes.get(element).handle : void 0; + + for (let labelCount = 0; labelCount <= this.getChildrenHandles(parentHandle).length; labelCount++) { + const handle = `${prefix}/${labelCount}:${label}`; + if (!this.elements.has(handle) || existingHandle === handle) { + return handle; + } + } + + throw new Error('This should not be reached'); } private getLightIconPath(extensionTreeItem: vscode.TreeItem): string { @@ -197,75 +232,48 @@ class ExtHostTreeView extends Disposable { return URI.file(iconPath).toString(); } - private getHandlesToRefresh(elements: T[]): TreeItemHandle[] { - const elementsToUpdate = new Set(); - for (const element of elements) { - let elementNode = this.nodes.get(element); - if (elementNode && !elementsToUpdate.has(elementNode.handle)) { - // check if an ancestor of extElement is already in the elements to update list - let currentNode = elementNode; - while (currentNode && currentNode.parentHandle && !elementsToUpdate.has(currentNode.parentHandle)) { - const parentElement = this.elements.get(currentNode.parentHandle); - currentNode = this.nodes.get(parentElement); - } - if (!currentNode.parentHandle) { - elementsToUpdate.add(elementNode.handle); - } - } - } + private getChildrenHandles(parentHandle?: TreeItemHandle): TreeItemHandle[] { + return parentHandle ? this.nodes.get(this.getExtensionElement(parentHandle)).childrenHandles : this.rootHandles; + } - const handlesToUpdate: TreeItemHandle[] = []; - // Take only top level elements - elementsToUpdate.forEach((handle) => { - const element = this.elements.get(handle); - let node = this.nodes.get(element); - if (node && !elementsToUpdate.has(node.parentHandle)) { - handlesToUpdate.push(handle); - } - }); + private update(element: T, handle: TreeItemHandle, parentHandle: TreeItemHandle): void { + const node = this.nodes.get(element); + const childrenHandles = this.getChildrenHandles(parentHandle); - return handlesToUpdate; - } + // Update parent node + if (node) { + if (node.handle !== handle) { + childrenHandles[childrenHandles.indexOf(node.handle)] = handle; + this.clearChildren(element); + } + } else { + childrenHandles.push(handle); + } - 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); - promises.push(this.resolveElement(extElement, treeItemHandle, node.parentHandle) - .then(treeItem => { - itemsToRefresh[treeItemHandle] = treeItem; - })); + // Update element maps + this.elements.set(handle, element); + this.nodes.set(element, { + handle, + parentHandle, + childrenHandles: node ? node.childrenHandles : [] }); - return TPromise.join(promises) - .then(treeItems => { - this.proxy.$refresh(this.viewId, itemsToRefresh); - }); } - private updateChildren(newChildren: ITreeItem[], parentElement?: T): ITreeItem[] { - let existingChildrenHandles: TreeItemHandle[] = []; + private clearChildren(parentElement?: T): void { if (parentElement) { - const parentNode = this.nodes.get(parentElement); - existingChildrenHandles = parentNode.childrenHandles || []; - parentNode.childrenHandles = newChildren.map(c => c.handle); - } else { - this.nodes.forEach(node => { - if (!node.parentHandle) { - existingChildrenHandles.push(node.handle); + let node = this.nodes.get(parentElement); + if (node.childrenHandles) { + for (const childHandle of node.childrenHandles) { + const childEleement = this.elements.get(childHandle); + if (childEleement) { + this.clear(childEleement); + } } - }); - } - - for (const existingChildHandle of existingChildrenHandles) { - const existingChildElement = this.elements.get(existingChildHandle); - if (existingChildElement && newChildren.every(c => c.handle !== existingChildHandle)) { - this.clear(existingChildElement); } + node.childrenHandles = []; + } else { + this.clearAll(); } - - return newChildren; } private clear(element: T): void { @@ -282,8 +290,13 @@ class ExtHostTreeView extends Disposable { this.elements.delete(node.handle); } - dispose() { + private clearAll(): void { + this.rootHandles = []; this.elements.clear(); this.nodes.clear(); } + + dispose() { + this.clearAll(); + } } \ No newline at end of file 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 1a692f0c257d54316aa05dd7b5536834ee401a2d..c5a1d5824b6aa161b5fd9c53ae71fb3ec58fc9f3 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts @@ -83,24 +83,24 @@ suite('ExtHostTreeView', function () { return testObject.$getElements('testNodeTreeProvider') .then(elements => { const actuals = elements.map(e => e.handle); - assert.deepEqual(actuals, ['0/0:a', '0/1:b']); + assert.deepEqual(actuals, ['0/0:a', '0/0:b']); return TPromise.join([ testObject.$getChildren('testNodeTreeProvider', '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']); + assert.deepEqual(actuals, ['0/0:a/0:aa', '0/0:a/0:ab']); return TPromise.join([ testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:aa').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testNodeTreeProvider', '0/0:a/1:ab').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:ab').then(children => assert.equal(children.length, 0)) ]); }), - testObject.$getChildren('testNodeTreeProvider', '0/1:b') + testObject.$getChildren('testNodeTreeProvider', '0/0:b') .then(children => { const actuals = children.map(e => e.handle); - assert.deepEqual(actuals, ['0/1:b/0:ba', '0/1:b/1:bb']); + assert.deepEqual(actuals, ['0/0:b/0:ba', '0/0:b/0:bb']); return TPromise.join([ - testObject.$getChildren('testNodeTreeProvider', '0/1:b/0:ba').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testNodeTreeProvider', '0/1:b/1:bb').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:ba').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:bb').then(children => assert.equal(children.length, 0)) ]); }) ]); @@ -111,24 +111,24 @@ suite('ExtHostTreeView', function () { return testObject.$getElements('testStringTreeProvider') .then(elements => { const actuals = elements.map(e => e.handle); - assert.deepEqual(actuals, ['0/0:a', '0/1:b']); + assert.deepEqual(actuals, ['0/0:a', '0/0:b']); return TPromise.join([ testObject.$getChildren('testStringTreeProvider', '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']); + assert.deepEqual(actuals, ['0/0:a/0:aa', '0/0:a/0:ab']); return TPromise.join([ testObject.$getChildren('testStringTreeProvider', '0/0:a/0:aa').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testStringTreeProvider', '0/0:a/1:ab').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testStringTreeProvider', '0/0:a/0:ab').then(children => assert.equal(children.length, 0)) ]); }), - testObject.$getChildren('testStringTreeProvider', '0/1:b') + testObject.$getChildren('testStringTreeProvider', '0/0:b') .then(children => { const actuals = children.map(e => e.handle); - assert.deepEqual(actuals, ['0/1:b/0:ba', '0/1:b/1:bb']); + assert.deepEqual(actuals, ['0/0:b/0:ba', '0/0:b/0:bb']); return TPromise.join([ - testObject.$getChildren('testStringTreeProvider', '0/1:b/0:ba').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testStringTreeProvider', '0/1:b/1:bb').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testStringTreeProvider', '0/0:b/0:ba').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testStringTreeProvider', '0/0:b/0:bb').then(children => assert.equal(children.length, 0)) ]); }) ]); @@ -146,9 +146,9 @@ suite('ExtHostTreeView', function () { 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', + assert.deepEqual(['0/0:b'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), { + handle: '0/0:b', label: 'b', }); c(null); @@ -159,10 +159,10 @@ suite('ExtHostTreeView', function () { test('refresh a leaf node', function (done) { target.onRefresh.event(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', + assert.deepEqual(['0/0:b/0:bb'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/0:b/0:bb']), { + handle: '0/0:b/0:bb', + parentHandle: '0/0:b', label: 'bb' }); done(); @@ -172,9 +172,9 @@ 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'], Object.keys(actuals)); - assert.deepEqual(removeUnsetKeys(actuals['0/1:b']), { - handle: '0/1:b', + assert.deepEqual(['0/0:b', '0/0:a/0:aa'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), { + handle: '0/0:b', label: 'b', }); assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), { @@ -191,9 +191,9 @@ 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'], Object.keys(actuals)); - assert.deepEqual(removeUnsetKeys(actuals['0/1:b']), { - handle: '0/1:b', + assert.deepEqual(['0/0:a/0:aa', '0/0:b'], Object.keys(actuals)); + assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), { + handle: '0/0:b', label: 'b', }); assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), { @@ -213,7 +213,7 @@ suite('ExtHostTreeView', function () { target.onRefresh.event(actuals => { assert.deepEqual(['0/0:a'], Object.keys(actuals)); assert.deepEqual(removeUnsetKeys(actuals['0/0:a']), { - handle: '0/0:a', + handle: '0/0:aa', label: 'aa', }); done(); @@ -234,7 +234,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'], Object.keys(actuals)); + assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals)); done(); }); @@ -246,7 +246,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'], Object.keys(actuals)); + assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals)); done(); }); @@ -293,6 +293,50 @@ suite('ExtHostTreeView', function () { }); }); + test('tree with duplicate labels', () => { + + const dupItems = { + 'adup1': 'c', + 'adup2': 'g', + 'bdup1': 'e', + 'hdup1': 'i', + 'hdup2': 'l', + 'jdup1': 'k' + }; + + labels['c'] = 'a'; + labels['e'] = 'b'; + labels['g'] = 'a'; + labels['i'] = 'h'; + labels['l'] = 'h'; + labels['k'] = 'j'; + + tree[dupItems['adup1']] = {}; + tree['d'] = {}; + + const bdup1Tree = {}; + bdup1Tree['h'] = {}; + bdup1Tree[dupItems['hdup1']] = {}; + bdup1Tree['j'] = {}; + bdup1Tree[dupItems['jdup1']] = {}; + bdup1Tree[dupItems['hdup2']] = {}; + + tree[dupItems['bdup1']] = bdup1Tree; + tree['f'] = {}; + tree[dupItems['adup2']] = {}; + + return testObject.$getElements('testNodeTreeProvider') + .then(elements => { + const actuals = elements.map(e => e.handle); + assert.deepEqual(actuals, ['0/0:a', '0/0:b', '0/1:a', '0/0:d', '0/1:b', '0/0:f', '0/2:a']); + return testObject.$getChildren('testNodeTreeProvider', '0/1:b') + .then(elements => { + const actuals = elements.map(e => e.handle); + assert.deepEqual(actuals, ['0/1:b/0:h', '0/1:b/1:h', '0/1:b/0:j', '0/1:b/1:j', '0/1:b/2:h']); + }); + }); + }); + function removeUnsetKeys(obj: any): any { const result = {}; for (const key of Object.keys(obj)) { @@ -369,4 +413,4 @@ suite('ExtHostTreeView', function () { return nodes[key]; } -}); +}); \ No newline at end of file