debt - cleanup file model a bit

src/vs/workbench/services/textfile/common/saveSequenzializer.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+interface IPendingSave {
+	versionId: number;
+	promise: Promise<void>;
+}
+
+interface ISaveOperation {
+	promise: Promise<void>;
+	promiseResolve: () => void;
+	promiseReject: (error: Error) => void;
+	run: () => Promise<void>;
+}
+
+export class SaveSequentializer {
+	private _pendingSave?: IPendingSave;
+	private _nextSave?: ISaveOperation;
+
+	hasPendingSave(versionId?: number): boolean {
+		if (!this._pendingSave) {
+			return false;
+		}
+
+		if (typeof versionId === 'number') {
+			return this._pendingSave.versionId === versionId;
+		}
+
+		return !!this._pendingSave;
+	}
+
+	get pendingSave(): Promise<void> | undefined {
+		return this._pendingSave ? this._pendingSave.promise : undefined;
+	}
+
+	setPending(versionId: number, promise: Promise<void>): Promise<void> {
+		this._pendingSave = { versionId, promise };
+
+		promise.then(() => this.donePending(versionId), () => this.donePending(versionId));
+
+		return promise;
+	}
+
+	private donePending(versionId: number): void {
+		if (this._pendingSave && versionId === this._pendingSave.versionId) {
+
+			// only set pending to done if the promise finished that is associated with that versionId
+			this._pendingSave = undefined;
+
+			// schedule the next save now that we are free if we have any
+			this.triggerNextSave();
+		}
+	}
+
+	private triggerNextSave(): void {
+		if (this._nextSave) {
+			const saveOperation = this._nextSave;
+			this._nextSave = undefined;
+
+			// Run next save and complete on the associated promise
+			saveOperation.run().then(saveOperation.promiseResolve, saveOperation.promiseReject);
+		}
+	}
+
+	setNext(run: () => Promise<void>): Promise<void> {
+
+		// this is our first next save, so we create associated promise with it
+		// so that we can return a promise that completes when the save operation
+		// has completed.
+		if (!this._nextSave) {
+			let promiseResolve: () => void;
+			let promiseReject: (error: Error) => void;
+			const promise = new Promise<void>((resolve, reject) => {
+				promiseResolve = resolve;
+				promiseReject = reject;
+			});
+
+			this._nextSave = {
+				run,
+				promise,
+				promiseResolve: promiseResolve!,
+				promiseReject: promiseReject!
+			};
+		}
+
+		// we have a previous next save, just overwrite it
+		else {
+			this._nextSave.run = run;
+		}
+
+		return this._nextSave.promise;
+	}
+}

src/vs/workbench/services/textfile/common/textFileEditorModel.ts

@@ -13,7 +13,6 @@ import { EncodingMode, IRevertOptions, SaveReason } from 'vs/workbench/common/ed
 import { BaseTextEditorModel } from 'vs/workbench/common/editor/textEditorModel';
 import { IBackupFileService } from 'vs/workbench/services/backup/common/backup';
 import { IFileService, FileOperationError, FileOperationResult, FileChangesEvent, FileChangeType, IFileStatWithMetadata, ETAG_DISABLED, FileSystemProviderCapabilities } from 'vs/platform/files/common/files';
