Undo after external modification does not turn editor dirty (fixes #33580)

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

@@ -408,8 +408,10 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 	private doUpdateTextModel(value: string | IRawTextSource): TPromise<TextFileEditorModel> {
 		diag('load() - updated text editor model', this.resource, new Date());
 
-		this.setDirty(false); // Ensure we are not tracking a stale state
+		// Ensure we are not tracking a stale state
+		this.setDirty(false);
 
+		// Update model value in a block that ignores model content change events
 		this.blockModelContentChange = true;
 		try {
 			this.updateTextEditorModel(value);
@@ -417,6 +419,9 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 			this.blockModelContentChange = false;
 		}
 
+		// Ensure we track the latest saved version ID given that the contents changed
+		this.updateSavedVersionId();
+
 		return TPromise.as<TextFileEditorModel>(this);
 	}
 
@@ -767,15 +772,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 			this.dirty = false;
 			this.inConflictMode = false;
 			this.inErrorMode = false;
-
-			// we remember the models alternate version id to remember when the version
-			// of the model matches with the saved version on disk. we need to keep this
-			// in order to find out if the model changed back to a saved version (e.g.
-			// when undoing long enough to reach to a version that is saved and then to
-			// clear the dirty flag)
-			if (this.textEditorModel) {
-				this.bufferSavedVersionId = this.textEditorModel.getAlternativeVersionId();
-			}
+			this.updateSavedVersionId();
 		} else {
 			this.dirty = true;
 		}
@@ -789,6 +786,17 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 		};
 	}
 
+	private updateSavedVersionId(): void {
+		// we remember the models alternate version id to remember when the version
+		// of the model matches with the saved version on disk. we need to keep this
+		// in order to find out if the model changed back to a saved version (e.g.
+		// when undoing long enough to reach to a version that is saved and then to
+		// clear the dirty flag)
+		if (this.textEditorModel) {
+			this.bufferSavedVersionId = this.textEditorModel.getAlternativeVersionId();
+		}
+	}
+
 	private updateLastResolvedDiskStat(newVersionOnDiskStat: IFileStat): void {
 
 		// First resolve - just take

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

@@ -32,15 +32,18 @@ suite('Files - TextFileEditorModel', () => {
 
 	let instantiationService: IInstantiationService;
 	let accessor: ServiceAccessor;
+	let content: string;
 
 	setup(() => {
 		instantiationService = workbenchInstantiationService();
 		accessor = instantiationService.createInstance(ServiceAccessor);
+		content = accessor.fileService.getContent();
 	});
 
 	teardown(() => {
 		(<TextFileEditorModelManager>accessor.textFileService.models).clear();
 		TextFileEditorModel.setSaveParticipant(null); // reset any set participant
+		accessor.fileService.setContent(content);
 	});
 
 	test('Save', function (done) {
@@ -190,6 +193,20 @@ suite('Files - TextFileEditorModel', () => {
 		}, error => onError(error, done));
 	});
 
+	test('Load and undo turns model dirty', function (done) {
+		const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8');
+		model.load().done(() => {
+			accessor.fileService.setContent('Hello Change');
+			model.load().done(() => {
+				model.textEditorModel.undo();
+
+				assert.ok(model.isDirty());
+
+				done();
+			});
+		}, error => onError(error, done));
+	});
+
 	test('File not modified error is handled gracefully', function (done) {
 		const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8');
 

src/vs/workbench/test/workbenchTestServices.ts

@@ -631,11 +631,21 @@ export class TestFileService implements IFileService {
 	private _onFileChanges: Emitter<FileChangesEvent>;
 	private _onAfterOperation: Emitter<FileOperationEvent>;
 
+	private content = 'Hello Html';
+
 	constructor() {
 		this._onFileChanges = new Emitter<FileChangesEvent>();
 		this._onAfterOperation = new Emitter<FileOperationEvent>();
 	}
 
+	public setContent(content: string): void {
+		this.content = content;
+	}
+
+	public getContent(): string {
+		return this.content;
+	}
+
 	public get onFileChanges(): Event<FileChangesEvent> {
 		return this._onFileChanges.event;
 	}
@@ -675,7 +685,7 @@ export class TestFileService implements IFileService {
 	resolveContent(resource: URI, options?: IResolveContentOptions): TPromise<IContent> {
 		return TPromise.as({
 			resource: resource,
-			value: 'Hello Html',
+			value: this.content,
 			etag: 'index.txt',
 			encoding: 'utf8',
 			mtime: Date.now(),
@@ -689,7 +699,7 @@ export class TestFileService implements IFileService {
 			value: {
 				on: (event: string, callback: Function): void => {
 					if (event === 'data') {
-						callback('Hello Html');
+						callback(this.content);
 					}
 					if (event === 'end') {
 						callback();