Skip to content

Commit 89ef869

Browse files
author
Benjamin Pasero
committed
editors - dispose editors on close unless the same instance is opened in another group
1 parent 8a17a70 commit 89ef869

6 files changed

Lines changed: 34 additions & 59 deletions

File tree

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -526,15 +526,19 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
526526
const editorsToClose = [editor];
527527

528528
// Include both sides of side by side editors when being closed and not opened multiple times
529-
if (editor instanceof SideBySideEditorInput && !this.accessor.groups.some(groupView => groupView.group.contains(editor))) {
529+
if (editor instanceof SideBySideEditorInput && !this.accessor.groups.some(groupView => groupView.group.contains(editor, { strictEquals: true }))) {
530530
editorsToClose.push(editor.master, editor.details);
531531
}
532532

533-
// Forward close to editor input for handling within
533+
// Dispose the editor if it is the last opened one
534+
// including diff editors
534535
editorsToClose.forEach(editorToClose => {
535-
const openedInOtherGroups = this.accessor.groups.some(groupView => groupView.group.contains(editorToClose, true /* include side by side editor master & details */));
536-
537-
editorToClose.close(this._group.id, openedInOtherGroups);
536+
if (!this.accessor.groups.some(groupView => groupView.group.contains(editorToClose, {
537+
strictEquals: true, // only if this input is not shared across editor groups
538+
searchInSideBySideEditors: true // include side by side editor master & details
539+
}))) {
540+
editorToClose.dispose();
541+
}
538542
});
539543

540544
/* __GDPR__

‎src/vs/workbench/common/editor.ts‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -477,16 +477,6 @@ export interface IEditorInput extends IDisposable {
477477
*/
478478
move(group: GroupIdentifier, target: URI): IMoveResult | undefined;
479479

480-
/**
481-
* Called when this input was closed in a group. The second parameter
482-
* is a hint wether the editor is still opened in other groups. This
483-
* may include normal editors as well as side-by-side or diff editors.
484-
*
485-
* Subclasses can override what should happen. By default, an editor
486-
* input will dispose when it is closed.
487-
*/
488-
close(group: GroupIdentifier, openedInOtherGroups: boolean): void;
489-
490480
/**
491481
* Subclasses can set this to false if it does not make sense to split the editor input.
492482
*/
@@ -596,16 +586,6 @@ export abstract class EditorInput extends Disposable implements IEditorInput {
596586
return undefined;
597587
}
598588

599-
close(group: GroupIdentifier, openedInOtherGroups: boolean): void {
600-
// TODO@ben revisit this behaviour, should just dispose by default after adoption
601-
// However this requires that we never open the same input in multiple editor groups
602-
// which today we cannot enforce (e.g. when opening the same editor in an empty
603-
// group via quick open editor history)
604-
if (!openedInOtherGroups) {
605-
this.dispose();
606-
}
607-
}
608-
609589
supportsSplitEditor(): boolean {
610590
return true;
611591
}

‎src/vs/workbench/common/editor/editorGroup.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -689,14 +689,14 @@ export class EditorGroup extends Disposable {
689689
return [this.editors[index], index];
690690
}
691691

692-
contains(candidate: EditorInput, searchInSideBySideEditors?: boolean): boolean {
692+
contains(candidate: EditorInput, options?: { searchInSideBySideEditors?: boolean, strictEquals?: boolean }): boolean {
693693
for (const editor of this.editors) {
694-
if (this.matches(editor, candidate)) {
694+
if (this.matches(editor, candidate, options?.strictEquals)) {
695695
return true;
696696
}
697697

698-
if (searchInSideBySideEditors && editor instanceof SideBySideEditorInput) {
699-
if (this.matches(editor.master, candidate) || this.matches(editor.details, candidate)) {
698+
if (options?.searchInSideBySideEditors && editor instanceof SideBySideEditorInput) {
699+
if (this.matches(editor.master, candidate, options?.strictEquals) || this.matches(editor.details, candidate, options?.strictEquals)) {
700700
return true;
701701
}
702702
}
@@ -705,11 +705,15 @@ export class EditorGroup extends Disposable {
705705
return false;
706706
}
707707

708-
private matches(editor: IEditorInput | null, candidate: IEditorInput | null): boolean {
708+
private matches(editor: IEditorInput | null, candidate: IEditorInput | null, strictEquals?: boolean): boolean {
709709
if (!editor || !candidate) {
710710
return false;
711711
}
712712

713+
if (strictEquals) {
714+
return editor === candidate;
715+
}
716+
713717
return editor.matches(candidate);
714718
}
715719

‎src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -449,9 +449,7 @@ suite('EditorGroupsService', () => {
449449
assert.equal(editorCloseCounter1, 1);
450450
assert.equal(editorWillCloseCounter, 1);
451451

452-
assert.ok(inputInactive.gotClosed);
453-
assert.equal(inputInactive.gotClosed?.group, group.id);
454-
assert.equal(inputInactive.gotClosed?.openedInOtherGroups, false);
452+
assert.ok(inputInactive.gotDisposed);
455453

456454
assert.equal(group.activeEditor, input);
457455

@@ -487,12 +485,8 @@ suite('EditorGroupsService', () => {
487485

488486
await group.closeEditors([input, inputInactive]);
489487

490-
assert.ok(input.gotClosed);
491-
assert.equal(input.gotClosed?.group, group.id);
492-
assert.equal(input.gotClosed?.openedInOtherGroups, false);
493-
assert.ok(inputInactive.gotClosed);
494-
assert.equal(inputInactive.gotClosed?.group, group.id);
495-
assert.equal(inputInactive.gotClosed?.openedInOtherGroups, false);
488+
assert.ok(input.gotDisposed);
489+
assert.ok(inputInactive.gotDisposed);
496490

497491
assert.equal(group.isEmpty, true);
498492
part.dispose();
@@ -513,15 +507,11 @@ suite('EditorGroupsService', () => {
513507

514508
await rightGroup.closeEditor(input);
515509

516-
assert.ok(input.gotClosed);
517-
assert.equal(input.gotClosed?.group, rightGroup.id);
518-
assert.equal(input.gotClosed?.openedInOtherGroups, true);
510+
assert.ok(!input.gotDisposed);
519511

520512
await group.closeEditor(input);
521513

522-
assert.ok(input.gotClosed);
523-
assert.equal(input.gotClosed?.group, group.id);
524-
assert.equal(input.gotClosed?.openedInOtherGroups, false);
514+
assert.ok(input.gotDisposed);
525515
});
526516

527517
test('closeEditors (except one)', async () => {

‎src/vs/workbench/test/browser/parts/editor/editorGroups.test.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,11 @@ suite('Workbench editor groups', () => {
277277
group.openEditor(input1, { pinned: true, active: true });
278278

279279
assert.equal(group.contains(input1), true);
280-
assert.equal(group.contains(input1, true), true);
280+
assert.equal(group.contains(input1, { strictEquals: true }), true);
281+
assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
281282
assert.equal(group.contains(input2), false);
282-
assert.equal(group.contains(input2, true), false);
283+
assert.equal(group.contains(input2, { strictEquals: true }), false);
284+
assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
283285
assert.equal(group.contains(diffInput1), false);
284286
assert.equal(group.contains(diffInput2), false);
285287

@@ -307,35 +309,35 @@ suite('Workbench editor groups', () => {
307309
group.closeEditor(input1);
308310

309311
assert.equal(group.contains(input1), false);
310-
assert.equal(group.contains(input1, true), true);
312+
assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
311313
assert.equal(group.contains(input2), true);
312314
assert.equal(group.contains(diffInput1), true);
313315
assert.equal(group.contains(diffInput2), true);
314316

315317
group.closeEditor(input2);
316318

317319
assert.equal(group.contains(input1), false);
318-
assert.equal(group.contains(input1, true), true);
320+
assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
319321
assert.equal(group.contains(input2), false);
320-
assert.equal(group.contains(input2, true), true);
322+
assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
321323
assert.equal(group.contains(diffInput1), true);
322324
assert.equal(group.contains(diffInput2), true);
323325

324326
group.closeEditor(diffInput1);
325327

326328
assert.equal(group.contains(input1), false);
327-
assert.equal(group.contains(input1, true), true);
329+
assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
328330
assert.equal(group.contains(input2), false);
329-
assert.equal(group.contains(input2, true), true);
331+
assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
330332
assert.equal(group.contains(diffInput1), false);
331333
assert.equal(group.contains(diffInput2), true);
332334

333335
group.closeEditor(diffInput2);
334336

335337
assert.equal(group.contains(input1), false);
336-
assert.equal(group.contains(input1, true), false);
338+
assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), false);
337339
assert.equal(group.contains(input2), false);
338-
assert.equal(group.contains(input2, true), false);
340+
assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
339341
assert.equal(group.contains(diffInput1), false);
340342
assert.equal(group.contains(diffInput2), false);
341343

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,6 @@ export class TestFileEditorInput extends EditorInput implements IFileEditorInput
10601060
gotSaved = false;
10611061
gotSavedAs = false;
10621062
gotReverted = false;
1063-
gotClosed: { group: GroupIdentifier, openedInOtherGroups: boolean } | undefined = undefined;
10641063
dirty = false;
10651064
private fails = false;
10661065

@@ -1107,10 +1106,6 @@ export class TestFileEditorInput extends EditorInput implements IFileEditorInput
11071106
return false;
11081107
}
11091108
isResolved(): boolean { return false; }
1110-
close(group: GroupIdentifier, openedInOtherGroups: boolean): void {
1111-
this.gotClosed = { group, openedInOtherGroups };
1112-
super.close(group, openedInOtherGroups);
1113-
}
11141109
dispose(): void {
11151110
super.dispose();
11161111
this.gotDisposed = true;

0 commit comments

Comments
 (0)