-import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IModeService } from 'vs/editor/common/services/modeService';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { timeout } from 'vs/base/common/async';
@@ -24,8 +23,9 @@ import { isEqual, basename } from 'vs/base/common/resources';
 import { onUnexpectedError } from 'vs/base/common/errors';
 import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
 import { IFilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
+import { SaveSequentializer } from 'vs/workbench/services/textfile/common/saveSequenzializer';
 
-export interface IBackupMetaData {
+interface IBackupMetaData {
 	mtime: number;
 	ctime: number;
 	size: number;
@@ -80,7 +80,6 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 		@IModeService modeService: IModeService,
 		@IModelService modelService: IModelService,
 		@IFileService private readonly fileService: IFileService,
-		@IInstantiationService private readonly instantiationService: IInstantiationService,
 		@ITextFileService private readonly textFileService: ITextFileService,
 		@IBackupFileService private readonly backupFileService: IBackupFileService,
 		@ILogService private readonly logService: ILogService,
@@ -755,7 +754,12 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 
 		// Prepare handler
 		if (!TextFileEditorModel.saveErrorHandler) {
-			TextFileEditorModel.setSaveErrorHandler(this.instantiationService.createInstance(DefaultSaveErrorHandler));
+			const notificationService = this.notificationService;
+			TextFileEditorModel.setSaveErrorHandler({
+				onSaveError(error: Error, model: TextFileEditorModel): void {
+					notificationService.error(nls.localize('genericSaveError', "Failed to save '{0}': {1}", basename(model.resource), toErrorMessage(error, false)));
+				}
+			});
 		}
 
 		// Handle
@@ -887,102 +891,4 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 	}
 }
 
-interface IPendingSave {
-	versionId: number;
-	promise: Promise<void>;
-}
-
-interface ISaveOperation {
-	promise: Promise<void>;
-	promiseResolve: () => void;
-	promiseReject: (error: Error) => void;
-	run: () => Promise<void>;
-}
-
-export class SaveSequentializer {
-	private _pendingSave?: IPendingSave;
-	private _nextSave?: ISaveOperation;
-
-	hasPendingSave(versionId?: number): boolean {
-		if (!this._pendingSave) {
-			return false;
-		}
-
-		if (typeof versionId === 'number') {
-			return this._pendingSave.versionId === versionId;
-		}
-
-		return !!this._pendingSave;
-	}
-
-	get pendingSave(): Promise<void> | undefined {
-		return this._pendingSave ? this._pendingSave.promise : undefined;
-	}
-
-	setPending(versionId: number, promise: Promise<void>): Promise<void> {
-		this._pendingSave = { versionId, promise };
-
-		promise.then(() => this.donePending(versionId), () => this.donePending(versionId));
-
-		return promise;
-	}
-
-	private donePending(versionId: number): void {
-		if (this._pendingSave && versionId === this._pendingSave.versionId) {
-
-			// only set pending to done if the promise finished that is associated with that versionId
-			this._pendingSave = undefined;
-
-			// schedule the next save now that we are free if we have any
-			this.triggerNextSave();
-		}
-	}
-
-	private triggerNextSave(): void {
-		if (this._nextSave) {
-			const saveOperation = this._nextSave;
-			this._nextSave = undefined;
-
-			// Run next save and complete on the associated promise
-			saveOperation.run().then(saveOperation.promiseResolve, saveOperation.promiseReject);
-		}
-	}
-
-	setNext(run: () => Promise<void>): Promise<void> {
-
-		// this is our first next save, so we create associated promise with it
-		// so that we can return a promise that completes when the save operation
-		// has completed.
-		if (!this._nextSave) {
-			let promiseResolve: () => void;
-			let promiseReject: (error: Error) => void;
-			const promise = new Promise<void>((resolve, reject) => {
-				promiseResolve = resolve;
-				promiseReject = reject;
-			});
-
-			this._nextSave = {
-				run,
-				promise,
-				promiseResolve: promiseResolve!,
-				promiseReject: promiseReject!
-			};
-		}
-
-		// we have a previous next save, just overwrite it
-		else {
-			this._nextSave.run = run;
-		}
-
-		return this._nextSave.promise;
-	}
-}
-
-class DefaultSaveErrorHandler implements ISaveErrorHandler {
-
-	constructor(@INotificationService private readonly notificationService: INotificationService) { }
 
-	onSaveError(error: Error, model: TextFileEditorModel): void {
-		this.notificationService.error(nls.localize('genericSaveError', "Failed to save '{0}': {1}", basename(model.resource), toErrorMessage(error, false)));
-	}
-}

src/vs/workbench/services/textfile/test/saveSequenzializer.test.ts

+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { timeout } from 'vs/base/common/async';
+import { SaveSequentializer } from 'vs/workbench/services/textfile/common/saveSequenzializer';
+
+suite('Files - SaveSequentializer', () => {
+
+	test('SaveSequentializer - pending basics', async function () {
+		const sequentializer = new SaveSequentializer();
+
+		assert.ok(!sequentializer.hasPendingSave());
+		assert.ok(!sequentializer.hasPendingSave(2323));
+		assert.ok(!sequentializer.pendingSave);
+
+		// pending removes itself after done
+		await sequentializer.setPending(1, Promise.resolve());
+		assert.ok(!sequentializer.hasPendingSave());
+		assert.ok(!sequentializer.hasPendingSave(1));
+		assert.ok(!sequentializer.pendingSave);
+
+		// pending removes itself after done (use timeout)
+		sequentializer.setPending(2, timeout(1));
+		assert.ok(sequentializer.hasPendingSave());
+		assert.ok(sequentializer.hasPendingSave(2));
+		assert.ok(!sequentializer.hasPendingSave(1));
+		assert.ok(sequentializer.pendingSave);
+
+		await timeout(2);
+		assert.ok(!sequentializer.hasPendingSave());
+		assert.ok(!sequentializer.hasPendingSave(2));
+		assert.ok(!sequentializer.pendingSave);
+	});
+
+	test('SaveSequentializer - pending and next (finishes instantly)', async function () {
+		const sequentializer = new SaveSequentializer();
+
+		let pendingDone = false;
+		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
+
+		// next finishes instantly
+		let nextDone = false;
+		const res = sequentializer.setNext(() => Promise.resolve(null).then(() => { nextDone = true; return; }));
+
+		await res;
+		assert.ok(pendingDone);
+		assert.ok(nextDone);
+	});
+
+	test('SaveSequentializer - pending and next (finishes after timeout)', async function () {
+		const sequentializer = new SaveSequentializer();
+
+		let pendingDone = false;
+		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
+
+		// next finishes after timeout
+		let nextDone = false;
+		const res = sequentializer.setNext(() => timeout(1).then(() => { nextDone = true; return; }));
+
+		await res;
+		assert.ok(pendingDone);
+		assert.ok(nextDone);
+	});
+
+	test('SaveSequentializer - pending and multiple next (last one wins)', async function () {
+		const sequentializer = new SaveSequentializer();
+
+		let pendingDone = false;
+		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
+
+		// next finishes after timeout
+		let firstDone = false;
+		let firstRes = sequentializer.setNext(() => timeout(2).then(() => { firstDone = true; return; }));
+
+		let secondDone = false;
+		let secondRes = sequentializer.setNext(() => timeout(3).then(() => { secondDone = true; return; }));
+
+		let thirdDone = false;
+		let thirdRes = sequentializer.setNext(() => timeout(4).then(() => { thirdDone = true; return; }));
+
+		await Promise.all([firstRes, secondRes, thirdRes]);
+		assert.ok(pendingDone);
+		assert.ok(!firstDone);
+		assert.ok(!secondDone);
+		assert.ok(thirdDone);
+	});
+});

src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts

@@ -6,7 +6,7 @@
 import * as assert from 'assert';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { EncodingMode } from 'vs/workbench/common/editor';
-import { TextFileEditorModel, SaveSequentializer } from 'vs/workbench/services/textfile/common/textFileEditorModel';
+import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
 import { ITextFileService, ModelState, StateChange, snapshotToString } from 'vs/workbench/services/textfile/common/textfiles';
 import { workbenchInstantiationService, TestTextFileService, createFileInput, TestFileService } from 'vs/workbench/test/workbenchTestServices';
 import { toResource } from 'vs/base/test/common/utils';
@@ -475,83 +475,4 @@ suite('Files - TextFileEditorModel', () => {
 		await model.save();
 		model.dispose();
 	});
-
-	test('SaveSequentializer - pending basics', async function () {
-		const sequentializer = new SaveSequentializer();
-
-		assert.ok(!sequentializer.hasPendingSave());
-		assert.ok(!sequentializer.hasPendingSave(2323));
-		assert.ok(!sequentializer.pendingSave);
-
-		// pending removes itself after done
-		await sequentializer.setPending(1, Promise.resolve());
-		assert.ok(!sequentializer.hasPendingSave());
-		assert.ok(!sequentializer.hasPendingSave(1));
-		assert.ok(!sequentializer.pendingSave);
-
-		// pending removes itself after done (use timeout)
-		sequentializer.setPending(2, timeout(1));
-		assert.ok(sequentializer.hasPendingSave());
-		assert.ok(sequentializer.hasPendingSave(2));
-		assert.ok(!sequentializer.hasPendingSave(1));
-		assert.ok(sequentializer.pendingSave);
-
-		await timeout(2);
-		assert.ok(!sequentializer.hasPendingSave());
-		assert.ok(!sequentializer.hasPendingSave(2));
-		assert.ok(!sequentializer.pendingSave);
-	});
-
-	test('SaveSequentializer - pending and next (finishes instantly)', async function () {
-		const sequentializer = new SaveSequentializer();
-
-		let pendingDone = false;
-		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
-
-		// next finishes instantly
-		let nextDone = false;
-		const res = sequentializer.setNext(() => Promise.resolve(null).then(() => { nextDone = true; return; }));
-
-		await res;
-		assert.ok(pendingDone);
-		assert.ok(nextDone);
-	});
-
-	test('SaveSequentializer - pending and next (finishes after timeout)', async function () {
-		const sequentializer = new SaveSequentializer();
-
-		let pendingDone = false;
-		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
-
-		// next finishes after timeout
-		let nextDone = false;
-		const res = sequentializer.setNext(() => timeout(1).then(() => { nextDone = true; return; }));
-
-		await res;
-		assert.ok(pendingDone);
-		assert.ok(nextDone);
-	});
-
-	test('SaveSequentializer - pending and multiple next (last one wins)', async function () {
-		const sequentializer = new SaveSequentializer();
-
-		let pendingDone = false;
-		sequentializer.setPending(1, timeout(1).then(() => { pendingDone = true; return; }));
-
-		// next finishes after timeout
-		let firstDone = false;
-		let firstRes = sequentializer.setNext(() => timeout(2).then(() => { firstDone = true; return; }));
-
-		let secondDone = false;
-		let secondRes = sequentializer.setNext(() => timeout(3).then(() => { secondDone = true; return; }));
-
-		let thirdDone = false;
-		let thirdRes = sequentializer.setNext(() => timeout(4).then(() => { thirdDone = true; return; }));
-
-		await Promise.all([firstRes, secondRes, thirdRes]);
-		assert.ok(pendingDone);
-		assert.ok(!firstDone);
-		assert.ok(!secondDone);
-		assert.ok(thirdDone);
-	});
 });

