diff --git a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts index 473c4eef9ac86790e04d9aedb4343433e9fe37ad..a75a49625a7614834a1110bf11aeb76287c0d3cc 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { Event, Emitter } from 'vs/base/common/event'; import { TPromise } from 'vs/base/common/winjs.base'; import { Disposable } from 'vs/base/common/lifecycle'; import { ExtHostContext, MainThreadTreeViewsShape, ExtHostTreeViewsShape, MainContext, IExtHostContext } from '../node/extHost.protocol'; @@ -29,7 +28,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie } $registerTreeViewDataProvider(treeViewId: string): void { - const dataProvider = this._register(new TreeViewDataProvider(treeViewId, this._proxy, this.notificationService)); + const dataProvider = new TreeViewDataProvider(treeViewId, this._proxy, this.notificationService); this._dataProviders.set(treeViewId, dataProvider); const treeViewer = this.viewsService.getTreeViewer(treeViewId); if (treeViewer) { @@ -48,11 +47,14 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie }); } - $refresh(treeViewId: string, itemsToRefresh: { [treeItemHandle: string]: ITreeItem }): void { + $refresh(treeViewId: string, itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }): TPromise { + const viewer = this.viewsService.getTreeViewer(treeViewId); const dataProvider = this._dataProviders.get(treeViewId); - if (dataProvider) { - dataProvider.refresh(itemsToRefresh); + if (viewer && dataProvider) { + const itemsToRefresh = dataProvider.getItemsToRefresh(itemsToRefreshByHandle); + return viewer.refresh(itemsToRefresh.length ? itemsToRefresh : void 0); } + return TPromise.as(null); } private registerListeners(treeViewId: string, treeViewer: ITreeViewer): void { @@ -62,6 +64,12 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie } dispose(): void { + this._dataProviders.forEach((dataProvider, treeViewId) => { + const treeViewer = this.viewsService.getTreeViewer(treeViewId); + if (treeViewer) { + treeViewer.dataProvider = null; + } + }); this._dataProviders.clear(); super.dispose(); } @@ -71,12 +79,6 @@ type TreeItemHandle = string; class TreeViewDataProvider implements ITreeViewDataProvider { - private readonly _onDidChange: Emitter = new Emitter(); - readonly onDidChange: Event = this._onDidChange.event; - - private readonly _onDispose: Emitter = new Emitter(); - readonly onDispose: Event = this._onDispose.event; - private itemsMap: Map = new Map(); constructor(private treeViewId: string, @@ -98,7 +100,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider { }); } - refresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }) { + getItemsToRefresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }): ITreeItem[] { const itemsToRefresh: ITreeItem[] = []; if (itemsToRefreshByHandle) { for (const treeItemHandle of Object.keys(itemsToRefreshByHandle)) { @@ -121,11 +123,7 @@ class TreeViewDataProvider implements ITreeViewDataProvider { } } } - if (itemsToRefresh.length) { - this._onDidChange.fire(itemsToRefresh); - } else { - this._onDidChange.fire(); - } + return itemsToRefresh; } private postGetChildren(elements: ITreeItem[]): ITreeItem[] { @@ -148,8 +146,4 @@ class TreeViewDataProvider implements ITreeViewDataProvider { } } } - - dispose(): void { - this._onDispose.fire(); - } } diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index e24f4b33b3740eddd2f8acf0ae616dccde8cd0f0..659213efef86a6e48bbf19881a8976d3b71a6f6f 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -206,7 +206,7 @@ export interface MainThreadTextEditorsShape extends IDisposable { export interface MainThreadTreeViewsShape extends IDisposable { $registerTreeViewDataProvider(treeViewId: string): void; - $refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void; + $refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): TPromise; $reveal(treeViewId: string, treeItem: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean }): TPromise; } diff --git a/src/vs/workbench/api/node/extHostTreeViews.ts b/src/vs/workbench/api/node/extHostTreeViews.ts index 44e7d43d4a637d60cbf3e65ebdd7b51c21f37f75..ead30bdf54d0d79379b2ed0c3cc4e82b32496d13 100644 --- a/src/vs/workbench/api/node/extHostTreeViews.ts +++ b/src/vs/workbench/api/node/extHostTreeViews.ts @@ -119,11 +119,25 @@ class ExtHostTreeView extends Disposable { private _onDidCollapseElement: Emitter = this._register(new Emitter()); readonly onDidCollapseElement: Event = this._onDidCollapseElement.event; + private refreshPromise: TPromise = TPromise.as(null); + constructor(private viewId: string, private dataProvider: vscode.TreeDataProvider, private proxy: MainThreadTreeViewsShape, private commands: CommandsConverter) { super(); this.proxy.$registerTreeViewDataProvider(viewId); if (this.dataProvider.onDidChangeTreeData) { - this._register(debounceEvent(this.dataProvider.onDidChangeTreeData, (last, current) => last ? [...last, current] : [current], 200)(elements => this.refresh(elements))); + let refreshingPromise, promiseCallback; + this._register(debounceEvent(this.dataProvider.onDidChangeTreeData, (last, current) => { + if (!refreshingPromise) { + // New refresh has started + refreshingPromise = new TPromise((c, e) => promiseCallback = c); + this.refreshPromise = this.refreshPromise.then(() => refreshingPromise); + } + return last ? [...last, current] : [current]; + }, 200, true)(elements => { + const _promiseCallback = promiseCallback; + refreshingPromise = null; + this.refresh(elements).then(() => _promiseCallback()); + })); } } @@ -145,9 +159,10 @@ class ExtHostTreeView extends Disposable { reveal(element: T, options?: { select?: boolean }): TPromise { if (typeof this.dataProvider.getParent !== 'function') { - return TPromise.wrapError(new Error(`Required registered TreeDataProvider to implement 'getParent' method to access 'reveal' mehtod`)); + return TPromise.wrapError(new Error(`Required registered TreeDataProvider to implement 'getParent' method to access 'reveal' method`)); } - return this.resolveUnknownParentChain(element) + return this.refreshPromise + .then(() => this.resolveUnknownParentChain(element)) .then(parentChain => this.resolveTreeNode(element, parentChain[parentChain.length - 1]) .then(treeNode => this.proxy.$reveal(this.viewId, treeNode.item, parentChain.map(p => p.item), options))); } @@ -234,17 +249,18 @@ class ExtHostTreeView extends Disposable { .then(nodes => nodes.filter(n => !!n)); } - private refresh(elements: T[]): void { + private refresh(elements: T[]): TPromise { const hasRoot = elements.some(element => !element); if (hasRoot) { this.clearAll(); // clear cache - this.proxy.$refresh(this.viewId); + return this.proxy.$refresh(this.viewId); } else { const handlesToRefresh = this.getHandlesToRefresh(elements); if (handlesToRefresh.length) { - this.refreshHandles(handlesToRefresh); + return this.refreshHandles(handlesToRefresh); } } + return TPromise.as(null); } private getHandlesToRefresh(elements: T[]): TreeItemHandle[] { diff --git a/src/vs/workbench/browser/parts/views/customView.ts b/src/vs/workbench/browser/parts/views/customView.ts index 29fe9493a0a6b779464ac7a8d58daeafd06d8e4c..8fcfc3f1c499235c2048895ed259d8da0f1d1d49 100644 --- a/src/vs/workbench/browser/parts/views/customView.ts +++ b/src/vs/workbench/browser/parts/views/customView.ts @@ -5,7 +5,7 @@ import 'vs/css!./media/views'; import * as errors from 'vs/base/common/errors'; -import { IDisposable, Disposable, dispose } from 'vs/base/common/lifecycle'; +import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { TPromise } from 'vs/base/common/winjs.base'; import * as DOM from 'vs/base/browser/dom'; @@ -111,7 +111,6 @@ class CustomTreeViewer extends Disposable implements ITreeViewer { private elementsToRefresh: ITreeItem[] = []; private _dataProvider: ITreeViewDataProvider; - private dataProviderDisposables: IDisposable[] = []; private _onDidExpandItem: Emitter = this._register(new Emitter()); readonly onDidExpandItem: Event = this._onDidExpandItem.event; @@ -141,11 +140,8 @@ class CustomTreeViewer extends Disposable implements ITreeViewer { } set dataProvider(dataProvider: ITreeViewDataProvider) { - dispose(this.dataProviderDisposables); if (dataProvider) { this._dataProvider = new class implements ITreeViewDataProvider { - onDidChange = dataProvider.onDidChange; - onDispose = dataProvider.onDispose; getChildren(node?: ITreeItem): TPromise { if (node && node.children) { return TPromise.as(node.children); @@ -157,8 +153,6 @@ class CustomTreeViewer extends Disposable implements ITreeViewer { }); } }; - this._register(dataProvider.onDidChange(elements => this.refresh(elements), this, this.dataProviderDisposables)); - this._register(dataProvider.onDispose(() => this.dataProvider = null, this, this.dataProviderDisposables)); } else { this._dataProvider = null; } diff --git a/src/vs/workbench/common/views.ts b/src/vs/workbench/common/views.ts index 4c07be8e5f4e8b1e663108d1681f55dafdd1b6ac..29d774d0c4ef0c04922e945c0c9403faef160032 100644 --- a/src/vs/workbench/common/views.ts +++ b/src/vs/workbench/common/views.ts @@ -241,9 +241,6 @@ export interface ITreeItem { export interface ITreeViewDataProvider { - onDidChange: Event; - - onDispose: Event; - getChildren(element?: ITreeItem): TPromise; + } \ 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 969cac303595fa76c0259555d699446fded89f3a..feefbc62d17609173e4a5fccf044dfedc3122333 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts @@ -31,8 +31,8 @@ suite('ExtHostTreeView', function () { $registerTreeViewDataProvider(treeViewId: string): void { } - $refresh(viewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): void { - this.onRefresh.fire(itemsToRefresh); + $refresh(viewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): TPromise { + return TPromise.as(null).then(() => this.onRefresh.fire(itemsToRefresh)); } $reveal(): TPromise { @@ -78,11 +78,7 @@ suite('ExtHostTreeView', function () { testObject.createTreeView('testNodeTreeProvider', { treeDataProvider: aNodeTreeDataProvider() }); testObject.createTreeView('testNodeWithIdTreeProvider', { treeDataProvider: aNodeWithIdTreeDataProvider() }); - testObject.$getChildren('testNodeTreeProvider').then(elements => { - for (const element of elements) { - testObject.$getChildren('testNodeTreeProvider', element.handle); - } - }); + return loadCompleteTree('testNodeTreeProvider'); }); test('construct node tree', () => { @@ -256,9 +252,13 @@ suite('ExtHostTreeView', function () { }); test('refresh calls are throttled on roots', function (done) { + let counter = 0; target.onRefresh.event(actuals => { + counter++; assert.equal(undefined, actuals); - done(); + if (counter > 1) { + done(); + } }); onDidChangeTreeNode.fire(); onDidChangeTreeNode.fire(); @@ -267,9 +267,18 @@ suite('ExtHostTreeView', function () { }); test('refresh calls are throttled on elements', function (done) { + let counter = 0; target.onRefresh.event(actuals => { - assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals)); - done(); + counter++; + if (counter === 1) { + assert.deepEqual(['0/0:a'], Object.keys(actuals)); + } + if (counter === 2) { + assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals)); + } + if (counter > 1) { + done(); + } }); onDidChangeTreeNode.fire(getNode('a')); @@ -319,7 +328,6 @@ suite('ExtHostTreeView', function () { 'a/0:b': {} }; - onDidChangeTreeNode.fire(); target.onRefresh.event(() => { testObject.$getChildren('testNodeTreeProvider') .then(elements => { @@ -327,6 +335,7 @@ suite('ExtHostTreeView', function () { done(); }); }); + onDidChangeTreeNode.fire(); }); test('tree with duplicate labels', (done) => { @@ -361,8 +370,6 @@ suite('ExtHostTreeView', function () { tree['f'] = {}; tree[dupItems['adup2']] = {}; - onDidChangeTreeNode.fire(); - target.onRefresh.event(() => { testObject.$getChildren('testNodeTreeProvider') .then(elements => { @@ -376,6 +383,8 @@ suite('ExtHostTreeView', function () { }); }); }); + + onDidChangeTreeNode.fire(); }); test('getChildren is not returned from cache if refreshed', (done) => { @@ -383,7 +392,6 @@ suite('ExtHostTreeView', function () { 'c': {} }; - onDidChangeTreeNode.fire(); target.onRefresh.event(() => { testObject.$getChildren('testNodeTreeProvider') .then(elements => { @@ -391,6 +399,8 @@ suite('ExtHostTreeView', function () { done(); }); }); + + onDidChangeTreeNode.fire(); }); test('getChildren is returned from cache if not refreshed', () => { @@ -474,6 +484,79 @@ suite('ExtHostTreeView', function () { }); }); + test('reveal after first udpate', () => { + const revealTarget = sinon.spy(target, '$reveal'); + const treeView = testObject.createTreeView('treeDataProvider', { treeDataProvider: aCompleteNodeTreeDataProvider() }); + return loadCompleteTree('treeDataProvider') + .then(() => { + tree = { + 'a': { + 'aa': {}, + 'ac': {} + }, + 'b': { + 'ba': {}, + 'bb': {} + } + }; + onDidChangeTreeNode.fire(getNode('a')); + + return treeView.reveal({ key: 'ac' }) + .then(() => { + assert.ok(revealTarget.calledOnce); + assert.deepEqual('treeDataProvider', revealTarget.args[0][0]); + assert.deepEqual({ handle: '0/0:a/0:ac', label: 'ac', collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:a' }, removeUnsetKeys(revealTarget.args[0][1])); + assert.deepEqual([{ handle: '0/0:a', label: 'a', collapsibleState: TreeItemCollapsibleState.Collapsed }], (>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg))); + assert.equal(void 0, revealTarget.args[0][3]); + }); + }); + }); + + test('reveal after second udpate', () => { + const revealTarget = sinon.spy(target, '$reveal'); + const treeView = testObject.createTreeView('treeDataProvider', { treeDataProvider: aCompleteNodeTreeDataProvider() }); + return loadCompleteTree('treeDataProvider') + .then(() => { + tree = { + 'a': { + 'aa': {}, + 'ac': {} + }, + 'b': { + 'ba': {}, + 'bb': {} + } + }; + onDidChangeTreeNode.fire(getNode('a')); + tree = { + 'a': { + 'aa': {}, + 'ac': {} + }, + 'b': { + 'ba': {}, + 'bc': {} + } + }; + onDidChangeTreeNode.fire(getNode('b')); + + return treeView.reveal({ key: 'bc' }) + .then(() => { + assert.ok(revealTarget.calledOnce); + assert.deepEqual('treeDataProvider', revealTarget.args[0][0]); + assert.deepEqual({ handle: '0/0:b/0:bc', label: 'bc', collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:b' }, removeUnsetKeys(revealTarget.args[0][1])); + assert.deepEqual([{ handle: '0/0:b', label: 'b', collapsibleState: TreeItemCollapsibleState.Collapsed }], (>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg))); + assert.equal(void 0, revealTarget.args[0][3]); + }); + }); + }); + + function loadCompleteTree(treeId, element?: string): TPromise { + return testObject.$getChildren(treeId, element) + .then(elements => elements.map(e => loadCompleteTree(treeId, e.handle))) + .then(() => null); + } + function removeUnsetKeys(obj: any): any { const result = {}; for (const key of Object.keys(obj)) {