diff --git a/src/vs/workbench/api/node/extHostTreeViews.ts b/src/vs/workbench/api/node/extHostTreeViews.ts index 67f75fd138677..5a63e004f563e 100644 --- a/src/vs/workbench/api/node/extHostTreeViews.ts +++ b/src/vs/workbench/api/node/extHostTreeViews.ts @@ -107,8 +107,8 @@ class ExtHostTreeView extends Disposable { asWinJsPromise(() => this.dataProvider.getTreeItem(element)) .then(extTreeItem => { if (extTreeItem) { - if (typeof element === 'string' && this.elements.has(this.createHandle(element, extTreeItem))) { - throw new Error(localize('treeView.duplicateElement', 'Element {0} is already registered', element)); + if (extTreeItem.id && this.elements.has(this.createHandle(element, extTreeItem))) { + throw new Error(localize('treeView.duplicateElement', 'Element with id {0} is already registered', extTreeItem.id)); } return { element, extTreeItem }; } @@ -199,9 +199,9 @@ class ExtHostTreeView extends Disposable { }; } - private createHandle(element: T, { label, resourceUri }: vscode.TreeItem, parentHandle?: TreeItemHandle): TreeItemHandle { - if (typeof element === 'string') { - return `${ExtHostTreeView.ID_HANDLE_PREFIX}/${element}`; + private createHandle(element: T, { id, label, resourceUri }: vscode.TreeItem, parentHandle?: TreeItemHandle): TreeItemHandle { + if (id) { + return `${ExtHostTreeView.ID_HANDLE_PREFIX}/${id}`; } const prefix = parentHandle ? parentHandle : ExtHostTreeView.LABEL_HANDLE_PREFIX; 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 8cf64585e53e9..71dea5ec0fd2f 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts @@ -38,7 +38,7 @@ suite('ExtHostTreeView', function () { let testObject: ExtHostTreeViews; let target: RecordingShape; let onDidChangeTreeNode: Emitter<{ key: string }>; - let onDidChangeTreeKey: Emitter; + let onDidChangeTreeNodeWithId: Emitter<{ key: string }>; let tree, labels, nodes; setup(() => { @@ -68,9 +68,9 @@ suite('ExtHostTreeView', function () { target = new RecordingShape(); testObject = new ExtHostTreeViews(target, new ExtHostCommands(rpcProtocol, new ExtHostHeapService(), new NullLogService())); onDidChangeTreeNode = new Emitter<{ key: string }>(); - onDidChangeTreeKey = new Emitter(); + onDidChangeTreeNodeWithId = new Emitter<{ key: string }>(); testObject.registerTreeDataProvider('testNodeTreeProvider', aNodeTreeDataProvider()); - testObject.registerTreeDataProvider('testStringTreeProvider', aStringTreeDataProvider()); + testObject.registerTreeDataProvider('testNodeWithIdTreeProvider', aNodeWithIdTreeDataProvider()); testObject.$getElements('testNodeTreeProvider').then(elements => { for (const element of elements) { @@ -107,34 +107,47 @@ suite('ExtHostTreeView', function () { }); }); - test('construct string tree', () => { - return testObject.$getElements('testStringTreeProvider') + test('construct id tree', () => { + return testObject.$getElements('testNodeWithIdTreeProvider') .then(elements => { const actuals = elements.map(e => e.handle); assert.deepEqual(actuals, ['1/a', '1/b']); return TPromise.join([ - testObject.$getChildren('testStringTreeProvider', '1/a') + testObject.$getChildren('testNodeWithIdTreeProvider', '1/a') .then(children => { const actuals = children.map(e => e.handle); assert.deepEqual(actuals, ['1/aa', '1/ab']); return TPromise.join([ - testObject.$getChildren('testStringTreeProvider', '1/aa').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testStringTreeProvider', '1/ab').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testNodeWithIdTreeProvider', '1/aa').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testNodeWithIdTreeProvider', '1/ab').then(children => assert.equal(children.length, 0)) ]); }), - testObject.$getChildren('testStringTreeProvider', '1/b') + testObject.$getChildren('testNodeWithIdTreeProvider', '1/b') .then(children => { const actuals = children.map(e => e.handle); assert.deepEqual(actuals, ['1/ba', '1/bb']); return TPromise.join([ - testObject.$getChildren('testStringTreeProvider', '1/ba').then(children => assert.equal(children.length, 0)), - testObject.$getChildren('testStringTreeProvider', '1/bb').then(children => assert.equal(children.length, 0)) + testObject.$getChildren('testNodeWithIdTreeProvider', '1/ba').then(children => assert.equal(children.length, 0)), + testObject.$getChildren('testNodeWithIdTreeProvider', '1/bb').then(children => assert.equal(children.length, 0)) ]); }) ]); }); }); + test('error is thrown if id is not unique', () => { + tree['a'] = { + 'a': {} + }; + return testObject.$getElements('testNodeWithIdTreeProvider') + .then(elements => { + const actuals = elements.map(e => e.handle); + assert.deepEqual(actuals, ['1/a', '1/b']); + return testObject.$getChildren('testNodeWithIdTreeProvider', '1/a') + .then(children => assert.fail('Should fail with duplicate id'), () => null); + }); + }); + test('refresh root', function (done) { target.onRefresh.event(actuals => { assert.equal(undefined, actuals); @@ -359,15 +372,17 @@ suite('ExtHostTreeView', function () { }; } - function aStringTreeDataProvider(): TreeDataProvider { + function aNodeWithIdTreeDataProvider(): TreeDataProvider<{ key: string }> { return { - getChildren: (element: string): string[] => { - return getChildren(element); + getChildren: (element: { key: string }): { key: string }[] => { + return getChildren(element ? element.key : undefined).map(key => getNode(key)); }, - getTreeItem: (element: string): TreeItem => { - return getTreeItem(element); + getTreeItem: (element: { key: string }): TreeItem => { + const treeItem = getTreeItem(element.key); + treeItem.id = element.key; + return treeItem; }, - onDidChangeTreeData: onDidChangeTreeKey.event + onDidChangeTreeData: onDidChangeTreeNodeWithId.event }; }