src/vs/workbench/services/textfile/test/textFileEditorModelManager.test.ts

@@ -141,11 +141,18 @@ suite('Files - TextFileEditorModelManager', () => {
 		const resource1 = toResource.call(this, '/path/index.txt');
 		const resource2 = toResource.call(this, '/path/other.txt');
 
+		let loadedCounter = 0;
 		let dirtyCounter = 0;
 		let revertedCounter = 0;
 		let savedCounter = 0;
 		let encodingCounter = 0;
 
+		manager.onModelLoaded(e => {
+			if (e.resource.toString() === resource1.toString()) {
+				loadedCounter++;
+			}
+		});
+
 		manager.onModelDirty(e => {
 			if (e.resource.toString() === resource1.toString()) {
 				dirtyCounter++;
@@ -171,10 +178,14 @@ suite('Files - TextFileEditorModelManager', () => {
 		});
 
 		const model1 = await manager.loadOrCreate(resource1, { encoding: 'utf8' });
+		assert.equal(loadedCounter, 1);
+
 		accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource: resource1, type: FileChangeType.DELETED }]));
 		accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource: resource1, type: FileChangeType.ADDED }]));
 
 		const model2 = await manager.loadOrCreate(resource2, { encoding: 'utf8' });
+		assert.equal(loadedCounter, 2);
+
 		model1.textEditorModel!.setValue('changed');
 		model1.updatePreferredEncoding('utf16');
 

src/vs/workbench/workbench.common.main.ts

@@ -71,7 +71,7 @@ import 'vs/workbench/services/history/browser/history';
 import 'vs/workbench/services/activity/browser/activityService';
 import 'vs/workbench/services/keybinding/browser/keybindingService';
 import 'vs/workbench/services/untitled/common/untitledTextEditorService';
-import 'vs/workbench/services/textfile/common/textResourcePropertiesService';
+import 'vs/workbench/services/textresourceProperties/common/textResourcePropertiesService';
 import 'vs/workbench/services/mode/common/workbenchModeService';
 import 'vs/workbench/services/commands/common/commandService';
 import 'vs/workbench/services/themes/browser/workbenchThemeService';