editors - dispose editors on close unless the same instance is opened in another group

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

@@ -526,15 +526,19 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		const editorsToClose = [editor];
 
 		// Include both sides of side by side editors when being closed and not opened multiple times
-		if (editor instanceof SideBySideEditorInput && !this.accessor.groups.some(groupView => groupView.group.contains(editor))) {
+		if (editor instanceof SideBySideEditorInput && !this.accessor.groups.some(groupView => groupView.group.contains(editor, { strictEquals: true }))) {
 			editorsToClose.push(editor.master, editor.details);
 		}
 
-		// Forward close to editor input for handling within
+		// Dispose the editor if it is the last opened one
+		// including diff editors
 		editorsToClose.forEach(editorToClose => {
-			const openedInOtherGroups = this.accessor.groups.some(groupView => groupView.group.contains(editorToClose, true /* include side by side editor master & details */));
-
-			editorToClose.close(this._group.id, openedInOtherGroups);
+			if (!this.accessor.groups.some(groupView => groupView.group.contains(editorToClose, {
+				strictEquals: true,				// only if this input is not shared across editor groups
+				searchInSideBySideEditors: true // include side by side editor master & details
+			}))) {
+				editorToClose.dispose();
+			}
 		});
 
 		/* __GDPR__

src/vs/workbench/common/editor.ts

@@ -477,16 +477,6 @@ export interface IEditorInput extends IDisposable {
 	 */
 	move(group: GroupIdentifier, target: URI): IMoveResult | undefined;
 
-	/**
-	 * Called when this input was closed in a group. The second parameter
-	 * is a hint wether the editor is still opened in other groups. This
-	 * may include normal editors as well as side-by-side or diff editors.
-	 *
-	 * Subclasses can override what should happen. By default, an editor
-	 * input will dispose when it is closed.
-	 */
-	close(group: GroupIdentifier, openedInOtherGroups: boolean): void;
-
 	/**
 	 * Subclasses can set this to false if it does not make sense to split the editor input.
 	 */
@@ -596,16 +586,6 @@ export abstract class EditorInput extends Disposable implements IEditorInput {
 		return undefined;
 	}
 
-	close(group: GroupIdentifier, openedInOtherGroups: boolean): void {
-		// TODO@ben revisit this behaviour, should just dispose by default after adoption
-		// However this requires that we never open the same input in multiple editor groups
-		// which today we cannot enforce (e.g. when opening the same editor in an empty
-		// group via quick open editor history)
-		if (!openedInOtherGroups) {
-			this.dispose();
-		}
-	}
-
 	supportsSplitEditor(): boolean {
 		return true;
 	}

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

@@ -689,14 +689,14 @@ export class EditorGroup extends Disposable {
 		return [this.editors[index], index];
 	}
 
-	contains(candidate: EditorInput, searchInSideBySideEditors?: boolean): boolean {
+	contains(candidate: EditorInput, options?: { searchInSideBySideEditors?: boolean, strictEquals?: boolean }): boolean {
 		for (const editor of this.editors) {
-			if (this.matches(editor, candidate)) {
+			if (this.matches(editor, candidate, options?.strictEquals)) {
 				return true;
 			}
 
-			if (searchInSideBySideEditors && editor instanceof SideBySideEditorInput) {
-				if (this.matches(editor.master, candidate) || this.matches(editor.details, candidate)) {
+			if (options?.searchInSideBySideEditors && editor instanceof SideBySideEditorInput) {
+				if (this.matches(editor.master, candidate, options?.strictEquals) || this.matches(editor.details, candidate, options?.strictEquals)) {
 					return true;
 				}
 			}
@@ -705,11 +705,15 @@ export class EditorGroup extends Disposable {
 		return false;
 	}
 
-	private matches(editor: IEditorInput | null, candidate: IEditorInput | null): boolean {
+	private matches(editor: IEditorInput | null, candidate: IEditorInput | null, strictEquals?: boolean): boolean {
 		if (!editor || !candidate) {
 			return false;
 		}
 
+		if (strictEquals) {
+			return editor === candidate;
+		}
+
 		return editor.matches(candidate);
 	}
 

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

@@ -449,9 +449,7 @@ suite('EditorGroupsService', () => {
 		assert.equal(editorCloseCounter1, 1);
 		assert.equal(editorWillCloseCounter, 1);
 
-		assert.ok(inputInactive.gotClosed);
-		assert.equal(inputInactive.gotClosed?.group, group.id);
-		assert.equal(inputInactive.gotClosed?.openedInOtherGroups, false);
+		assert.ok(inputInactive.gotDisposed);
 
 		assert.equal(group.activeEditor, input);
 
@@ -487,12 +485,8 @@ suite('EditorGroupsService', () => {
 
 		await group.closeEditors([input, inputInactive]);
 
-		assert.ok(input.gotClosed);
-		assert.equal(input.gotClosed?.group, group.id);
-		assert.equal(input.gotClosed?.openedInOtherGroups, false);
-		assert.ok(inputInactive.gotClosed);
-		assert.equal(inputInactive.gotClosed?.group, group.id);
-		assert.equal(inputInactive.gotClosed?.openedInOtherGroups, false);
+		assert.ok(input.gotDisposed);
+		assert.ok(inputInactive.gotDisposed);
 
 		assert.equal(group.isEmpty, true);
 		part.dispose();
@@ -513,15 +507,11 @@ suite('EditorGroupsService', () => {
 
 		await rightGroup.closeEditor(input);
 
-		assert.ok(input.gotClosed);
-		assert.equal(input.gotClosed?.group, rightGroup.id);
-		assert.equal(input.gotClosed?.openedInOtherGroups, true);
+		assert.ok(!input.gotDisposed);
 
 		await group.closeEditor(input);
 
-		assert.ok(input.gotClosed);
-		assert.equal(input.gotClosed?.group, group.id);
-		assert.equal(input.gotClosed?.openedInOtherGroups, false);
+		assert.ok(input.gotDisposed);
 	});
 
 	test('closeEditors (except one)', async () => {

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

@@ -277,9 +277,11 @@ suite('Workbench editor groups', () => {
 		group.openEditor(input1, { pinned: true, active: true });
 
 		assert.equal(group.contains(input1), true);
-		assert.equal(group.contains(input1, true), true);
+		assert.equal(group.contains(input1, { strictEquals: true }), true);
+		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, true), false);
+		assert.equal(group.contains(input2, { strictEquals: true }), false);
+		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), false);
 
@@ -307,7 +309,7 @@ suite('Workbench editor groups', () => {
 		group.closeEditor(input1);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, true), true);
+		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(input2), true);
 		assert.equal(group.contains(diffInput1), true);
 		assert.equal(group.contains(diffInput2), true);
@@ -315,27 +317,27 @@ suite('Workbench editor groups', () => {
 		group.closeEditor(input2);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, true), true);
+		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, true), true);
+		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(diffInput1), true);
 		assert.equal(group.contains(diffInput2), true);
 
 		group.closeEditor(diffInput1);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, true), true);
+		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, true), true);
+		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), true);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), true);
 
 		group.closeEditor(diffInput2);
 
 		assert.equal(group.contains(input1), false);
-		assert.equal(group.contains(input1, true), false);
+		assert.equal(group.contains(input1, { searchInSideBySideEditors: true }), false);
 		assert.equal(group.contains(input2), false);
-		assert.equal(group.contains(input2, true), false);
+		assert.equal(group.contains(input2, { searchInSideBySideEditors: true }), false);
 		assert.equal(group.contains(diffInput1), false);
 		assert.equal(group.contains(diffInput2), false);
 

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

@@ -1060,7 +1060,6 @@ export class TestFileEditorInput extends EditorInput implements IFileEditorInput
 	gotSaved = false;
 	gotSavedAs = false;
 	gotReverted = false;
-	gotClosed: { group: GroupIdentifier, openedInOtherGroups: boolean } | undefined = undefined;
 	dirty = false;
 	private fails = false;
 
@@ -1107,10 +1106,6 @@ export class TestFileEditorInput extends EditorInput implements IFileEditorInput
 		return false;
 	}
 	isResolved(): boolean { return false; }
-	close(group: GroupIdentifier, openedInOtherGroups: boolean): void {
-		this.gotClosed = { group, openedInOtherGroups };
-		super.close(group, openedInOtherGroups);
-	}
 	dispose(): void {
 		super.dispose();
 		this.gotDisposed = true;