editors - no need for working copy service getDirty()

src/vs/platform/dialogs/common/dialogs.ts

@@ -242,7 +242,7 @@ export interface IFileDialogService {
 	/**
 	 * Shows a confirm dialog for saving 1-N files.
 	 */
-	showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult>;
+	showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult>;
 
 	/**
 	 * Shows a open file dialog and returns the chosen file URI.
@@ -257,16 +257,16 @@ export const enum ConfirmResult {
 }
 
 const MAX_CONFIRM_FILES = 10;
-export function getConfirmMessage(start: string, resourcesToConfirm: readonly URI[]): string {
+export function getConfirmMessage(start: string, fileNamesOrResources: readonly (string | URI)[]): string {
 	const message = [start];
 	message.push('');
-	message.push(...resourcesToConfirm.slice(0, MAX_CONFIRM_FILES).map(r => basename(r)));
+	message.push(...fileNamesOrResources.slice(0, MAX_CONFIRM_FILES).map(fileNameOrResource => typeof fileNameOrResource === 'string' ? fileNameOrResource : basename(fileNameOrResource)));
 
-	if (resourcesToConfirm.length > MAX_CONFIRM_FILES) {
-		if (resourcesToConfirm.length - MAX_CONFIRM_FILES === 1) {
+	if (fileNamesOrResources.length > MAX_CONFIRM_FILES) {
+		if (fileNamesOrResources.length - MAX_CONFIRM_FILES === 1) {
 			message.push(localize('moreFile', "...1 additional file not shown"));
 		} else {
-			message.push(localize('moreFiles', "...{0} additional files not shown", resourcesToConfirm.length - MAX_CONFIRM_FILES));
+			message.push(localize('moreFiles', "...{0} additional files not shown", fileNamesOrResources.length - MAX_CONFIRM_FILES));
 		}
 	}
 

src/vs/workbench/browser/parts/editor/editorActions.ts

@@ -23,6 +23,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle';
 import { IWorkspacesService } from 'vs/platform/workspaces/common/workspaces';
 import { IFileDialogService, ConfirmResult } from 'vs/platform/dialogs/common/dialogs';
 import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
+import { ResourceMap, values } from 'vs/base/common/map';
 
 export class ExecuteCommandAction extends Action {
 
@@ -639,7 +640,23 @@ export abstract class BaseCloseAllAction extends Action {
 			return undefined;
 		}));
 
-		const confirm = await this.fileDialogService.showSaveConfirm(this.workingCopyService.getDirty().map(copy => copy.resource));
+		const dirtyEditorsToConfirmByName = new Set<string>();
+		const dirtyEditorsToConfirmByResource = new ResourceMap();
+
+		for (const editor of this.editorService.editors) {
+			if (!editor.isDirty()) {
+				continue; // only interested in dirty editors
+			}
+
+			const resource = editor.getResource();
+			if (resource) {
+				dirtyEditorsToConfirmByResource.set(resource, true);
+			} else {
+				dirtyEditorsToConfirmByName.add(editor.getName());
+			}
+		}
+
+		const confirm = await this.fileDialogService.showSaveConfirm([...dirtyEditorsToConfirmByResource.keys(), ...values(dirtyEditorsToConfirmByName)]);
 		if (confirm === ConfirmResult.CANCEL) {
 			return;
 		}

src/vs/workbench/browser/parts/editor/editorGroupView.ts

@@ -1297,7 +1297,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		await this.openEditor(editor);
 
 		const editorResource = toResource(editor, { supportSideBySide: SideBySideEditor.MASTER });
-		const res = await this.fileDialogService.showSaveConfirm(editorResource ? [editorResource] : editor.getName());
+		const res = await this.fileDialogService.showSaveConfirm(editorResource ? [editorResource] : [editor.getName()]);
 
 		// It could be that the editor saved meanwhile, so we check again
 		// to see if anything needs to happen before closing for good.

src/vs/workbench/services/dialogs/browser/abstractFileDialogService.ts

@@ -80,24 +80,24 @@ export abstract class AbstractFileDialogService implements IFileDialogService {
 		return this.defaultFilePath(schemeFilter);
 	}
 
-	async showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult> {
+	async showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
 		if (this.environmentService.isExtensionDevelopment) {
 			return ConfirmResult.DONT_SAVE; // no veto when we are in extension dev mode because we cannot assume we run interactive (e.g. tests)
 		}
 
-		if (Array.isArray(fileNameOrResources) && fileNameOrResources.length === 0) {
+		if (fileNamesOrResources.length === 0) {
 			return ConfirmResult.DONT_SAVE;
 		}
 
 		let message: string;
-		if (typeof fileNameOrResources === 'string' || fileNameOrResources.length === 1) {
-			message = nls.localize('saveChangesMessage', "Do you want to save the changes you made to {0}?", typeof fileNameOrResources === 'string' ? fileNameOrResources : resources.basename(fileNameOrResources[0]));
+		if (fileNamesOrResources.length === 1) {
+			message = nls.localize('saveChangesMessage', "Do you want to save the changes you made to {0}?", typeof fileNamesOrResources[0] === 'string' ? fileNamesOrResources[0] : resources.basename(fileNamesOrResources[0]));
 		} else {
-			message = getConfirmMessage(nls.localize('saveChangesMessages', "Do you want to save the changes to the following {0} files?", fileNameOrResources.length), fileNameOrResources);
+			message = getConfirmMessage(nls.localize('saveChangesMessages', "Do you want to save the changes to the following {0} files?", fileNamesOrResources.length), fileNamesOrResources);
 		}
 
 		const buttons: string[] = [
-			Array.isArray(fileNameOrResources) && fileNameOrResources.length > 1 ? nls.localize({ key: 'saveAll', comment: ['&& denotes a mnemonic'] }, "&&Save All") : nls.localize({ key: 'save', comment: ['&& denotes a mnemonic'] }, "&&Save"),
+			fileNamesOrResources.length > 1 ? nls.localize({ key: 'saveAll', comment: ['&& denotes a mnemonic'] }, "&&Save All") : nls.localize({ key: 'save', comment: ['&& denotes a mnemonic'] }, "&&Save"),
 			nls.localize({ key: 'dontSave', comment: ['&& denotes a mnemonic'] }, "Do&&n't Save"),
 			nls.localize('cancel', "Cancel")
 		];

src/vs/workbench/services/dialogs/electron-browser/fileDialogService.ts

@@ -184,14 +184,14 @@ export class FileDialogService extends AbstractFileDialogService implements IFil
 		return schema === Schemas.untitled ? [Schemas.file] : (schema !== Schemas.file ? [schema, Schemas.file] : [schema]);
 	}
 
-	async showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult> {
+	async showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
 		if (this.environmentService.isExtensionDevelopment) {
 			if (!this.environmentService.args['extension-development-confirm-save']) {
 				return ConfirmResult.DONT_SAVE; // no veto when we are in extension dev mode because we cannot assume we run interactive (e.g. tests)
 			}
 		}
 
-		return super.showSaveConfirm(fileNameOrResources);
+		return super.showSaveConfirm(fileNamesOrResources);
 	}
 }
 

src/vs/workbench/services/workingCopy/common/workingCopyService.ts

@@ -51,8 +51,6 @@ export interface IWorkingCopyService {
 
 	isDirty(resource: URI): boolean;
 
-	getDirty(...resources: URI[]): IWorkingCopy[];
-
 	//#endregion
 
 
@@ -72,34 +70,6 @@ export class WorkingCopyService extends Disposable implements IWorkingCopyServic
 	private readonly _onDidChangeDirty = this._register(new Emitter<IWorkingCopy>());
 	readonly onDidChangeDirty = this._onDidChangeDirty.event;
 
-	getDirty(...resources: URI[]): IWorkingCopy[] {
-		const dirtyWorkingCopies: IWorkingCopy[] = [];
-
-		// Specific resource(s)
-		if (resources.length > 0) {
-			for (const resource of resources) {
-				this.fillDirty(this.mapResourceToWorkingCopy.get(resource.toString()), dirtyWorkingCopies);
-			}
-		}
-
-		// All resources
-		else {
-			this.fillDirty(this.workingCopies, dirtyWorkingCopies);
-		}
-
-		return dirtyWorkingCopies;
-	}
-
-	private fillDirty(workingCopies: Set<IWorkingCopy> | undefined, target: IWorkingCopy[]): void {
-		if (workingCopies) {
-			for (const workingCopy of workingCopies) {
-				if (workingCopy.isDirty()) {
-					target.push(workingCopy);
-				}
-			}
-		}
-	}
-
 	isDirty(resource: URI): boolean {
 		const workingCopies = this.mapResourceToWorkingCopy.get(resource.toString());
 		if (workingCopies) {

src/vs/workbench/services/workingCopy/test/common/workingCopyService.test.ts

@@ -56,8 +56,6 @@ suite('WorkingCopyService', () => {
 
 		assert.equal(service.hasDirty, false);
 		assert.equal(service.dirtyCount, 0);
-		assert.equal(service.getDirty().length, 0);
-		assert.equal(service.getDirty(URI.file('/'), URI.file('/some')).length, 0);
 		assert.equal(service.isDirty(URI.file('/')), false);
 
 		// resource 1
@@ -68,8 +66,6 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.dirtyCount, 0);
 		assert.equal(service.isDirty(resource1), false);
 		assert.equal(service.hasDirty, false);
-		assert.equal(service.getDirty(resource1).length, 0);
-		assert.equal(service.getDirty().length, 0);
 
 		copy1.setDirty(true);
 
@@ -78,8 +74,6 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.hasDirty, true);
 		assert.equal(onDidChangeDirty.length, 1);
 		assert.equal(onDidChangeDirty[0], copy1);
-		assert.equal(service.getDirty(resource1).length, 1);
-		assert.equal(service.getDirty().length, 1);
 
 		copy1.setDirty(false);
 
@@ -88,8 +82,6 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.hasDirty, false);
 		assert.equal(onDidChangeDirty.length, 2);
 		assert.equal(onDidChangeDirty[1], copy1);
-		assert.equal(service.getDirty(resource1).length, 0);
-		assert.equal(service.getDirty().length, 0);
 
 		unregister1.dispose();
 
@@ -101,8 +93,6 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.dirtyCount, 1);
 		assert.equal(service.isDirty(resource2), true);
 		assert.equal(service.hasDirty, true);
-		assert.equal(service.getDirty(resource1, resource2).length, 1);
-		assert.equal(service.getDirty().length, 1);
 
 		assert.equal(onDidChangeDirty.length, 3);
 		assert.equal(onDidChangeDirty[2], copy2);
@@ -112,8 +102,6 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.hasDirty, false);
 		assert.equal(onDidChangeDirty.length, 4);
 		assert.equal(onDidChangeDirty[3], copy2);
-		assert.equal(service.getDirty(resource1, resource2).length, 0);
-		assert.equal(service.getDirty().length, 0);
 	});
 
 	test('registry - multiple copies on same resource', () => {
@@ -135,31 +123,23 @@ suite('WorkingCopyService', () => {
 		assert.equal(service.dirtyCount, 1);
 		assert.equal(onDidChangeDirty.length, 1);
 		assert.equal(service.isDirty(resource), true);
-		assert.equal(service.getDirty(resource).length, 1);
-		assert.equal(service.getDirty().length, 1);
 
 		copy2.setDirty(true);
 
 		assert.equal(service.dirtyCount, 2);
 		assert.equal(onDidChangeDirty.length, 2);
 		assert.equal(service.isDirty(resource), true);
-		assert.equal(service.getDirty(resource).length, 2);
-		assert.equal(service.getDirty().length, 2);
 
 		unregister1.dispose();
 
 		assert.equal(service.dirtyCount, 1);
 		assert.equal(onDidChangeDirty.length, 3);
 		assert.equal(service.isDirty(resource), true);
-		assert.equal(service.getDirty(resource).length, 1);
-		assert.equal(service.getDirty().length, 1);
 
 		unregister2.dispose();
 
 		assert.equal(service.dirtyCount, 0);
 		assert.equal(onDidChangeDirty.length, 4);
 		assert.equal(service.isDirty(resource), false);
-		assert.equal(service.getDirty(resource).length, 0);
-		assert.equal(service.getDirty().length, 0);
 	});
 });

src/vs/workbench/test/workbenchTestServices.ts

@@ -444,7 +444,7 @@ export class TestFileDialogService implements IFileDialogService {
 	public setConfirmResult(result: ConfirmResult): void {
 		this.confirmResult = result;
 	}
-	public showSaveConfirm(resources: string | URI[]): Promise<ConfirmResult> {
+	public showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
 		return Promise.resolve(this.confirmResult);
 	}
 }