From e2e55d63684976ad7f25085a9405579f88e20611 Mon Sep 17 00:00:00 2001
From: Benjamin Pasero <benjpas@microsoft.com>
Date: Wed, 20 Nov 2019 08:36:49 +0100
Subject: [PATCH] fix #85180

---
 .../textfile/browser/textFileService.ts       | 31 ++++++++-----------
 .../services/textfile/common/textfiles.ts     |  2 +-
 .../textfile/test/textFileService.test.ts     |  2 +-
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts
index 6228f117813..d0fb5b46abd 100644
--- a/src/vs/workbench/services/textfile/browser/textFileService.ts
+++ b/src/vs/workbench/services/textfile/browser/textFileService.ts
@@ -239,7 +239,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 		if (confirm === ConfirmResult.SAVE) {
 			const result = await this.saveAll(true /* includeUntitled */, { skipSaveParticipants: true });
 
-			if (result.results.some(r => !r.success)) {
+			if (result.results.some(r => r.error)) {
 				return true; // veto if some saves failed
 			}
 
@@ -492,9 +492,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 			}
 		}
 
-		const result = await this.saveAll([resource], options);
-
-		return result.results.length === 1 && !!result.results[0].success;
+		return !(await this.saveAll([resource], options)).results.some(result => result.error);
 	}
 
 	saveAll(includeUntitled?: boolean, options?: ITextFileSaveOptions): Promise<ITextFileOperationResult>;
@@ -560,7 +558,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 			result.results.push({
 				source: untitledResources[index],
 				target: uri,
-				success: !!uri
+				error: !uri // the operation was canceled or failed, so mark as error
 			});
 		}));
 
@@ -648,10 +646,11 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 		await Promise.all(dirtyFileModels.map(async model => {
 			await model.save(options);
 
-			if (!model.isDirty()) {
+			// If model is still dirty, mark the resulting operation as error
+			if (model.isDirty()) {
 				const result = mapResourceToResult.get(model.resource);
 				if (result) {
-					result.success = true;
+					result.error = true;
 				}
 			}
 		}));
@@ -838,9 +837,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 	}
 
 	async revert(resource: URI, options?: IRevertOptions): Promise<boolean> {
-		const result = await this.revertAll([resource], options);
-
-		return result.results.length === 1 && !!result.results[0].success;
+		return !(await this.revertAll([resource], options)).results.some(result => result.error);
 	}
 
 	async revertAll(resources?: URI[], options?: IRevertOptions): Promise<ITextFileOperationResult> {
@@ -850,7 +847,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 
 		// Revert untitled
 		const untitledReverted = this.untitledTextEditorService.revertAll(resources);
-		untitledReverted.forEach(untitled => revertOperationResult.results.push({ source: untitled, success: true }));
+		untitledReverted.forEach(untitled => revertOperationResult.results.push({ source: untitled }));
 
 		return revertOperationResult;
 	}
@@ -869,20 +866,18 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 			try {
 				await model.revert(options);
 
-				if (!model.isDirty()) {
+				// If model is still dirty, mark the resulting operation as error
+				if (model.isDirty()) {
 					const result = mapResourceToResult.get(model.resource);
 					if (result) {
-						result.success = true;
+						result.error = true;
 					}
 				}
 			} catch (error) {
 
-				// FileNotFound means the file got deleted meanwhile, so still record as successful revert
+				// FileNotFound means the file got deleted meanwhile, so ignore it
 				if ((<FileOperationError>error).fileOperationResult === FileOperationResult.FILE_NOT_FOUND) {
-					const result = mapResourceToResult.get(model.resource);
-					if (result) {
-						result.success = true;
-					}
+					return;
 				}
 
 				// Otherwise bubble up the error
diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts
index a1383b93901..f2eef06db52 100644
--- a/src/vs/workbench/services/textfile/common/textfiles.ts
+++ b/src/vs/workbench/services/textfile/common/textfiles.ts
@@ -309,7 +309,7 @@ export interface ITextFileOperationResult {
 export interface IResult {
 	source: URI;
 	target?: URI;
-	success?: boolean;
+	error?: boolean;
 }
 
 export const enum LoadReason {
diff --git a/src/vs/workbench/services/textfile/test/textFileService.test.ts b/src/vs/workbench/services/textfile/test/textFileService.test.ts
index db7e2bd8033..e0bb57aac5b 100644
--- a/src/vs/workbench/services/textfile/test/textFileService.test.ts
+++ b/src/vs/workbench/services/textfile/test/textFileService.test.ts
@@ -205,7 +205,7 @@ suite('Files - TextFileService', () => {
 		const res = await accessor.textFileService.saveAll(true);
 		assert.ok(loadOrCreateStub.calledOnce);
 		assert.equal(res.results.length, 1);
-		assert.ok(res.results[0].success);
+		assert.ok(!res.results[0].error);
 		assert.equal(res.results[0].target!.scheme, Schemas.file);
 		assert.equal(res.results[0].target!.authority, untitledUncUri.authority);
 		assert.equal(res.results[0].target!.path, untitledUncUri.path);
-- 
GitLab