Editor: load changes to file in the background if file already loaded (fixes microsoft#53532)

Benjamin Pasero · Benjamin Pasero · commit 2902a13bfc81 · 2018-07-04T14:53:25.000+02:00
diff --git a/src/vs/workbench/browser/parts/editor/binaryEditor.ts b/src/vs/workbench/browser/parts/editor/binaryEditor.ts
@@ -73,7 +73,7 @@ export abstract class BaseBinaryResourceEditor extends BaseEditor {
 
 	setInput(input: EditorInput, options: EditorOptions, token: CancellationToken): Thenable<void> {
 		return super.setInput(input, options, token).then(() => {
-			return input.resolve(true).then(model => {
+			return input.resolve().then(model => {
 
 				// Check for cancellation
 				if (token.isCancellationRequested) {
diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts
@@ -104,7 +104,7 @@ export class TextDiffEditor extends BaseTextEditor implements ITextDiffEditor {
 
 		// Set input and resolve
 		return super.setInput(input, options, token).then(() => {
-			return input.resolve(true).then(resolvedModel => {
+			return input.resolve().then(resolvedModel => {
 
 				// Check for cancellation
 				if (token.isCancellationRequested) {
diff --git a/src/vs/workbench/browser/parts/editor/textResourceEditor.ts b/src/vs/workbench/browser/parts/editor/textResourceEditor.ts
@@ -61,7 +61,7 @@ export class AbstractTextResourceEditor extends BaseTextEditor {
 
 		// Set input and resolve
 		return super.setInput(input, options, token).then(() => {
-			return input.resolve(true).then((resolvedModel: EditorModel) => {
+			return input.resolve().then((resolvedModel: EditorModel) => {
 
 				// Check for cancellation
 				if (token.isCancellationRequested) {
diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts
@@ -395,11 +395,9 @@ export abstract class EditorInput extends Disposable implements IEditorInput {
 
 	/**
 	 * Returns a type of EditorModel that represents the resolved input. Subclasses should
-	 * override to provide a meaningful model. The optional second argument allows to specify
-	 * if the EditorModel should be refreshed before returning it. Depending on the implementation
-	 * this could mean to refresh the editor model contents with the version from disk.
+	 * override to provide a meaningful model.
 	 */
-	abstract resolve(refresh?: boolean): TPromise<IEditorModel>;
+	abstract resolve(): TPromise<IEditorModel>;
 
 	/**
 	 * An editor that is dirty will be asked to be saved once it closes.
@@ -582,7 +580,7 @@ export class SideBySideEditorInput extends EditorInput {
 		this._register(this.master.onDidChangeLabel(() => this._onDidChangeLabel.fire()));
 	}
 
-	resolve(refresh?: boolean): TPromise<EditorModel> {
+	resolve(): TPromise<EditorModel> {
 		return TPromise.as(null);
 	}
 
diff --git a/src/vs/workbench/common/editor/dataUriEditorInput.ts b/src/vs/workbench/common/editor/dataUriEditorInput.ts
@@ -65,7 +65,7 @@ export class DataUriEditorInput extends EditorInput {
 		return this.description;
 	}
 
-	resolve(refresh?: boolean): TPromise<BinaryEditorModel> {
+	resolve(): TPromise<BinaryEditorModel> {
 		return this.instantiationService.createInstance(BinaryEditorModel, this.resource, this.getName()).load().then(m => m as BinaryEditorModel);
 	}
 
diff --git a/src/vs/workbench/common/editor/diffEditorInput.ts b/src/vs/workbench/common/editor/diffEditorInput.ts
@@ -36,23 +36,13 @@ export class DiffEditorInput extends SideBySideEditorInput {
 		return this.master;
 	}
 
-	resolve(refresh?: boolean): TPromise<EditorModel> {
-		let modelPromise: TPromise<EditorModel>;
-
-		// Use Cached Model
-		if (this.cachedModel && !refresh) {
-			modelPromise = TPromise.as<EditorModel>(this.cachedModel);
-		}
+	resolve(): TPromise<EditorModel> {
 
 		// Create Model - we never reuse our cached model if refresh is true because we cannot
 		// decide for the inputs within if the cached model can be reused or not. There may be
 		// inputs that need to be loaded again and thus we always recreate the model and dispose
 		// the previous one - if any.
-		else {
-			modelPromise = this.createModel(refresh);
-		}
-
-		return modelPromise.then((resolvedModel: DiffEditorModel) => {
+		return this.createModel().then(resolvedModel => {
 			if (this.cachedModel) {
 				this.cachedModel.dispose();
 			}
@@ -71,9 +61,9 @@ export class DiffEditorInput extends SideBySideEditorInput {
 
 		// Join resolve call over two inputs and build diff editor model
 		return TPromise.join([
-			this.originalInput.resolve(refresh),
-			this.modifiedInput.resolve(refresh)
-		]).then((models) => {
+			this.originalInput.resolve(),
+			this.modifiedInput.resolve()
+		]).then(models => {
 			const originalEditorModel = models[0];
 			const modifiedEditorModel = models[1];
 
diff --git a/src/vs/workbench/common/editor/resourceEditorInput.ts b/src/vs/workbench/common/editor/resourceEditorInput.ts
@@ -82,7 +82,7 @@ export class ResourceEditorInput extends EditorInput {
 		return descriptor;
 	}
 
-	resolve(refresh?: boolean): TPromise<ITextEditorModel> {
+	resolve(): TPromise<ITextEditorModel> {
 		if (!this.modelReference) {
 			this.modelReference = this.textModelResolverService.createModelReference(this.resource);
 		}
diff --git a/src/vs/workbench/parts/extensions/common/extensionsInput.ts b/src/vs/workbench/parts/extensions/common/extensionsInput.ts
@@ -44,7 +44,7 @@ export class ExtensionsInput extends EditorInput {
 		return this.extension === otherExtensionInput.extension;
 	}
 
-	resolve(refresh?: boolean): TPromise<any> {
+	resolve(): TPromise<any> {
 		return TPromise.as(null);
 	}
 
diff --git a/src/vs/workbench/parts/extensions/electron-browser/runtimeExtensionsEditor.ts b/src/vs/workbench/parts/extensions/electron-browser/runtimeExtensionsEditor.ts
@@ -443,7 +443,7 @@ export class RuntimeExtensionsInput extends EditorInput {
 		return true;
 	}
 
-	resolve(refresh?: boolean): TPromise<any> {
+	resolve(): TPromise<any> {
 		return TPromise.as(null);
 	}
 
diff --git a/src/vs/workbench/parts/files/browser/editors/fileEditorTracker.ts b/src/vs/workbench/parts/files/browser/editors/fileEditorTracker.ts
@@ -6,12 +6,11 @@
 
 import { TPromise } from 'vs/base/common/winjs.base';
 import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
-import * as errors from 'vs/base/common/errors';
 import URI from 'vs/base/common/uri';
 import * as paths from 'vs/base/common/paths';
 import { IEditorViewState } from 'vs/editor/common/editorCommon';
 import { toResource, SideBySideEditorInput, IWorkbenchEditorConfiguration } from 'vs/workbench/common/editor';
-import { ITextFileService, ITextFileEditorModel } from 'vs/workbench/services/textfile/common/textfiles';
+import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
 import { FileOperationEvent, FileOperation, IFileService, FileChangeType, FileChangesEvent } from 'vs/platform/files/common/files';
 import { FileEditorInput } from 'vs/workbench/parts/files/common/editors/fileEditorInput';
 import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
@@ -20,7 +19,6 @@ import { distinct } from 'vs/base/common/arrays';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { isLinux } from 'vs/base/common/platform';
-import { ResourceQueue } from 'vs/base/common/async';
 import { ResourceMap } from 'vs/base/common/map';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { isCodeEditor } from 'vs/editor/browser/editorBrowser';
@@ -34,7 +32,6 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 
 	protected closeOnFileDelete: boolean;
 
-	private modelLoadQueue: ResourceQueue;
 	private activeOutOfWorkspaceWatchers: ResourceMap<URI>;
 
 	constructor(
@@ -50,7 +47,6 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 	) {
 		super();
 
-		this.modelLoadQueue = new ResourceQueue();
 		this.activeOutOfWorkspaceWatchers = new ResourceMap<URI>();
 
 		this.onConfigurationUpdated(configurationService.getValue<IWorkbenchEditorConfiguration>());
@@ -101,7 +97,7 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 					})
 					.filter(model => model && !model.isDirty()),
 				m => m.getResource().toString()
-			).forEach(model => this.queueModelLoad(model));
+			).forEach(model => this.textFileService.models.reload(model));
 		}
 	}
 
@@ -125,7 +121,9 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 	private onFileChanges(e: FileChangesEvent): void {
 
 		// Handle updates
-		this.handleUpdates(e);
+		if (e.gotAdded() || e.gotUpdated()) {
+			this.handleUpdates(e);
+		}
 
 		// Handle deletes
 		if (e.gotDeleted()) {
@@ -286,11 +284,23 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 
 	private handleUpdates(e: FileChangesEvent): void {
 
+		// Handle updates to text models
+		this.handleUpdatesToTextModels(e);
+
 		// Handle updates to visible binary editors
 		this.handleUpdatesToVisibleBinaryEditors(e);
+	}
 
-		// Handle updates to text models
-		this.handleUpdatesToTextModels(e);
+	private handleUpdatesToTextModels(e: FileChangesEvent): void {
+
+		// Collect distinct (saved) models to update.
+		//
+		// Note: we also consider the added event because it could be that a file was added
+		// and updated right after.
+		distinct([...e.getUpdated(), ...e.getAdded()]
+			.map(u => this.textFileService.models.get(u.resource))
+			.filter(model => model && !model.isDirty()), m => m.getResource().toString())
+			.forEach(model => this.textFileService.models.reload(model));
 	}
 
 	private handleUpdatesToVisibleBinaryEditors(e: FileChangesEvent): void {
@@ -313,29 +323,6 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 		});
 	}
 
-	private handleUpdatesToTextModels(e: FileChangesEvent): void {
-
-		// Collect distinct (saved) models to update.
-		//
-		// Note: we also consider the added event because it could be that a file was added
-		// and updated right after.
-		distinct([...e.getUpdated(), ...e.getAdded()]
-			.map(u => this.textFileService.models.get(u.resource))
-			.filter(model => model && !model.isDirty()), m => m.getResource().toString())
-			.forEach(model => this.queueModelLoad(model));
-	}
-
-	private queueModelLoad(model: ITextFileEditorModel): void {
-
-		// Load model to update (use a queue to prevent accumulation of loads
-		// when the load actually takes long. At most we only want the queue
-		// to have a size of 2 (1 running load and 1 queued load).
-		const queue = this.modelLoadQueue.queueFor(model.getResource());
-		if (queue.size <= 1) {
-			queue.queue(() => model.load().then(null, errors.onUnexpectedError));
-		}
-	}
-
 	private handleOutOfWorkspaceWatchers(): void {
 		const visibleOutOfWorkspacePaths = new ResourceMap<URI>();
 		this.editorService.visibleEditors.map(editorInput => {
diff --git a/src/vs/workbench/parts/files/browser/editors/textFileEditor.ts b/src/vs/workbench/parts/files/browser/editors/textFileEditor.ts
@@ -104,7 +104,7 @@ export class TextFileEditor extends BaseTextEditor {
 
 		// Set input and resolve
 		return super.setInput(input, options, token).then(() => {
-			return input.resolve(true).then(resolvedModel => {
+			return input.resolve().then(resolvedModel => {
 
 				// Check for cancellation
 				if (token.isCancellationRequested) {
diff --git a/src/vs/workbench/parts/files/common/editors/fileEditorInput.ts b/src/vs/workbench/parts/files/common/editors/fileEditorInput.ts
@@ -243,21 +243,25 @@ export class FileEditorInput extends EditorInput implements IFileEditorInput {
 		return this.forceOpenAsBinary ? BINARY_FILE_EDITOR_ID : TEXT_FILE_EDITOR_ID;
 	}
 
-	resolve(refresh?: boolean): TPromise<TextFileEditorModel | BinaryEditorModel> {
+	resolve(): TPromise<TextFileEditorModel | BinaryEditorModel> {
 
 		// Resolve as binary
 		if (this.forceOpenAsBinary) {
 			return this.doResolveAsBinary();
 		}
 
 		// Resolve as text
-		return this.doResolveAsText(refresh);
+		return this.doResolveAsText();
 	}
 
-	private doResolveAsText(reload?: boolean): TPromise<TextFileEditorModel | BinaryEditorModel> {
+	private doResolveAsText(): TPromise<TextFileEditorModel | BinaryEditorModel> {
 
 		// Resolve as text
-		return this.textFileService.models.loadOrCreate(this.resource, { encoding: this.preferredEncoding, reload, allowBinary: this.forceOpenAsText }).then(model => {
+		return this.textFileService.models.loadOrCreate(this.resource, {
+			encoding: this.preferredEncoding,
+			reload: { async: true }, // trigger a reload of the model if it exists already but do not wait to show the model
+			allowBinary: this.forceOpenAsText
+		}).then(model => {
 
 			// This is a bit ugly, because we first resolve the model and then resolve a model reference. the reason being that binary
 			// or very large files do not resolve to a text file model but should be opened as binary files without text. First calling into
diff --git a/src/vs/workbench/parts/files/test/browser/fileEditorInput.test.ts b/src/vs/workbench/parts/files/test/browser/fileEditorInput.test.ts
@@ -63,37 +63,32 @@ suite('Files - FileEditorInput', () => {
 		const inputToResolve: FileEditorInput = instantiationService.createInstance(FileEditorInput, toResource(this, '/foo/bar/file.js'), void 0);
 		const sameOtherInput: FileEditorInput = instantiationService.createInstance(FileEditorInput, toResource(this, '/foo/bar/file.js'), void 0);
 
-		return inputToResolve.resolve(true).then(resolved => {
+		return inputToResolve.resolve().then(resolved => {
 			assert.ok(inputToResolve.isResolved());
 
 			const resolvedModelA = resolved;
-			return inputToResolve.resolve(true).then(resolved => {
+			return inputToResolve.resolve().then(resolved => {
 				assert(resolvedModelA === resolved); // OK: Resolved Model cached globally per input
 
-				return sameOtherInput.resolve(true).then(otherResolved => {
+				return sameOtherInput.resolve().then(otherResolved => {
 					assert(otherResolved === resolvedModelA); // OK: Resolved Model cached globally per input
 
 					inputToResolve.dispose();
 
-					return inputToResolve.resolve(true).then(resolved => {
+					return inputToResolve.resolve().then(resolved => {
 						assert(resolvedModelA === resolved); // Model is still the same because we had 2 clients
 
 						inputToResolve.dispose();
 						sameOtherInput.dispose();
 
 						resolvedModelA.dispose();
 
-						return inputToResolve.resolve(true).then(resolved => {
+						return inputToResolve.resolve().then(resolved => {
 							assert(resolvedModelA !== resolved); // Different instance, because input got disposed
 
 							let stat = (resolved as TextFileEditorModel).getStat();
-							return inputToResolve.resolve(true).then(resolved => {
+							return inputToResolve.resolve().then(resolved => {
 								assert(stat !== (resolved as TextFileEditorModel).getStat()); // Different stat, because resolve always goes to the server for refresh
-
-								stat = (resolved as TextFileEditorModel).getStat();
-								return inputToResolve.resolve(false).then(resolved => {
-									assert(stat === (resolved as TextFileEditorModel).getStat()); // Same stat, because not refreshed
-								});
 							});
 						});
 					});
@@ -122,7 +117,7 @@ suite('Files - FileEditorInput', () => {
 		input.setEncoding('utf16', EncodingMode.Encode);
 		assert.equal(input.getEncoding(), 'utf16');
 
-		return input.resolve(true).then((resolved: TextFileEditorModel) => {
+		return input.resolve().then((resolved: TextFileEditorModel) => {
 			assert.equal(input.getEncoding(), resolved.getEncoding());
 
 			resolved.dispose();
@@ -132,7 +127,7 @@ suite('Files - FileEditorInput', () => {
 	test('save', function () {
 		const input = instantiationService.createInstance(FileEditorInput, toResource(this, '/foo/bar/updatefile.js'), void 0);
 
-		return input.resolve(true).then((resolved: TextFileEditorModel) => {
+		return input.resolve().then((resolved: TextFileEditorModel) => {
 			resolved.textEditorModel.setValue('changed');
 			assert.ok(input.isDirty());
 
@@ -147,7 +142,7 @@ suite('Files - FileEditorInput', () => {
 	test('revert', function () {
 		const input = instantiationService.createInstance(FileEditorInput, toResource(this, '/foo/bar/updatefile.js'), void 0);
 
-		return input.resolve(true).then((resolved: TextFileEditorModel) => {
+		return input.resolve().then((resolved: TextFileEditorModel) => {
 			resolved.textEditorModel.setValue('changed');
 			assert.ok(input.isDirty());
 
@@ -164,7 +159,7 @@ suite('Files - FileEditorInput', () => {
 
 		accessor.textFileService.setResolveTextContentErrorOnce(new FileOperationError('error', FileOperationResult.FILE_IS_BINARY));
 
-		return input.resolve(true).then(resolved => {
+		return input.resolve().then(resolved => {
 			assert.ok(resolved);
 
 			resolved.dispose();
@@ -176,7 +171,7 @@ suite('Files - FileEditorInput', () => {
 
 		accessor.textFileService.setResolveTextContentErrorOnce(new FileOperationError('error', FileOperationResult.FILE_TOO_LARGE));
 
-		return input.resolve(true).then(resolved => {
+		return input.resolve().then(resolved => {
 			assert.ok(resolved);
 
 			resolved.dispose();
diff --git a/src/vs/workbench/parts/webview/electron-browser/webviewEditorInput.ts b/src/vs/workbench/parts/webview/electron-browser/webviewEditorInput.ts
@@ -154,7 +154,7 @@ export class WebviewEditorInput extends EditorInput {
 		}
 	}
 
-	public resolve(refresh?: boolean): TPromise<IEditorModel, any> {
+	public resolve(): TPromise<IEditorModel, any> {
 		if (this.reviver && !this._revived) {
 			this._revived = true;
 			return this.reviver.reviveWebview(this).then(() => new EditorModel());
diff --git a/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts b/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts
@@ -259,7 +259,7 @@ export class WalkThroughPart extends BaseEditor {
 
 		return super.setInput(input, options, token)
 			.then(() => {
-				return input.resolve(true);
+				return input.resolve();
 			})
 			.then(model => {
 				if (token.isCancellationRequested) {
diff --git a/src/vs/workbench/parts/welcome/walkThrough/node/walkThroughInput.ts b/src/vs/workbench/parts/welcome/walkThrough/node/walkThroughInput.ts
@@ -102,7 +102,7 @@ export class WalkThroughInput extends EditorInput {
 		return this.options.onReady;
 	}
 
-	resolve(refresh?: boolean): TPromise<WalkThroughModel> {
+	resolve(): TPromise<WalkThroughModel> {
 		if (!this.promise) {
 			this.promise = this.textModelResolverService.createModelReference(this.options.resource)
 				.then(ref => {
diff --git a/src/vs/workbench/services/preferences/common/preferencesEditorInput.ts b/src/vs/workbench/services/preferences/common/preferencesEditorInput.ts
diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts b/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts
diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts
diff --git a/src/vs/workbench/services/textfile/test/textFileEditorModelManager.test.ts b/src/vs/workbench/services/textfile/test/textFileEditorModelManager.test.ts
diff --git a/src/vs/workbench/test/browser/parts/editor/baseEditor.test.ts b/src/vs/workbench/test/browser/parts/editor/baseEditor.test.ts
diff --git a/src/vs/workbench/test/common/editor/editor.test.ts b/src/vs/workbench/test/common/editor/editor.test.ts
diff --git a/src/vs/workbench/test/common/editor/editorDiffModel.test.ts b/src/vs/workbench/test/common/editor/editorDiffModel.test.ts
diff --git a/src/vs/workbench/test/common/editor/editorInput.test.ts b/src/vs/workbench/test/common/editor/editorInput.test.ts

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ export class DataUriEditorInput extends EditorInput {`
`65`	`65`	`return this.description;`
`66`	`66`	`}`
`67`	`67`
`68`		`- resolve(refresh?: boolean): TPromise<BinaryEditorModel> {`
	`68`	`+ resolve(): TPromise<BinaryEditorModel> {`
`69`	`69`	`return this.instantiationService.createInstance(BinaryEditorModel, this.resource, this.getName()).load().then(m => m as BinaryEditorModel);`
`70`	`70`	`}`
`71`	`71`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ export class ResourceEditorInput extends EditorInput {`
`82`	`82`	`return descriptor;`
`83`	`83`	`}`
`84`	`84`
`85`		`- resolve(refresh?: boolean): TPromise<ITextEditorModel> {`
	`85`	`+ resolve(): TPromise<ITextEditorModel> {`
`86`	`86`	`if (!this.modelReference) {`
`87`	`87`	`this.modelReference = this.textModelResolverService.createModelReference(this.resource);`
`88`	`88`	`}`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ export class ExtensionsInput extends EditorInput {`
`44`	`44`	`return this.extension === otherExtensionInput.extension;`
`45`	`45`	`}`
`46`	`46`
`47`		`- resolve(refresh?: boolean): TPromise<any> {`
	`47`	`+ resolve(): TPromise<any> {`
`48`	`48`	`return TPromise.as(null);`
`49`	`49`	`}`
`50`	`50`
Original file line number	Diff line number	Diff line change
`@@ -443,7 +443,7 @@ export class RuntimeExtensionsInput extends EditorInput {`
`443`	`443`	`return true;`
`444`	`444`	`}`
`445`	`445`
`446`		`- resolve(refresh?: boolean): TPromise<any> {`
	`446`	`+ resolve(): TPromise<any> {`
`447`	`447`	`return TPromise.as(null);`
`448`	`448`	`}`
`449`	`449`