Skip to content

Commit 07ad685

Browse files
author
Benjamin Pasero
committed
editors - no need for working copy service getDirty()
1 parent daa77e1 commit 07ad685

8 files changed

Lines changed: 34 additions & 67 deletions

File tree

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ export interface IFileDialogService {
242242
/**
243243
* Shows a confirm dialog for saving 1-N files.
244244
*/
245-
showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult>;
245+
showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult>;
246246

247247
/**
248248
* Shows a open file dialog and returns the chosen file URI.
@@ -257,16 +257,16 @@ export const enum ConfirmResult {
257257
}
258258

259259
const MAX_CONFIRM_FILES = 10;
260-
export function getConfirmMessage(start: string, resourcesToConfirm: readonly URI[]): string {
260+
export function getConfirmMessage(start: string, fileNamesOrResources: readonly (string | URI)[]): string {
261261
const message = [start];
262262
message.push('');
263-
message.push(...resourcesToConfirm.slice(0, MAX_CONFIRM_FILES).map(r => basename(r)));
263+
message.push(...fileNamesOrResources.slice(0, MAX_CONFIRM_FILES).map(fileNameOrResource => typeof fileNameOrResource === 'string' ? fileNameOrResource : basename(fileNameOrResource)));
264264

265-
if (resourcesToConfirm.length > MAX_CONFIRM_FILES) {
266-
if (resourcesToConfirm.length - MAX_CONFIRM_FILES === 1) {
265+
if (fileNamesOrResources.length > MAX_CONFIRM_FILES) {
266+
if (fileNamesOrResources.length - MAX_CONFIRM_FILES === 1) {
267267
message.push(localize('moreFile', "...1 additional file not shown"));
268268
} else {
269-
message.push(localize('moreFiles', "...{0} additional files not shown", resourcesToConfirm.length - MAX_CONFIRM_FILES));
269+
message.push(localize('moreFiles', "...{0} additional files not shown", fileNamesOrResources.length - MAX_CONFIRM_FILES));
270270
}
271271
}
272272

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle';
2323
import { IWorkspacesService } from 'vs/platform/workspaces/common/workspaces';
2424
import { IFileDialogService, ConfirmResult } from 'vs/platform/dialogs/common/dialogs';
2525
import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
26+
import { ResourceMap, values } from 'vs/base/common/map';
2627

2728
export class ExecuteCommandAction extends Action {
2829

@@ -639,7 +640,23 @@ export abstract class BaseCloseAllAction extends Action {
639640
return undefined;
640641
}));
641642

642-
const confirm = await this.fileDialogService.showSaveConfirm(this.workingCopyService.getDirty().map(copy => copy.resource));
643+
const dirtyEditorsToConfirmByName = new Set<string>();
644+
const dirtyEditorsToConfirmByResource = new ResourceMap();
645+
646+
for (const editor of this.editorService.editors) {
647+
if (!editor.isDirty()) {
648+
continue; // only interested in dirty editors
649+
}
650+
651+
const resource = editor.getResource();
652+
if (resource) {
653+
dirtyEditorsToConfirmByResource.set(resource, true);
654+
} else {
655+
dirtyEditorsToConfirmByName.add(editor.getName());
656+
}
657+
}
658+
659+
const confirm = await this.fileDialogService.showSaveConfirm([...dirtyEditorsToConfirmByResource.keys(), ...values(dirtyEditorsToConfirmByName)]);
643660
if (confirm === ConfirmResult.CANCEL) {
644661
return;
645662
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1297,7 +1297,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
12971297
await this.openEditor(editor);
12981298

12991299
const editorResource = toResource(editor, { supportSideBySide: SideBySideEditor.MASTER });
1300-
const res = await this.fileDialogService.showSaveConfirm(editorResource ? [editorResource] : editor.getName());
1300+
const res = await this.fileDialogService.showSaveConfirm(editorResource ? [editorResource] : [editor.getName()]);
13011301

13021302
// It could be that the editor saved meanwhile, so we check again
13031303
// to see if anything needs to happen before closing for good.

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,24 +80,24 @@ export abstract class AbstractFileDialogService implements IFileDialogService {
8080
return this.defaultFilePath(schemeFilter);
8181
}
8282

83-
async showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult> {
83+
async showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
8484
if (this.environmentService.isExtensionDevelopment) {
8585
return ConfirmResult.DONT_SAVE; // no veto when we are in extension dev mode because we cannot assume we run interactive (e.g. tests)
8686
}
8787

88-
if (Array.isArray(fileNameOrResources) && fileNameOrResources.length === 0) {
88+
if (fileNamesOrResources.length === 0) {
8989
return ConfirmResult.DONT_SAVE;
9090
}
9191

9292
let message: string;
93-
if (typeof fileNameOrResources === 'string' || fileNameOrResources.length === 1) {
94-
message = nls.localize('saveChangesMessage', "Do you want to save the changes you made to {0}?", typeof fileNameOrResources === 'string' ? fileNameOrResources : resources.basename(fileNameOrResources[0]));
93+
if (fileNamesOrResources.length === 1) {
94+
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]));
9595
} else {
96-
message = getConfirmMessage(nls.localize('saveChangesMessages', "Do you want to save the changes to the following {0} files?", fileNameOrResources.length), fileNameOrResources);
96+
message = getConfirmMessage(nls.localize('saveChangesMessages', "Do you want to save the changes to the following {0} files?", fileNamesOrResources.length), fileNamesOrResources);
9797
}
9898

9999
const buttons: string[] = [
100-
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"),
100+
fileNamesOrResources.length > 1 ? nls.localize({ key: 'saveAll', comment: ['&& denotes a mnemonic'] }, "&&Save All") : nls.localize({ key: 'save', comment: ['&& denotes a mnemonic'] }, "&&Save"),
101101
nls.localize({ key: 'dontSave', comment: ['&& denotes a mnemonic'] }, "Do&&n't Save"),
102102
nls.localize('cancel', "Cancel")
103103
];

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,14 @@ export class FileDialogService extends AbstractFileDialogService implements IFil
184184
return schema === Schemas.untitled ? [Schemas.file] : (schema !== Schemas.file ? [schema, Schemas.file] : [schema]);
185185
}
186186

187-
async showSaveConfirm(fileNameOrResources: string | URI[]): Promise<ConfirmResult> {
187+
async showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
188188
if (this.environmentService.isExtensionDevelopment) {
189189
if (!this.environmentService.args['extension-development-confirm-save']) {
190190
return ConfirmResult.DONT_SAVE; // no veto when we are in extension dev mode because we cannot assume we run interactive (e.g. tests)
191191
}
192192
}
193193

194-
return super.showSaveConfirm(fileNameOrResources);
194+
return super.showSaveConfirm(fileNamesOrResources);
195195
}
196196
}
197197

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ export interface IWorkingCopyService {
5151

5252
isDirty(resource: URI): boolean;
5353

54-
getDirty(...resources: URI[]): IWorkingCopy[];
55-
5654
//#endregion
5755

5856

@@ -72,34 +70,6 @@ export class WorkingCopyService extends Disposable implements IWorkingCopyServic
7270
private readonly _onDidChangeDirty = this._register(new Emitter<IWorkingCopy>());
7371
readonly onDidChangeDirty = this._onDidChangeDirty.event;
7472

75-
getDirty(...resources: URI[]): IWorkingCopy[] {
76-
const dirtyWorkingCopies: IWorkingCopy[] = [];
77-
78-
// Specific resource(s)
79-
if (resources.length > 0) {
80-
for (const resource of resources) {
81-
this.fillDirty(this.mapResourceToWorkingCopy.get(resource.toString()), dirtyWorkingCopies);
82-
}
83-
}
84-
85-
// All resources
86-
else {
87-
this.fillDirty(this.workingCopies, dirtyWorkingCopies);
88-
}
89-
90-
return dirtyWorkingCopies;
91-
}
92-
93-
private fillDirty(workingCopies: Set<IWorkingCopy> | undefined, target: IWorkingCopy[]): void {
94-
if (workingCopies) {
95-
for (const workingCopy of workingCopies) {
96-
if (workingCopy.isDirty()) {
97-
target.push(workingCopy);
98-
}
99-
}
100-
}
101-
}
102-
10373
isDirty(resource: URI): boolean {
10474
const workingCopies = this.mapResourceToWorkingCopy.get(resource.toString());
10575
if (workingCopies) {

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ suite('WorkingCopyService', () => {
5656

5757
assert.equal(service.hasDirty, false);
5858
assert.equal(service.dirtyCount, 0);
59-
assert.equal(service.getDirty().length, 0);
60-
assert.equal(service.getDirty(URI.file('/'), URI.file('/some')).length, 0);
6159
assert.equal(service.isDirty(URI.file('/')), false);
6260

6361
// resource 1
@@ -68,8 +66,6 @@ suite('WorkingCopyService', () => {
6866
assert.equal(service.dirtyCount, 0);
6967
assert.equal(service.isDirty(resource1), false);
7068
assert.equal(service.hasDirty, false);
71-
assert.equal(service.getDirty(resource1).length, 0);
72-
assert.equal(service.getDirty().length, 0);
7369

7470
copy1.setDirty(true);
7571

@@ -78,8 +74,6 @@ suite('WorkingCopyService', () => {
7874
assert.equal(service.hasDirty, true);
7975
assert.equal(onDidChangeDirty.length, 1);
8076
assert.equal(onDidChangeDirty[0], copy1);
81-
assert.equal(service.getDirty(resource1).length, 1);
82-
assert.equal(service.getDirty().length, 1);
8377

8478
copy1.setDirty(false);
8579

@@ -88,8 +82,6 @@ suite('WorkingCopyService', () => {
8882
assert.equal(service.hasDirty, false);
8983
assert.equal(onDidChangeDirty.length, 2);
9084
assert.equal(onDidChangeDirty[1], copy1);
91-
assert.equal(service.getDirty(resource1).length, 0);
92-
assert.equal(service.getDirty().length, 0);
9385

9486
unregister1.dispose();
9587

@@ -101,8 +93,6 @@ suite('WorkingCopyService', () => {
10193
assert.equal(service.dirtyCount, 1);
10294
assert.equal(service.isDirty(resource2), true);
10395
assert.equal(service.hasDirty, true);
104-
assert.equal(service.getDirty(resource1, resource2).length, 1);
105-
assert.equal(service.getDirty().length, 1);
10696

10797
assert.equal(onDidChangeDirty.length, 3);
10898
assert.equal(onDidChangeDirty[2], copy2);
@@ -112,8 +102,6 @@ suite('WorkingCopyService', () => {
112102
assert.equal(service.hasDirty, false);
113103
assert.equal(onDidChangeDirty.length, 4);
114104
assert.equal(onDidChangeDirty[3], copy2);
115-
assert.equal(service.getDirty(resource1, resource2).length, 0);
116-
assert.equal(service.getDirty().length, 0);
117105
});
118106

119107
test('registry - multiple copies on same resource', () => {
@@ -135,31 +123,23 @@ suite('WorkingCopyService', () => {
135123
assert.equal(service.dirtyCount, 1);
136124
assert.equal(onDidChangeDirty.length, 1);
137125
assert.equal(service.isDirty(resource), true);
138-
assert.equal(service.getDirty(resource).length, 1);
139-
assert.equal(service.getDirty().length, 1);
140126

141127
copy2.setDirty(true);
142128

143129
assert.equal(service.dirtyCount, 2);
144130
assert.equal(onDidChangeDirty.length, 2);
145131
assert.equal(service.isDirty(resource), true);
146-
assert.equal(service.getDirty(resource).length, 2);
147-
assert.equal(service.getDirty().length, 2);
148132

149133
unregister1.dispose();
150134

151135
assert.equal(service.dirtyCount, 1);
152136
assert.equal(onDidChangeDirty.length, 3);
153137
assert.equal(service.isDirty(resource), true);
154-
assert.equal(service.getDirty(resource).length, 1);
155-
assert.equal(service.getDirty().length, 1);
156138

157139
unregister2.dispose();
158140

159141
assert.equal(service.dirtyCount, 0);
160142
assert.equal(onDidChangeDirty.length, 4);
161143
assert.equal(service.isDirty(resource), false);
162-
assert.equal(service.getDirty(resource).length, 0);
163-
assert.equal(service.getDirty().length, 0);
164144
});
165145
});

‎src/vs/workbench/test/workbenchTestServices.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ export class TestFileDialogService implements IFileDialogService {
444444
public setConfirmResult(result: ConfirmResult): void {
445445
this.confirmResult = result;
446446
}
447-
public showSaveConfirm(resources: string | URI[]): Promise<ConfirmResult> {
447+
public showSaveConfirm(fileNamesOrResources: (string | URI)[]): Promise<ConfirmResult> {
448448
return Promise.resolve(this.confirmResult);
449449
}
450450
}

0 commit comments

Comments
 (0)