From 419744928a3cb7cc5207ca46a6aef75bc0ac39b8 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 8 Nov 2021 19:35:22 +0100 Subject: [PATCH] editors - await closing of editor before opening to the side (fix #122363) (#136602) --- .../api/browser/mainThreadEditorTabs.ts | 2 +- .../api/browser/mainThreadEditors.ts | 3 ++- .../browser/parts/editor/editorActions.ts | 8 ++++--- .../browser/parts/editor/editorGroupView.ts | 4 ++-- .../editor/browser/editorResolverService.ts | 23 +++++++++++++------ .../editor/common/editorGroupsService.ts | 6 +++-- .../test/browser/editorGroupsService.test.ts | 15 ++++++++---- .../test/browser/workbenchTestServices.ts | 2 +- 8 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadEditorTabs.ts b/src/vs/workbench/api/browser/mainThreadEditorTabs.ts index a6fd0b055f2..8c78575c99f 100644 --- a/src/vs/workbench/api/browser/mainThreadEditorTabs.ts +++ b/src/vs/workbench/api/browser/mainThreadEditorTabs.ts @@ -280,7 +280,7 @@ export class MainThreadEditorTabs { if (!editor) { return; } - return group.closeEditor(editor); + await group.closeEditor(editor); } //#endregion } diff --git a/src/vs/workbench/api/browser/mainThreadEditors.ts b/src/vs/workbench/api/browser/mainThreadEditors.ts index d458ccae1ac..99fe9d8e332 100644 --- a/src/vs/workbench/api/browser/mainThreadEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadEditors.ts @@ -177,7 +177,8 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { const editorPanes = this._editorService.visibleEditorPanes; for (let editorPane of editorPanes) { if (mainThreadEditor.matches(editorPane)) { - return editorPane.group.closeEditor(editorPane.input); + await editorPane.group.closeEditor(editorPane.input); + return; } } } diff --git a/src/vs/workbench/browser/parts/editor/editorActions.ts b/src/vs/workbench/browser/parts/editor/editorActions.ts index 3b221a22be1..5724e89dfc9 100644 --- a/src/vs/workbench/browser/parts/editor/editorActions.ts +++ b/src/vs/workbench/browser/parts/editor/editorActions.ts @@ -460,13 +460,15 @@ export class CloseOneEditorAction extends Action { if (typeof editorIndex === 'number') { const editorAtIndex = group.getEditorByIndex(editorIndex); if (editorAtIndex) { - return group.closeEditor(editorAtIndex); + await group.closeEditor(editorAtIndex); + return; } } // Otherwise close active editor in group if (group.activeEditor) { - return group.closeEditor(group.activeEditor); + await group.closeEditor(group.activeEditor); + return; } } } @@ -501,7 +503,7 @@ export class RevertAndCloseEditorAction extends Action { await this.editorService.revert({ editor, groupId: group.id }, { soft: true }); } - return group.closeEditor(editor); + await group.closeEditor(editor); } } } diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index a1e60ea3fb3..411f50891eb 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -1372,8 +1372,8 @@ export class EditorGroupView extends Themable implements IEditorGroupView { //#region closeEditor() - async closeEditor(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions): Promise { - await this.doCloseEditorWithDirtyHandling(editor, options); + async closeEditor(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions): Promise { + return this.doCloseEditorWithDirtyHandling(editor, options); } private async doCloseEditorWithDirtyHandling(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions, internalOptions?: IInternalEditorCloseOptions): Promise { diff --git a/src/vs/workbench/services/editor/browser/editorResolverService.ts b/src/vs/workbench/services/editor/browser/editorResolverService.ts index 6a3714379a0..fe5f390dfb4 100644 --- a/src/vs/workbench/services/editor/browser/editorResolverService.ts +++ b/src/vs/workbench/services/editor/browser/editorResolverService.ts @@ -463,20 +463,23 @@ export class EditorResolverService extends Disposable implements IEditorResolver // If the editor states it can only be opened once per resource we must close all existing ones first const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource; if (singleEditorPerResource) { - this.closeExistingEditorsForResource(resource, selectedEditor.editorInfo.id, group); + const closed = await this.closeExistingEditorsForResource(resource, selectedEditor.editorInfo.id, group); + if (!closed) { + return undefined; + } } return { editor: input, options }; } - private closeExistingEditorsForResource( + private async closeExistingEditorsForResource( resource: URI, viewType: string, targetGroup: IEditorGroup, - ): void { + ): Promise { const editorInfoForResource = this.findExistingEditorsForResource(resource, viewType); if (!editorInfoForResource.length) { - return; + return true; } const editorToUse = editorInfoForResource[0]; @@ -484,14 +487,20 @@ export class EditorResolverService extends Disposable implements IEditorResolver // Replace all other editors for (const { editor, group } of editorInfoForResource) { if (editor !== editorToUse.editor) { - group.closeEditor(editor); + const closed = await group.closeEditor(editor); + if (!closed) { + return false; + } } } if (targetGroup.id !== editorToUse.group.id) { - editorToUse.group.closeEditor(editorToUse.editor); + const closed = await editorToUse.group.closeEditor(editorToUse.editor); + if (!closed) { + return false; + } } - return; + return true; } /** diff --git a/src/vs/workbench/services/editor/common/editorGroupsService.ts b/src/vs/workbench/services/editor/common/editorGroupsService.ts index 4aa219be99b..a0ec7b7eb71 100644 --- a/src/vs/workbench/services/editor/common/editorGroupsService.ts +++ b/src/vs/workbench/services/editor/common/editorGroupsService.ts @@ -654,9 +654,11 @@ export interface IEditorGroup { * @param editor the editor to close, or the currently active editor * if unspecified. * - * @returns a promise when the editor is closed. + * @returns a promise when the editor is closed or not. If `true`, the editor + * is closed and if `false` there was a veto closing the editor, e.g. when it + * is dirty. */ - closeEditor(editor?: EditorInput, options?: ICloseEditorOptions): Promise; + closeEditor(editor?: EditorInput, options?: ICloseEditorOptions): Promise; /** * Closes specific editors in this group. This may trigger a confirmation dialog if diff --git a/src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts b/src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts index d668eded170..e4de03f00cf 100644 --- a/src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts +++ b/src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts @@ -490,7 +490,8 @@ suite('EditorGroupsService', () => { assert.strictEqual(group.activeEditor, inputInactive); await group.openEditor(input); - await group.closeEditor(inputInactive); + const closed = await group.closeEditor(inputInactive); + assert.strictEqual(closed, true); assert.strictEqual(activeEditorChangeCounter, 3); assert.strictEqual(editorCloseCounter, 1); @@ -553,12 +554,14 @@ suite('EditorGroupsService', () => { await group.openEditor(input); accessor.fileDialogService.setConfirmResult(ConfirmResult.CANCEL); - await group.closeEditor(input); + let closed = await group.closeEditor(input); + assert.strictEqual(closed, false); assert.ok(!input.gotDisposed); accessor.fileDialogService.setConfirmResult(ConfirmResult.DONT_SAVE); - await group.closeEditor(input); + closed = await group.closeEditor(input); + assert.strictEqual(closed, true); assert.ok(input.gotDisposed); }); @@ -576,11 +579,13 @@ suite('EditorGroupsService', () => { await group.openEditors([{ editor: input, options: { pinned: true } }, { editor: inputInactive }]); await rightGroup.openEditors([{ editor: input, options: { pinned: true } }, { editor: inputInactive }]); - await rightGroup.closeEditor(input); + let closed = await rightGroup.closeEditor(input); + assert.strictEqual(closed, true); assert.ok(!input.gotDisposed); - await group.closeEditor(input); + closed = await group.closeEditor(input); + assert.strictEqual(closed, true); assert.ok(input.gotDisposed); }); diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index d78d69a0612..c655f96a87c 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -821,7 +821,7 @@ export class TestEditorGroupView implements IEditorGroupView { moveEditors(_editors: EditorInputWithOptions[], _target: IEditorGroup): void { } copyEditor(_editor: EditorInput, _target: IEditorGroup, _options?: IEditorOptions): void { } copyEditors(_editors: EditorInputWithOptions[], _target: IEditorGroup): void { } - async closeEditor(_editor?: EditorInput, options?: ICloseEditorOptions): Promise { } + async closeEditor(_editor?: EditorInput, options?: ICloseEditorOptions): Promise { return true; } async closeEditors(_editors: EditorInput[] | ICloseEditorsFilter, options?: ICloseEditorOptions): Promise { } async closeAllEditors(options?: ICloseAllEditorsOptions): Promise { } async replaceEditors(_editors: IEditorReplacement[]): Promise { }