diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index 01eab7e6856..ba94efe8d29 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -514,15 +514,21 @@ 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.containsEditorByInstance(editor))) { editorsToClose.push(editor.master, editor.details); } - // Close the editor when it is no longer open in any group including diff editors + // Dispose the editor when it is no longer open in any group including diff editors editorsToClose.forEach(editorToClose => { const resource = editorToClose ? editorToClose.getResource() : undefined; // prefer resource to not close right-hand side editors of a diff editor - if (!this.accessor.groups.some(groupView => groupView.group.contains(resource || editorToClose))) { - editorToClose.close(); + if (!this.accessor.groups.some(groupView => { + if (resource) { + return groupView.group.containsEditorByResource(resource, SideBySideEditor.MASTER); + } + + return groupView.group.containsEditorByInstance(editorToClose); + })) { + editorToClose.dispose(); } }); @@ -578,7 +584,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { editors.forEach(editor => { if (this._group.isActive(editor)) { activeEditor = editor; - } else if (this._group.contains(editor)) { + } else if (this._group.containsEditorByInstance(editor)) { inactiveEditors.push(editor); } }); @@ -770,7 +776,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { } isOpened(editor: EditorInput): boolean { - return this._group.contains(editor); + return this._group.containsEditorByInstance(editor); } focus(): void { @@ -1264,7 +1270,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { private async doHandleDirty(editor: EditorInput): Promise { if ( !editor.isDirty() || // editor must be dirty - this.accessor.groups.some(groupView => groupView !== this && groupView.group.contains(editor, true /* support side by side */)) || // editor is opened in other group + this.accessor.groups.some(groupView => groupView !== this && groupView.group.containsEditorByInstance(editor, SideBySideEditor.MASTER /* support side by side */)) || // editor is opened in other group editor instanceof SideBySideEditorInput && this.isOpened(editor.master) // side by side editor master is still opened ) { return false; diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index 1764e7ed832..e6a70ab4702 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -333,14 +333,14 @@ export interface IEditorInput extends IDisposable { */ export abstract class EditorInput extends Disposable implements IEditorInput { - protected readonly _onDidChangeDirty: Emitter = this._register(new Emitter()); - readonly onDidChangeDirty: Event = this._onDidChangeDirty.event; + protected readonly _onDidChangeDirty = this._register(new Emitter()); + readonly onDidChangeDirty = this._onDidChangeDirty.event; - protected readonly _onDidChangeLabel: Emitter = this._register(new Emitter()); - readonly onDidChangeLabel: Event = this._onDidChangeLabel.event; + protected readonly _onDidChangeLabel = this._register(new Emitter()); + readonly onDidChangeLabel = this._onDidChangeLabel.event; - private readonly _onDispose: Emitter = this._register(new Emitter()); - readonly onDispose: Event = this._onDispose.event; + private readonly _onDispose = this._register(new Emitter()); + readonly onDispose = this._onDispose.event; private disposed: boolean = false; @@ -429,13 +429,6 @@ export abstract class EditorInput extends Disposable implements IEditorInput { return Promise.resolve(true); } - /** - * Called when this input is no longer opened in any editor. Subclasses can free resources as needed. - */ - close(): void { - this.dispose(); - } - /** * Subclasses can set this to false if it does not make sense to split the editor input. */ @@ -640,8 +633,8 @@ export interface ITextEditorModel extends IEditorModel { */ export class EditorModel extends Disposable implements IEditorModel { - private readonly _onDispose: Emitter = this._register(new Emitter()); - readonly onDispose: Event = this._onDispose.event; + private readonly _onDispose = this._register(new Emitter()); + readonly onDispose = this._onDispose.event; /** * Causes this model to load returning a promise when loading is completed. diff --git a/src/vs/workbench/common/editor/editorGroup.ts b/src/vs/workbench/common/editor/editorGroup.ts index 72f1223b558..13df58c3ac5 100644 --- a/src/vs/workbench/common/editor/editorGroup.ts +++ b/src/vs/workbench/common/editor/editorGroup.ts @@ -60,31 +60,31 @@ export class EditorGroup extends Disposable { //#region events private readonly _onDidEditorActivate = this._register(new Emitter()); - readonly onDidEditorActivate: Event = this._onDidEditorActivate.event; + readonly onDidEditorActivate = this._onDidEditorActivate.event; private readonly _onDidEditorOpen = this._register(new Emitter()); - readonly onDidEditorOpen: Event = this._onDidEditorOpen.event; + readonly onDidEditorOpen = this._onDidEditorOpen.event; private readonly _onDidEditorClose = this._register(new Emitter()); - readonly onDidEditorClose: Event = this._onDidEditorClose.event; + readonly onDidEditorClose = this._onDidEditorClose.event; private readonly _onDidEditorDispose = this._register(new Emitter()); - readonly onDidEditorDispose: Event = this._onDidEditorDispose.event; + readonly onDidEditorDispose = this._onDidEditorDispose.event; private readonly _onDidEditorBecomeDirty = this._register(new Emitter()); - readonly onDidEditorBecomeDirty: Event = this._onDidEditorBecomeDirty.event; + readonly onDidEditorBecomeDirty = this._onDidEditorBecomeDirty.event; private readonly _onDidEditorLabelChange = this._register(new Emitter()); - readonly onDidEditorLabelChange: Event = this._onDidEditorLabelChange.event; + readonly onDidEditorLabelChange = this._onDidEditorLabelChange.event; private readonly _onDidEditorMove = this._register(new Emitter()); - readonly onDidEditorMove: Event = this._onDidEditorMove.event; + readonly onDidEditorMove = this._onDidEditorMove.event; private readonly _onDidEditorPin = this._register(new Emitter()); - readonly onDidEditorPin: Event = this._onDidEditorPin.event; + readonly onDidEditorPin = this._onDidEditorPin.event; private readonly _onDidEditorUnpin = this._register(new Emitter()); - readonly onDidEditorUnpin: Event = this._onDidEditorUnpin.event; + readonly onDidEditorUnpin = this._onDidEditorUnpin.event; //#endregion @@ -93,7 +93,10 @@ export class EditorGroup extends Disposable { private editors: EditorInput[] = []; private mru: EditorInput[] = []; - private mapResourceToEditorCount: ResourceMap = new ResourceMap(); + + private mapResourceToEditorCount = new ResourceMap(); + private mapResourceToMasterEditorCount = new ResourceMap(); + private mapResourceToDetailsEditorCount = new ResourceMap(); private preview: EditorInput | null = null; // editor in preview state private active: EditorInput | null = null; // editor in active state @@ -143,7 +146,7 @@ export class EditorGroup extends Disposable { } const resource: URI = arg1; - if (!this.contains(resource)) { + if (!this.containsEditorByResource(resource, SideBySideEditor.MASTER)) { return undefined; // fast check for resource opened or not } @@ -509,7 +512,7 @@ export class EditorGroup extends Disposable { // Add if (!del && editor) { this.mru.push(editor); // make it LRU editor - this.updateResourceMap(editor, false /* add */); // add new to resource map + this.updateResourceCounterMap(editor, false /* add */); // add new to resource map } // Remove / Replace @@ -519,42 +522,63 @@ export class EditorGroup extends Disposable { // Remove if (del && !editor) { this.mru.splice(indexInMRU, 1); // remove from MRU - this.updateResourceMap(editorToDeleteOrReplace, true /* delete */); // remove from resource map + this.updateResourceCounterMap(editorToDeleteOrReplace, true /* delete */); // remove from resource map } // Replace else if (del && editor) { this.mru.splice(indexInMRU, 1, editor); // replace MRU at location - this.updateResourceMap(editor, false /* add */); // add new to resource map - this.updateResourceMap(editorToDeleteOrReplace, true /* delete */); // remove replaced from resource map + this.updateResourceCounterMap(editor, false /* add */); // add new to resource map + this.updateResourceCounterMap(editorToDeleteOrReplace, true /* delete */); // remove replaced from resource map } } } - private updateResourceMap(editor: EditorInput, remove: boolean): void { - const resource = toResource(editor, { supportSideBySide: SideBySideEditor.MASTER }); + private updateResourceCounterMap(editor: EditorInput, remove: boolean): void { + + // Remember editor resource in map for fast lookup + const resource = toResource(editor); if (resource) { + this.doUpdateResourceCounterMap(resource, this.mapResourceToEditorCount, remove); + } - // It is possible to have the same resource opened twice (once as normal input and once as diff input) - // So we need to do ref counting on the resource to provide the correct picture - const counter = this.mapResourceToEditorCount.get(resource) || 0; - - // Add - let newCounter: number; - if (!remove) { - newCounter = counter + 1; + // Side by Side editor: store resource information + // for master and details side in separate maps + // to be able to lookup properly. + if (editor instanceof SideBySideEditorInput) { + const masterResource = toResource(editor.master); + if (masterResource) { + this.doUpdateResourceCounterMap(masterResource, this.mapResourceToMasterEditorCount, remove); } - // Delete - else { - newCounter = counter - 1; + const detailsResource = toResource(editor.details); + if (detailsResource) { + this.doUpdateResourceCounterMap(detailsResource, this.mapResourceToDetailsEditorCount, remove); } + } + } - if (newCounter > 0) { - this.mapResourceToEditorCount.set(resource, newCounter); - } else { - this.mapResourceToEditorCount.delete(resource); - } + private doUpdateResourceCounterMap(resource: URI, map: ResourceMap, remove: boolean): void { + + // It is possible to have the same resource opened twice (once as normal input and once as diff input) + // So we need to do ref counting on the resource to provide the correct picture + const counter = map.get(resource) || 0; + + // Add + let newCounter: number; + if (!remove) { + newCounter = counter + 1; + } + + // Delete + else { + newCounter = counter - 1; + } + + if (newCounter > 0) { + map.set(resource, newCounter); + } else { + map.delete(resource); } } @@ -572,28 +596,38 @@ export class EditorGroup extends Disposable { return -1; } - contains(editorOrResource: EditorInput | URI): boolean; - contains(editor: EditorInput, supportSideBySide?: boolean): boolean; - contains(editorOrResource: EditorInput | URI, supportSideBySide?: boolean): boolean { - if (editorOrResource instanceof EditorInput) { - const index = this.indexOf(editorOrResource); + containsEditorByResource(resource: URI, supportSideBySide?: SideBySideEditor): boolean { + + // Check if exact editor match is contained + let counter = this.mapResourceToEditorCount.get(resource); + + // Optionally search by master/detail resource if instructed + if (supportSideBySide === SideBySideEditor.MASTER) { + counter = counter || this.mapResourceToMasterEditorCount.get(resource); + } else if (supportSideBySide === SideBySideEditor.DETAILS) { + counter = counter || this.mapResourceToDetailsEditorCount.get(resource); + } + + return typeof counter === 'number' && counter > 0; + } + + containsEditorByInstance(editor: EditorInput, supportSideBySide?: SideBySideEditor): boolean { + + // Check if exact editor match is contained + const index = this.indexOf(editor); + if (index >= 0) { + return true; + } + + // Optionally search by master/detail input if instructed + if (supportSideBySide && editor instanceof SideBySideEditorInput) { + const index = this.indexOf(supportSideBySide === SideBySideEditor.MASTER ? editor.master : editor.details); if (index >= 0) { return true; } - - if (supportSideBySide && editorOrResource instanceof SideBySideEditorInput) { - const index = this.indexOf(editorOrResource.master); - if (index >= 0) { - return true; - } - } - - return false; } - const counter = this.mapResourceToEditorCount.get(editorOrResource); - - return typeof counter === 'number' && counter > 0; + return false; } private setMostRecentlyUsed(editor: EditorInput): void { @@ -620,6 +654,8 @@ export class EditorGroup extends Disposable { group.editors = this.editors.slice(0); group.mru = this.mru.slice(0); group.mapResourceToEditorCount = this.mapResourceToEditorCount.clone(); + group.mapResourceToMasterEditorCount = this.mapResourceToMasterEditorCount.clone(); + group.mapResourceToDetailsEditorCount = this.mapResourceToDetailsEditorCount.clone(); group.preview = this.preview; group.active = this.active; group.editorOpenPositioning = this.editorOpenPositioning; @@ -678,7 +714,7 @@ export class EditorGroup extends Disposable { const editor = factory.deserialize(this.instantiationService, e.value); if (editor) { this.registerEditorListeners(editor); - this.updateResourceMap(editor, false /* add */); + this.updateResourceCounterMap(editor, false /* add */); } return editor; diff --git a/src/vs/workbench/test/common/editor/editorGroups.test.ts b/src/vs/workbench/test/common/editor/editorGroups.test.ts index 46805b2c9da..9f8721b7a68 100644 --- a/src/vs/workbench/test/common/editor/editorGroups.test.ts +++ b/src/vs/workbench/test/common/editor/editorGroups.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { EditorGroup, ISerializedEditorGroup, EditorCloseEvent } from 'vs/workbench/common/editor/editorGroup'; -import { Extensions as EditorExtensions, IEditorInputFactoryRegistry, EditorInput, IFileEditorInput, IEditorInputFactory, CloseDirection } from 'vs/workbench/common/editor'; +import { Extensions as EditorExtensions, IEditorInputFactoryRegistry, EditorInput, IFileEditorInput, IEditorInputFactory, CloseDirection, SideBySideEditor } from 'vs/workbench/common/editor'; import { URI } from 'vs/base/common/uri'; import { TestLifecycleService, TestContextService, TestStorageService } from 'vs/workbench/test/workbenchTestServices'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; @@ -193,13 +193,17 @@ suite('Workbench editor groups', () => { const input1 = input(); const input2 = input(); - const diffInput = new DiffEditorInput('name', 'description', input1, input2); + const diffInput1 = new DiffEditorInput('name', 'description', input1, input2); + const diffInput2 = new DiffEditorInput('name', 'description', input2, input1); group.openEditor(input2, { pinned: true, active: true }); - assert.equal(group.contains(input2), true); - assert.equal(group.contains(diffInput), false); - assert.equal(group.contains(diffInput, true), true); + assert.equal(group.containsEditorByInstance(input2, SideBySideEditor.MASTER), true); + assert.equal(group.containsEditorByInstance(diffInput1), false); + assert.equal(group.containsEditorByInstance(diffInput1, SideBySideEditor.MASTER), true); + assert.equal(group.containsEditorByInstance(diffInput1, SideBySideEditor.DETAILS), false); + assert.equal(group.containsEditorByInstance(diffInput2, SideBySideEditor.MASTER), false); + assert.equal(group.containsEditorByInstance(diffInput2, SideBySideEditor.DETAILS), true); }); test('group serialization', function () { @@ -1171,36 +1175,53 @@ suite('Workbench editor groups', () => { const input1 = input(undefined, false, input1Resource); group1.openEditor(input1); - assert.ok(group1.contains(input1Resource)); + assert.ok(group1.containsEditorByResource(input1Resource)); assert.equal(group1.getEditor(input1Resource), input1); assert.ok(!group1.getEditor(input1ResourceUpper)); - assert.ok(!group1.contains(input1ResourceUpper)); + assert.ok(!group1.containsEditorByResource(input1ResourceUpper)); group2.openEditor(input1); group1.closeEditor(input1); - assert.ok(!group1.contains(input1Resource)); + assert.ok(!group1.containsEditorByResource(input1Resource)); assert.ok(!group1.getEditor(input1Resource)); assert.ok(!group1.getEditor(input1ResourceUpper)); - assert.ok(group2.contains(input1Resource)); + assert.ok(group2.containsEditorByResource(input1Resource)); assert.equal(group2.getEditor(input1Resource), input1); const input1ResourceClone = URI.file('/hello/world.txt'); const input1Clone = input(undefined, false, input1ResourceClone); group1.openEditor(input1Clone); - assert.ok(group1.contains(input1Resource)); + assert.ok(group1.containsEditorByResource(input1Resource)); group2.closeEditor(input1); - assert.ok(group1.contains(input1Resource)); + assert.ok(group1.containsEditorByResource(input1Resource)); assert.equal(group1.getEditor(input1Resource), input1Clone); - assert.ok(!group2.contains(input1Resource)); + assert.ok(!group2.containsEditorByResource(input1Resource)); group1.closeEditor(input1Clone); - assert.ok(!group1.contains(input1Resource)); + assert.ok(!group1.containsEditorByResource(input1Resource)); + + const masterResource = URI.file('/hello/world2.txt'); + const masterInput = input(undefined, false, masterResource); + + const detailsResource = URI.file('/hello/world3.txt'); + const detailsInput = input(undefined, false, detailsResource); + + const diffEditorInput = new DiffEditorInput('name', 'description', detailsInput, masterInput); + group1.openEditor(diffEditorInput); + + assert.equal(group1.containsEditorByResource(masterResource), false); + assert.equal(group1.containsEditorByResource(masterResource, SideBySideEditor.MASTER), true); + assert.equal(group1.containsEditorByResource(masterResource, SideBySideEditor.DETAILS), false); + + assert.equal(group1.containsEditorByResource(detailsResource), false); + assert.equal(group1.containsEditorByResource(detailsResource, SideBySideEditor.MASTER), false); + assert.equal(group1.containsEditorByResource(detailsResource, SideBySideEditor.DETAILS), true); }); test('Multiple Editors - Editor Dispose', function () {