Skip to content

Commit

Permalink
Fix #40018
Browse files Browse the repository at this point in the history
  • Loading branch information
@sandy081 @isidorn
sandy081 authored and isidorn committed Jan 10, 2018
1 parent b5efda7 commit ca89ccb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 75 deletions.
27 changes: 7 additions & 20 deletions src/vs/workbench/api/node/extHostTreeViews.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ interface TreeNode {

class ExtHostTreeView<T> extends Disposable {

private static ROOT_HANDLE = '0';
private _itemHandlePool = 0;
private elements: Map<TreeItemHandle, T> = new Map<TreeItemHandle, T>();
private nodes: Map<T, TreeNode> = new Map<T, TreeNode>();

Expand All @@ -103,12 +103,10 @@ class ExtHostTreeView<T> extends Disposable {
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)
return this.resolveElement(element, node ? node.handle : `${++this._itemHandlePool}`, parentHandle)
.then(treeItem => {
if (treeItem) {
if (!currentHandle) {
// update the caches if current handle is not used
if (!node) {
this.nodes.set(element, {
handle: treeItem.handle,
parentHandle,
Expand Down Expand Up @@ -139,26 +137,21 @@ class ExtHostTreeView<T> extends Disposable {
}
}

private resolveElement(element: T, handleOrIndex: TreeItemHandle | number, parentHandle: TreeItemHandle): TPromise<ITreeItem> {
private resolveElement(element: T, handle: TreeItemHandle, parentHandle: TreeItemHandle): TPromise<ITreeItem> {
return asWinJsPromise(() => this.dataProvider.getTreeItem(element))
.then(extTreeItem => this.massageTreeItem(element, extTreeItem, handleOrIndex, parentHandle));
.then(extTreeItem => this.massageTreeItem(element, extTreeItem, handle, parentHandle));
}

private massageTreeItem(element: T, extensionTreeItem: vscode.TreeItem, handleOrIndex: TreeItemHandle | number, parentHandle: TreeItemHandle): ITreeItem {
private massageTreeItem(element: T, extensionTreeItem: vscode.TreeItem, handle: TreeItemHandle, parentHandle: TreeItemHandle): ITreeItem {
if (!extensionTreeItem) {
return null;
}

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

return {
handle,
parentHandle,
label,
label: extensionTreeItem.label,
command: extensionTreeItem.command ? this.commands.toInternal(extensionTreeItem.command) : void 0,
contextValue: extensionTreeItem.contextValue,
icon,
Expand All @@ -167,12 +160,6 @@ class ExtHostTreeView<T> extends Disposable {
};
}

private generateHandle(label: string, index: number, parentHandle: TreeItemHandle): TreeItemHandle {
parentHandle = parentHandle ? parentHandle : ExtHostTreeView.ROOT_HANDLE;
label = label.indexOf('/') !== -1 ? label.replace('/', '//') : label;
return `${parentHandle}/${index}:${label}`;
}

private getLightIconPath(extensionTreeItem: vscode.TreeItem): string {
if (extensionTreeItem.iconPath) {
if (typeof extensionTreeItem.iconPath === 'string' || extensionTreeItem.iconPath instanceof URI) {
Expand Down
97 changes: 42 additions & 55 deletions src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, ['1', '2']);
return TPromise.join([
testObject.$getChildren('testNodeTreeProvider', '0/0:a')
testObject.$getChildren('testNodeTreeProvider', '1')
.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, ['3', '4']);
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', '3').then(children => assert.equal(children.length, 0)),
testObject.$getChildren('testNodeTreeProvider', '4').then(children => assert.equal(children.length, 0))
]);
}),
testObject.$getChildren('testNodeTreeProvider', '0/1:b')
testObject.$getChildren('testNodeTreeProvider', '2')
.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, ['5', '6']);
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', '5').then(children => assert.equal(children.length, 0)),
testObject.$getChildren('testNodeTreeProvider', '6').then(children => assert.equal(children.length, 0))
]);
})
]);
Expand All @@ -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, ['1', '2']);
return TPromise.join([
testObject.$getChildren('testStringTreeProvider', '0/0:a')
testObject.$getChildren('testStringTreeProvider', '1')
.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, ['3', '4']);
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', '3').then(children => assert.equal(children.length, 0)),
testObject.$getChildren('testStringTreeProvider', '4').then(children => assert.equal(children.length, 0))
]);
}),
testObject.$getChildren('testStringTreeProvider', '0/1:b')
testObject.$getChildren('testStringTreeProvider', '2')
.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, ['5', '6']);
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', '5').then(children => assert.equal(children.length, 0)),
testObject.$getChildren('testStringTreeProvider', '6').then(children => assert.equal(children.length, 0))
]);
})
]);
Expand All @@ -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(['2'], Object.keys(actuals));
assert.deepEqual(removeUnsetKeys(actuals['2']), {
handle: '2',
label: 'b',
});
c(null);
Expand All @@ -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(['6'], Object.keys(actuals));
assert.deepEqual(removeUnsetKeys(actuals['6']), {
handle: '6',
parentHandle: '2',
label: 'bb'
});
done();
Expand All @@ -172,14 +172,14 @@ 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(['2', '3'], Object.keys(actuals));
assert.deepEqual(removeUnsetKeys(actuals['2']), {
handle: '2',
label: 'b',
});
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
handle: '0/0:a/0:aa',
parentHandle: '0/0:a',
assert.deepEqual(removeUnsetKeys(actuals['3']), {
handle: '3',
parentHandle: '1',
label: 'aa',
});
done();
Expand All @@ -191,14 +191,14 @@ 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(['2', '3'], Object.keys(actuals));
assert.deepEqual(removeUnsetKeys(actuals['2']), {
handle: '2',
label: 'b',
});
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
handle: '0/0:a/0:aa',
parentHandle: '0/0:a',
assert.deepEqual(removeUnsetKeys(actuals['3']), {
handle: '3',
parentHandle: '1',
label: 'aa',
});
done();
Expand All @@ -211,9 +211,9 @@ suite('ExtHostTreeView', function () {
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:a',
assert.deepEqual(['1'], Object.keys(actuals));
assert.deepEqual(removeUnsetKeys(actuals['1']), {
handle: '1',
label: 'aa',
});
done();
Expand All @@ -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(['1', '2'], Object.keys(actuals));
done();
});

Expand All @@ -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(['1', '2'], Object.keys(actuals));
done();
});

Expand Down Expand Up @@ -280,19 +280,6 @@ suite('ExtHostTreeView', function () {
onDidChangeTreeNode.fire(getNode('a'));
});

test('generate unique handles from labels by escaping them', () => {
tree = {
'a/0:b': {}
};

onDidChangeTreeNode.fire();

return testObject.$getElements('testNodeTreeProvider')
.then(elements => {
assert.deepEqual(elements.map(e => e.handle), ['0/0:a//0:b']);
});
});

function removeUnsetKeys(obj: any): any {
const result = {};
for (const key of Object.keys(obj)) {
Expand Down

0 comments on commit ca89ccb

Please sign in to comment.