contributable views model: emit right events

src/vs/workbench/browser/parts/views/contributableViews.ts

@@ -8,7 +8,6 @@ import { ViewsRegistry, IViewDescriptor, ViewLocation } from 'vs/workbench/commo
 import { IContextKeyService, IContextKeyChangeEvent, IReadableSet } from 'vs/platform/contextkey/common/contextkey';
 import { Event, chain, filterEvent, Emitter } from 'vs/base/common/event';
 import { sortedDiff, firstIndex, move } from 'vs/base/common/arrays';
-import { ISequence, Sequence, ISplice } from 'vs/base/common/sequence';
 
 function filterViewEvent(location: ViewLocation, event: Event<IViewDescriptor[]>): Event<IViewDescriptor[]> {
 	return chain(event)
@@ -174,18 +173,31 @@ interface IViewState {
 	order?: number;
 }
 
-// TODO: this should not be an ISequence but expose events:
-// onDidAdd
-// onDidRemove
-// onDidMove
-export class ContributableViewsModel implements ISequence<IViewDescriptor>{
+export interface IViewDescriptorRef {
+	viewDescriptor: IViewDescriptor;
+	index: number;
+}
+
+export class ContributableViewsModel {
 
-	private viewDescriptors: IViewDescriptor[] = [];
+	private allViewDescriptors: IViewDescriptor[] = [];
 	private viewStates = new Map<string, IViewState>();
 
-	private visibleViewDescriptors = new Sequence<IViewDescriptor>();
-	get elements() { return this.visibleViewDescriptors.elements; }
-	readonly onDidSplice: Event<ISplice<IViewDescriptor>> = this.visibleViewDescriptors.onDidSplice;
+	get viewDescriptors(): IViewDescriptor[] {
+		return this.allViewDescriptors.filter(v => {
+			const state = this.viewStates.get(v.id);
+			return state.visible;
+		});
+	}
+
+	private _onDidAdd = new Emitter<IViewDescriptorRef>();
+	readonly onDidAdd: Event<IViewDescriptorRef> = this._onDidAdd.event;
+
+	private _onDidRemove = new Emitter<IViewDescriptorRef>();
+	readonly onDidRemove: Event<IViewDescriptorRef> = this._onDidRemove.event;
+
+	private _onDidMove = new Emitter<{ from: IViewDescriptorRef; to: IViewDescriptorRef; }>();
+	readonly onDidMove: Event<{ from: IViewDescriptorRef; to: IViewDescriptorRef; }> = this._onDidMove.event;
 
 	private disposables: IDisposable[] = [];
 
@@ -209,30 +221,35 @@ export class ContributableViewsModel implements ISequence<IViewDescriptor>{
 		state.visible = visible;
 
 		if (visible) {
-			this.visibleViewDescriptors.splice(visibleIndex, 0, [viewDescriptor]);
+			this._onDidAdd.fire({ index: visibleIndex, viewDescriptor });
 		} else {
-			this.visibleViewDescriptors.splice(visibleIndex, 1);
+			this._onDidRemove.fire({ index: visibleIndex, viewDescriptor });
 		}
 	}
 
 	move(from: string, to: string): void {
-		const fromIndex = firstIndex(this.viewDescriptors, v => v.id === from);
-		const toIndex = firstIndex(this.viewDescriptors, v => v.id === to);
+		const fromIndex = firstIndex(this.allViewDescriptors, v => v.id === from);
+		const toIndex = firstIndex(this.allViewDescriptors, v => v.id === to);
+
+		const fromViewDescriptor = this.allViewDescriptors[fromIndex];
+		const toViewDescriptor = this.allViewDescriptors[toIndex];
 
-		const viewDescriptors = [...this.viewDescriptors];
-		move(viewDescriptors, fromIndex, toIndex);
+		move(this.allViewDescriptors, fromIndex, toIndex);
 
-		for (let index = 0; index < viewDescriptors.length; index++) {
-			const state = this.viewStates.get(viewDescriptors[index].id);
+		for (let index = 0; index < this.allViewDescriptors.length; index++) {
+			const state = this.viewStates.get(this.allViewDescriptors[index].id);
 			state.order = index;
 		}
 
-		this.onDidChangeViewDescriptors(viewDescriptors);
+		this._onDidMove.fire({
+			from: { index: fromIndex, viewDescriptor: fromViewDescriptor },
+			to: { index: toIndex, viewDescriptor: toViewDescriptor }
+		});
 	}
 
 	private find(id: string): { index: number, visibleIndex: number, viewDescriptor: IViewDescriptor, state: IViewState } {
-		for (let i = 0, visibleIndex = 0; i < this.viewDescriptors.length; i++) {
-			const viewDescriptor = this.viewDescriptors[i];
+		for (let i = 0, visibleIndex = 0; i < this.allViewDescriptors.length; i++) {
+			const viewDescriptor = this.allViewDescriptors[i];
 			const state = this.viewStates.get(viewDescriptor.id);
 
 			if (viewDescriptor.id === id) {
@@ -273,7 +290,7 @@ export class ContributableViewsModel implements ISequence<IViewDescriptor>{
 	private onDidChangeViewDescriptors(viewDescriptors: IViewDescriptor[]): void {
 		const ids = new Set<string>();
 
-		for (const viewDescriptor of this.viewDescriptors) {
+		for (const viewDescriptor of this.allViewDescriptors) {
 			ids.add(viewDescriptor.id);
 		}
 
@@ -288,21 +305,21 @@ export class ContributableViewsModel implements ISequence<IViewDescriptor>{
 		}
 
 		const splices = sortedDiff<IViewDescriptor>(
-			this.viewDescriptors,
+			this.allViewDescriptors,
 			viewDescriptors,
 			this.compareViewDescriptors.bind(this)
 		).reverse();
 
 		for (const splice of splices) {
-			const startViewDescriptor = this.viewDescriptors[splice.start];
+			const startViewDescriptor = this.allViewDescriptors[splice.start];
 			let startIndex = startViewDescriptor ? this.find(startViewDescriptor.id).visibleIndex : 0;
 
 			for (let i = 0; i < splice.deleteCount; i++) {
-				const viewDescriptor = this.viewDescriptors[splice.start + i];
+				const viewDescriptor = this.allViewDescriptors[splice.start + i];
 				const { state } = this.find(viewDescriptor.id);
 
 				if (state.visible) {
-					this.visibleViewDescriptors.splice(startIndex, 1);
+					this._onDidRemove.fire({ index: startIndex, viewDescriptor: viewDescriptor });
 				}
 			}
 
@@ -311,11 +328,11 @@ export class ContributableViewsModel implements ISequence<IViewDescriptor>{
 				const state = this.viewStates.get(viewDescriptor.id);
 
 				if (state.visible) {
-					this.visibleViewDescriptors.splice(startIndex++, 0, [viewDescriptor]);
+					this._onDidAdd.fire({ index: startIndex++, viewDescriptor: viewDescriptor });
 				}
 			}
 		}
 
-		this.viewDescriptors = viewDescriptors;
+		this.allViewDescriptors = viewDescriptors;
 	}
 }
\ No newline at end of file

src/vs/workbench/test/browser/parts/views/contributableViews.test.ts

@@ -9,10 +9,28 @@ import { ViewLocation, ViewsRegistry, IViewDescriptor } from 'vs/workbench/commo
 import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService';
 import { IContextKeyService, ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
 import { SimpleConfigurationService } from 'vs/editor/standalone/browser/simpleServices';
-import { Sequence } from 'vs/base/common/sequence';
+import { IDisposable, dispose } from 'vs/base/common/lifecycle';
+import { move } from 'vs/base/common/arrays';
 
 const location = new ViewLocation('test');
 
+class ViewDescriptorSequence {
+
+	readonly elements: IViewDescriptor[];
+	private disposables: IDisposable[] = [];
+
+	constructor(model: ContributableViewsModel) {
+		this.elements = [...model.viewDescriptors];
+		model.onDidAdd(({ viewDescriptor, index }) => this.elements.splice(index, 0, viewDescriptor), null, this.disposables);
+		model.onDidRemove(({ viewDescriptor, index }) => this.elements.splice(index, 1), null, this.disposables);
+		model.onDidMove(({ from, to }) => move(this.elements, from.index, to.index), null, this.disposables);
+	}
+
+	dispose() {
+		this.disposables = dispose(this.disposables);
+	}
+}
+
 suite('ContributableViewsModel', () => {
 	let contextKeyService: IContextKeyService;
 
@@ -27,16 +45,14 @@ suite('ContributableViewsModel', () => {
 
 	test('empty model', function () {
 		const model = new ContributableViewsModel(location, contextKeyService);
-		assert.equal(model.elements.length, 0);
+		assert.equal(model.viewDescriptors.length, 0);
 	});
 
 	test('register/unregister', function () {
 		const model = new ContributableViewsModel(location, contextKeyService);
-		const seq = new Sequence();
+		const seq = new ViewDescriptorSequence(model);
 
-		model.onDidSplice(({ start, deleteCount, toInsert }) => seq.splice(start, deleteCount, toInsert));
-
-		assert.equal(model.elements.length, 0);
+		assert.equal(model.viewDescriptors.length, 0);
 		assert.equal(seq.elements.length, 0);
 
 		const viewDescriptor: IViewDescriptor = {
@@ -48,21 +64,23 @@ suite('ContributableViewsModel', () => {
 
 		ViewsRegistry.registerViews([viewDescriptor]);
 
-		assert.equal(model.elements.length, 1);
+		assert.equal(model.viewDescriptors.length, 1);
 		assert.equal(seq.elements.length, 1);
-		assert.deepEqual(model.elements[0], viewDescriptor);
+		assert.deepEqual(model.viewDescriptors[0], viewDescriptor);
 		assert.deepEqual(seq.elements[0], viewDescriptor);
 
 		ViewsRegistry.deregisterViews(['view1'], location);
 
-		assert.equal(model.elements.length, 0);
+		assert.equal(model.viewDescriptors.length, 0);
 		assert.equal(seq.elements.length, 0);
 	});
 
 	test('when contexts', async function () {
 		const model = new ContributableViewsModel(location, contextKeyService);
+		const seq = new ViewDescriptorSequence(model);
 
-		assert.equal(model.elements.length, 0);
+		assert.equal(model.viewDescriptors.length, 0);
+		assert.equal(seq.elements.length, 0);
 
 		const viewDescriptor: IViewDescriptor = {
 			id: 'view1',
@@ -73,81 +91,106 @@ suite('ContributableViewsModel', () => {
 		};
 
 		ViewsRegistry.registerViews([viewDescriptor]);
-		assert.equal(model.elements.length, 0, 'view should not appear since context isnt in');
+		assert.equal(model.viewDescriptors.length, 0, 'view should not appear since context isnt in');
+		assert.equal(seq.elements.length, 0);
 
 		const key = contextKeyService.createKey('showview1', false);
-		assert.equal(model.elements.length, 0, 'view should still not appear since showview1 isnt true');
+		assert.equal(model.viewDescriptors.length, 0, 'view should still not appear since showview1 isnt true');
+		assert.equal(seq.elements.length, 0);
 
 		key.set(true);
 		await new Promise(c => setTimeout(c, 30));
-		assert.equal(model.elements.length, 1, 'view should appear');
-		assert.deepEqual(model.elements[0], viewDescriptor);
+		assert.equal(model.viewDescriptors.length, 1, 'view should appear');
+		assert.equal(seq.elements.length, 1);
+		assert.deepEqual(model.viewDescriptors[0], viewDescriptor);
+		assert.equal(seq.elements[0], viewDescriptor);
 
 		key.set(false);
 		await new Promise(c => setTimeout(c, 30));
-		assert.equal(model.elements.length, 0, 'view should disappear');
+		assert.equal(model.viewDescriptors.length, 0, 'view should disappear');
+		assert.equal(seq.elements.length, 0);
 
 		ViewsRegistry.deregisterViews(['view1'], location);
-		assert.equal(model.elements.length, 0, 'view should not be there anymore');
+		assert.equal(model.viewDescriptors.length, 0, 'view should not be there anymore');
+		assert.equal(seq.elements.length, 0);
 
 		key.set(true);
 		await new Promise(c => setTimeout(c, 30));
-		assert.equal(model.elements.length, 0, 'view should not be there anymore');
+		assert.equal(model.viewDescriptors.length, 0, 'view should not be there anymore');
+		assert.equal(seq.elements.length, 0);
 	});
 
 	test('setVisible', function () {
 		const model = new ContributableViewsModel(location, contextKeyService);
+		const seq = new ViewDescriptorSequence(model);
+
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, location, name: 'Test View 1' };
 		const view2: IViewDescriptor = { id: 'view2', ctor: null, location, name: 'Test View 2' };
 		const view3: IViewDescriptor = { id: 'view3', ctor: null, location, name: 'Test View 3' };
 
 		ViewsRegistry.registerViews([view1, view2, view3]);
-		assert.deepEqual(model.elements, [view1, view2, view3]);
+		assert.deepEqual(model.viewDescriptors, [view1, view2, view3]);
+		assert.deepEqual(seq.elements, [view1, view2, view3]);
 
 		model.setVisible('view2', true);
-		assert.deepEqual(model.elements, [view1, view2, view3], 'nothing should happen');
+		assert.deepEqual(model.viewDescriptors, [view1, view2, view3], 'nothing should happen');
+		assert.deepEqual(seq.elements, [view1, view2, view3]);
 
 		model.setVisible('view2', false);
-		assert.deepEqual(model.elements, [view1, view3], 'view2 should hide');
+		assert.deepEqual(model.viewDescriptors, [view1, view3], 'view2 should hide');
+		assert.deepEqual(seq.elements, [view1, view3]);
 
 		model.setVisible('view1', false);
-		assert.deepEqual(model.elements, [view3], 'view1 should hide');
+		assert.deepEqual(model.viewDescriptors, [view3], 'view1 should hide');
+		assert.deepEqual(seq.elements, [view3]);
 
 		model.setVisible('view3', false);
-		assert.deepEqual(model.elements, [], 'view3 shoud hide');
+		assert.deepEqual(model.viewDescriptors, [], 'view3 shoud hide');
+		assert.deepEqual(seq.elements, []);
 
 		model.setVisible('view1', true);
-		assert.deepEqual(model.elements, [view1], 'view1 should show');
+		assert.deepEqual(model.viewDescriptors, [view1], 'view1 should show');
+		assert.deepEqual(seq.elements, [view1]);
 
 		model.setVisible('view3', true);
-		assert.deepEqual(model.elements, [view1, view3], 'view3 should show');
+		assert.deepEqual(model.viewDescriptors, [view1, view3], 'view3 should show');
+		assert.deepEqual(seq.elements, [view1, view3]);
 
 		model.setVisible('view2', true);
-		assert.deepEqual(model.elements, [view1, view2, view3], 'view2 should show');
+		assert.deepEqual(model.viewDescriptors, [view1, view2, view3], 'view2 should show');
+		assert.deepEqual(seq.elements, [view1, view2, view3]);
 
 		ViewsRegistry.deregisterViews([view1.id, view2.id, view3.id], location);
-		assert.deepEqual(model.elements, []);
+		assert.deepEqual(model.viewDescriptors, []);
+		assert.deepEqual(seq.elements, []);
 	});
 
 	test('move', function () {
 		const model = new ContributableViewsModel(location, contextKeyService);
+		const seq = new ViewDescriptorSequence(model);
+
 		const view1: IViewDescriptor = { id: 'view1', ctor: null, location, name: 'Test View 1' };
 		const view2: IViewDescriptor = { id: 'view2', ctor: null, location, name: 'Test View 2' };
 		const view3: IViewDescriptor = { id: 'view3', ctor: null, location, name: 'Test View 3' };
 
 		ViewsRegistry.registerViews([view1, view2, view3]);
-		assert.deepEqual(model.elements, [view1, view2, view3]);
+		assert.deepEqual(model.viewDescriptors, [view1, view2, view3]);
+		assert.deepEqual(seq.elements, [view1, view2, view3]);
 
 		model.move('view3', 'view1');
-		assert.deepEqual(model.elements, [view3, view1, view2], 'view3 should go to the front');
+		assert.deepEqual(model.viewDescriptors, [view3, view1, view2], 'view3 should go to the front');
+		assert.deepEqual(seq.elements, [view3, view1, view2]);
 
 		model.move('view1', 'view2');
-		assert.deepEqual(model.elements, [view3, view2, view1], 'view1 should go to the end');
+		assert.deepEqual(model.viewDescriptors, [view3, view2, view1], 'view1 should go to the end');
+		assert.deepEqual(seq.elements, [view3, view2, view1]);
 
 		model.move('view1', 'view3');
-		assert.deepEqual(model.elements, [view1, view3, view2], 'view1 should go to the front');
+		assert.deepEqual(model.viewDescriptors, [view1, view3, view2], 'view1 should go to the front');
+		assert.deepEqual(seq.elements, [view1, view3, view2]);
 
 		model.move('view2', 'view3');
-		assert.deepEqual(model.elements, [view1, view2, view3], 'view2 should go to the middle');
+		assert.deepEqual(model.viewDescriptors, [view1, view2, view3], 'view2 should go to the middle');
+		assert.deepEqual(seq.elements, [view1, view2, view3]);
 	});
 });