Files: moving 2 dirty files does not report both as dirty (fix #90888)

src/vs/workbench/common/editor.ts

@@ -584,6 +584,11 @@ export abstract class TextResourceEditorInput extends EditorInput {
 	) {
 		super();
 
+		this.registerListeners();
+	}
+
+	protected registerListeners(): void {
+
 		// Clear label memoizer on certain events that have impact
 		this._register(this.labelService.onDidChangeFormatters(() => TextResourceEditorInput.MEMOIZER.clear()));
 		this._register(this.fileService.onDidChangeFileSystemProviderRegistrations(() => TextResourceEditorInput.MEMOIZER.clear()));

src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts

@@ -10,13 +10,15 @@ import { BinaryEditorModel } from 'vs/workbench/common/editor/binaryEditorModel'
 import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files';
 import { ITextFileService, ModelState, LoadReason, TextFileOperationError, TextFileOperationResult, ITextFileEditorModel } from 'vs/workbench/services/textfile/common/textfiles';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
-import { IReference, dispose } from 'vs/base/common/lifecycle';
+import { IReference, dispose, DisposableStore } from 'vs/base/common/lifecycle';
 import { ITextModelService } from 'vs/editor/common/services/resolverService';
 import { FILE_EDITOR_INPUT_ID, TEXT_FILE_EDITOR_ID, BINARY_FILE_EDITOR_ID } from 'vs/workbench/contrib/files/common/files';
 import { ILabelService } from 'vs/platform/label/common/label';
 import { IFilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
 import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
+import { isEqual } from 'vs/base/common/resources';
+import { Event } from 'vs/base/common/event';
 
 const enum ForceOpenAs {
 	None,
@@ -34,8 +36,11 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 
 	private forceOpenAs: ForceOpenAs = ForceOpenAs.None;
 
+	private model: ITextFileEditorModel | undefined = undefined;
 	private cachedTextFileModelReference: IReference<ITextFileEditorModel> | undefined = undefined;
 
+	private modelListeners: DisposableStore = this._register(new DisposableStore());
+
 	constructor(
 		resource: URI,
 		preferredEncoding: string | undefined,
@@ -60,10 +65,48 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 		}
 	}
 
+	protected registerListeners(): void {
+		super.registerListeners();
+
+		// Attach to model that matches our resource once created
+		this._register(this.textFileService.files.onDidCreate(model => this.onDidCreateTextFileModel(model)));
+	}
+
+	private onDidCreateTextFileModel(model: ITextFileEditorModel): void {
+
+		// Once the text file model is created, we keep it inside
+		// the input to be able to implement some methods properly
+		if (isEqual(model.resource, this.resource)) {
+			this.model = model;
+
+			this.registerModelListeners(model);
+		}
+	}
+
+	private registerModelListeners(model: ITextFileEditorModel): void {
+
+		// Clear any old
+		this.modelListeners.clear();
+
+		// re-emit some events from the model
+		this.modelListeners.add(model.onDidChangeDirty(() => this._onDidChangeDirty.fire()));
+		this.modelListeners.add(model.onDidChangeOrphaned(() => this._onDidChangeLabel.fire()));
+
+		// important: treat save errors as potential dirty change because
+		// a file that is in save conflict or error will report dirty even
+		// if auto save is turned on.
+		this.modelListeners.add(model.onDidSaveError(() => this._onDidChangeDirty.fire()));
+
+		// remove model association once it gets disposed
+		Event.once(model.onDispose)(() => {
+			this.modelListeners.clear();
+			this.model = undefined;
+		});
+	}
+
 	getEncoding(): string | undefined {
-		const textModel = this.textFileService.files.get(this.resource);
-		if (textModel) {
-			return textModel.getEncoding();
+		if (this.model) {
+			return this.model.getEncoding();
 		}
 
 		return this.preferredEncoding;
@@ -76,15 +119,14 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 	setEncoding(encoding: string, mode: EncodingMode): void {
 		this.setPreferredEncoding(encoding);
 
-		const textModel = this.textFileService.files.get(this.resource);
-		if (textModel) {
-			textModel.setEncoding(encoding, mode);
-		}
+		this.model?.setEncoding(encoding, mode);
 	}
 
 	setPreferredEncoding(encoding: string): void {
 		this.preferredEncoding = encoding;
-		this.setForceOpenAsText(); // encoding is a good hint to open the file as text
+
+		// encoding is a good hint to open the file as text
+		this.setForceOpenAsText();
 	}
 
 	getPreferredMode(): string | undefined {
@@ -94,15 +136,14 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 	setMode(mode: string): void {
 		this.setPreferredMode(mode);
 
-		const textModel = this.textFileService.files.get(this.resource);
-		if (textModel) {
-			textModel.setMode(mode);
-		}
+		this.model?.setMode(mode);
 	}
 
 	setPreferredMode(mode: string): void {
 		this.preferredMode = mode;
-		this.setForceOpenAsText(); // mode is a good hint to open the file as text
+
+		// mode is a good hint to open the file as text
+		this.setForceOpenAsText();
 	}
 
 	setForceOpenAsText(): void {
@@ -126,9 +167,7 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 	}
 
 	private decorateLabel(label: string): string {
-		const model = this.textFileService.files.get(this.resource);
-
-		if (model?.hasState(ModelState.ORPHAN)) {
+		if (this.model?.hasState(ModelState.ORPHAN)) {
 			return localize('orphanedFile', "{0} (deleted)", label);
 		}
 
@@ -140,21 +179,11 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 	}
 
 	isDirty(): boolean {
-		const model = this.textFileService.files.get(this.resource);
-		if (!model) {
-			return false;
-		}
-
-		return model.isDirty();
+		return !!(this.model?.isDirty());
 	}
 
 	isSaving(): boolean {
-		const model = this.textFileService.files.get(this.resource);
-		if (!model) {
-			return false;
-		}
-
-		if (model.hasState(ModelState.SAVED) || model.hasState(ModelState.CONFLICT) || model.hasState(ModelState.ERROR)) {
+		if (this.model?.hasState(ModelState.SAVED) || this.model?.hasState(ModelState.CONFLICT) || this.model?.hasState(ModelState.ERROR)) {
 			return false; // require the model to be dirty and not in conflict or error state
 		}
 
@@ -198,7 +227,7 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 			// resolve() ensures we are not creating model references for these kind of resources.
 			// In addition we have a bit of payload to take into account (encoding, reload) that the text resolver does not handle yet.
 			if (!this.cachedTextFileModelReference) {
-				this.cachedTextFileModelReference = await this.createTextModelReference();
+				this.cachedTextFileModelReference = await this.textModelResolverService.createModelReference(this.resource) as IReference<ITextFileEditorModel>;
 			}
 
 			return this.cachedTextFileModelReference.object;
@@ -217,38 +246,12 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 		}
 	}
 
-	private async createTextModelReference(): Promise<IReference<ITextFileEditorModel>> {
-		const reference = await this.textModelResolverService.createModelReference(this.resource) as IReference<ITextFileEditorModel>;
-
-		// Fire an initial dirty change if the model is already dirty
-		const model = reference.object;
-		if (model.isDirty()) {
-			this._onDidChangeDirty.fire();
-		}
-
-		this.registerModelListeners(model);
-
-		return reference;
-	}
-
-	private registerModelListeners(model: ITextFileEditorModel): void {
-
-		// re-emit some events from the model
-		this._register(model.onDidChangeDirty(() => this._onDidChangeDirty.fire()));
-		this._register(model.onDidChangeOrphaned(() => this._onDidChangeLabel.fire()));
-
-		// important: treat save errors as potential dirty change because
-		// a file that is in save conflict or error will report dirty even
-		// if auto save is turned on.
-		this._register(model.onDidSaveError(() => this._onDidChangeDirty.fire()));
-	}
-
 	private async doResolveAsBinary(): Promise<BinaryEditorModel> {
 		return this.instantiationService.createInstance(BinaryEditorModel, this.resource, this.getName()).load();
 	}
 
 	isResolved(): boolean {
-		return !!this.textFileService.files.get(this.resource);
+		return !!this.model;
 	}
 
 	matches(otherInput: unknown): boolean {
@@ -265,6 +268,9 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
 
 	dispose(): void {
 
+		// Model
+		this.model = undefined;
+
 		// Model reference
 		dispose(this.cachedTextFileModelReference);
 		this.cachedTextFileModelReference = undefined;

src/vs/workbench/contrib/files/test/browser/fileEditorInput.test.ts

@@ -187,4 +187,24 @@ suite('Files - FileEditorInput', () => {
 		assert.ok(resolved);
 		resolved.dispose();
 	});
+
+	test('attaches to model when created and reports dirty', async function () {
+		const input = instantiationService.createInstance(FileEditorInput, toResource.call(this, '/foo/bar/updatefile.js'), undefined, undefined);
+
+		let listenerCount = 0;
+		const listener = input.onDidChangeDirty(() => {
+			listenerCount++;
+		});
+
+		// instead of going through file input resolve method
+		// we resolve the model directly through the service
+		const model = await accessor.textFileService.files.resolve(input.getResource());
+		model.textEditorModel?.setValue('hello world');
+
+		assert.equal(listenerCount, 1);
+		assert.ok(input.isDirty());
+
+		input.dispose();
+		listener.dispose();
+	});
 });

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

@@ -29,6 +29,9 @@ import { PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry';
 
 export class TextFileEditorModelManager extends Disposable implements ITextFileEditorModelManager {
 
+	private readonly _onDidCreate = this._register(new Emitter<ITextFileEditorModel>());
+	readonly onDidCreate = this._onDidCreate.event;
+
 	private readonly _onDidLoad = this._register(new Emitter<ITextFileModelLoadEvent>());
 	readonly onDidLoad = this._onDidLoad.event;
 
@@ -245,9 +248,9 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
 		}
 
 		let modelPromise: Promise<ITextFileEditorModel>;
+		let model = this.get(resource);
 
 		// Model exists
-		let model = this.get(resource);
 		if (model) {
 			if (options?.reload) {
 
@@ -282,6 +285,9 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
 			listeners.add(model.onDidChangeOrphaned(() => this._onDidChangeOrphaned.fire(newModel)));
 
 			this.mapResourceToModelListeners.set(resource, listeners);
+
+			// Signal as event
+			this._onDidCreate.fire(newModel);
 		}
 
 		// Store pending loads to avoid race conditions
@@ -293,11 +299,6 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
 			// Make known to manager (if not already known)
 			this.add(resource, resolvedModel);
 
-			// Model can be dirty if a backup was restored, so we make sure to have this event delivered
-			if (resolvedModel.isDirty()) {
-				this._onDidChangeDirty.fire(resolvedModel);
-			}
-
 			// Remove from pending loads
 			this.mapResourceToPendingModelLoaders.delete(resource);
 
@@ -306,6 +307,11 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
 				resolvedModel.setMode(options.mode);
 			}
 
+			// Model can be dirty if a backup was restored, so we make sure to have this event delivered
+			if (resolvedModel.isDirty()) {
+				this._onDidChangeDirty.fire(resolvedModel);
+			}
+
 			return resolvedModel;
 		} catch (error) {
 

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

@@ -329,6 +329,7 @@ export interface ITextFileSaveParticipant {
 
 export interface ITextFileEditorModelManager {
 
+	readonly onDidCreate: Event<ITextFileEditorModel>;
 	readonly onDidLoad: Event<ITextFileModelLoadEvent>;
 	readonly onDidChangeDirty: Event<ITextFileEditorModel>;
 	readonly onDidSaveError: Event<ITextFileEditorModel>;

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

@@ -13,6 +13,7 @@ import { IFileService, FileChangesEvent, FileChangeType } from 'vs/platform/file
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { toResource } from 'vs/base/test/common/utils';
 import { ModesRegistry, PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry';
+import { ITextFileEditorModel } from 'vs/workbench/services/textfile/common/textfiles';
 
 class ServiceAccessor {
 	constructor(
@@ -92,6 +93,11 @@ suite('Files - TextFileEditorModelManager', () => {
 		const resource = URI.file('/test.html');
 		const encoding = 'utf8';
 
+		const events: ITextFileEditorModel[] = [];
+		const listener = manager.onDidCreate(model => {
+			events.push(model);
+		});
+
 		const model = await manager.resolve(resource, { encoding });
 		assert.ok(model);
 		assert.equal(model.getEncoding(), encoding);
@@ -105,6 +111,12 @@ suite('Files - TextFileEditorModelManager', () => {
 		assert.notEqual(model3, model2);
 		assert.equal(manager.get(resource), model3);
 		model3.dispose();
+
+		assert.equal(events.length, 2);
+		assert.equal(events[0].resource.toString(), model.resource.toString());
+		assert.equal(events[1].resource.toString(), model2.resource.toString());
+
+		listener.dispose();
 	});
 
 	test('removed from cache when model disposed', function () {