From 8f42ea43aa5a3aaca960391e80e349c460ae4d19 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sat, 17 Feb 2024 13:52:28 +0000 Subject: [PATCH 1/9] Add intial implementation --- .../editor/common/editorGroupFinder.ts | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index bcae9ca0e589c..ab6cb73a2dfdd 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -135,7 +135,40 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer } // Prefer a target group where the input is visible - group = groupWithInputActive || groupWithInputOpened; + const groupWithInput = groupWithInputActive || groupWithInputOpened; + + if (groupWithInput) { + const candidateGroup = editorGroupService.activeGroup; + let unlockedGroup: IEditorGroup | undefined = undefined; + + // Locked group: find the next non-locked group + // going up the neigbours of the group or create + // a new group otherwise + if (isGroupLockedForEditor(candidateGroup, editor)) { + for (const group of editorGroupService.getGroups(GroupsOrder.MOST_RECENTLY_ACTIVE)) { + if (isGroupLockedForEditor(group, editor)) { + continue; + } + + unlockedGroup = group; + break; + } + } + + // Non-locked group: take as is + else { + unlockedGroup = candidateGroup; + } + + if (unlockedGroup) { + group.moveEditor(editor, unlockedGroup); + group = unlockedGroup; + } + + else { + group = groupWithInput; + } + } } } } From 600df97f4776b4d8d201b883bddac0da8db4a00b Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sat, 17 Feb 2024 15:43:03 +0000 Subject: [PATCH 2/9] Fix implementation and add configuration option --- .../workbench/browser/parts/editor/editor.ts | 2 + .../browser/workbench.contribution.ts | 5 ++ src/vs/workbench/common/editor.ts | 1 + .../editor/common/editorGroupFinder.ts | 67 +++++++++---------- 4 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/editor.ts b/src/vs/workbench/browser/parts/editor/editor.ts index b118e9f193b45..c8538c883ea4c 100644 --- a/src/vs/workbench/browser/parts/editor/editor.ts +++ b/src/vs/workbench/browser/parts/editor/editor.ts @@ -63,6 +63,7 @@ export const DEFAULT_EDITOR_PART_OPTIONS: IEditorPartOptions = { restoreViewState: true, splitInGroupLayout: 'horizontal', revealIfOpen: false, + revealIfOpenInActiveGroup: false, // Properties that are Objects have to be defined as getters // to ensure no consumer modifies the default values get limit(): IEditorPartLimitOptions { return { enabled: false, value: 10, perEditorGroup: false, excludeDirty: false }; }, @@ -130,6 +131,7 @@ function validateEditorPartOptions(options: IEditorPartOptions): IEditorPartOpti 'closeOnFileDelete': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['closeOnFileDelete']), 'closeEmptyGroups': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['closeEmptyGroups']), 'revealIfOpen': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['revealIfOpen']), + 'revealIfOpenInActiveGroup': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['revealIfOpenInActiveGroup']), 'mouseBackForwardToNavigate': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['mouseBackForwardToNavigate']), 'restoreViewState': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['restoreViewState']), 'splitOnDragAndDrop': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['splitOnDragAndDrop']), diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index b715436c0d466..a56ebac748ae6 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -294,6 +294,11 @@ const registry = Registry.as(ConfigurationExtensions.Con 'description': localize('revealIfOpen', "Controls whether an editor is revealed in any of the visible groups if opened. If disabled, an editor will prefer to open in the currently active editor group. If enabled, an already opened editor will be revealed instead of opened again in the currently active editor group. Note that there are some cases where this setting is ignored, such as when forcing an editor to open in a specific group or to the side of the currently active group."), 'default': false }, + 'workbench.editor.revealIfOpenInActiveGroup': { + 'type': 'boolean', + 'markdownDescription': localize('revealIfOpenInActiveGroup', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies to `#workbench.editor.revealIfOpen#` is set."), + 'default': false + }, 'workbench.editor.mouseBackForwardToNavigate': { 'type': 'boolean', 'description': localize('mouseBackForwardToNavigate', "Enables the use of mouse buttons four and five for commands 'Go Back' and 'Go Forward'."), diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index bf6785d967516..890ee7fd6281c 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -1183,6 +1183,7 @@ interface IEditorPartConfiguration { closeEmptyGroups?: boolean; autoLockGroups?: Set; revealIfOpen?: boolean; + revealIfOpenInActiveGroup?: boolean; mouseBackForwardToNavigate?: boolean; labelFormat?: 'default' | 'short' | 'medium' | 'long'; restoreViewState?: boolean; diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index ab6cb73a2dfdd..a8d65efaa0623 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -114,59 +114,54 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer // We also try to reveal an editor if it has the `Singleton` capability which // indicates that the same editor cannot be opened across groups. if (!group) { - if (options?.revealIfOpened || configurationService.getValue('workbench.editor.revealIfOpen') || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { - let groupWithInputActive: IEditorGroup | undefined = undefined; - let groupWithInputOpened: IEditorGroup | undefined = undefined; + const revealIfOpen = configurationService.getValue('workbench.editor.revealIfOpen'); + + if (options?.revealIfOpened || revealIfOpen || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { + let groupAndEditorWithInputActive: [IEditorGroup, EditorInput] | undefined = undefined; + let groupAndEditorWithInputOpened: [IEditorGroup, EditorInput] | undefined = undefined; for (const group of groupsByLastActive) { - if (isOpened(group, editor)) { - if (!groupWithInputOpened) { - groupWithInputOpened = group; + let typedEditor: EditorInput | undefined = undefined; + + for (const groupTypedEditor of group.editors) { + if (groupTypedEditor.matches(editor)) { + typedEditor = groupTypedEditor; + break; + } + } + + if (typedEditor) { + if (!groupAndEditorWithInputOpened) { + groupAndEditorWithInputOpened = [group, typedEditor]; } - if (!groupWithInputActive && group.isActive(editor)) { - groupWithInputActive = group; + if (!groupAndEditorWithInputActive && group.isActive(editor)) { + groupAndEditorWithInputActive = [group, typedEditor]; } } - if (groupWithInputOpened && groupWithInputActive) { + if (groupAndEditorWithInputOpened && groupAndEditorWithInputActive) { break; // we found all groups we wanted } } // Prefer a target group where the input is visible - const groupWithInput = groupWithInputActive || groupWithInputOpened; - - if (groupWithInput) { - const candidateGroup = editorGroupService.activeGroup; - let unlockedGroup: IEditorGroup | undefined = undefined; - - // Locked group: find the next non-locked group - // going up the neigbours of the group or create - // a new group otherwise - if (isGroupLockedForEditor(candidateGroup, editor)) { - for (const group of editorGroupService.getGroups(GroupsOrder.MOST_RECENTLY_ACTIVE)) { - if (isGroupLockedForEditor(group, editor)) { - continue; - } - - unlockedGroup = group; - break; - } - } + const groupAndEditorWithInput = groupAndEditorWithInputActive || groupAndEditorWithInputOpened; - // Non-locked group: take as is - else { - unlockedGroup = candidateGroup; - } + if (groupAndEditorWithInput) { + const [groupWithInput, typedEditor] = groupAndEditorWithInput; + + const revealIfOpenInActiveGroup = configurationService.getValue('workbench.editor.revealIfOpenInActiveGroup'); - if (unlockedGroup) { - group.moveEditor(editor, unlockedGroup); - group = unlockedGroup; + // If workbench.editor.revealIfOpenInActiveGroup is set, + // move editor to active group unless it is locked + if (revealIfOpen && revealIfOpenInActiveGroup && !isGroupLockedForEditor(editorGroupService.activeGroup, editor)) { + groupWithInput.moveEditor(typedEditor, editorGroupService.activeGroup); + group = editorGroupService.activeGroup; } else { - group = groupWithInput; + group = groupWithInput; } } } From 8b1859514ad3b5339b2a05e438108d7dd0a46553 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sat, 17 Feb 2024 23:21:06 +0000 Subject: [PATCH 3/9] Fix typo --- src/vs/workbench/browser/workbench.contribution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index a56ebac748ae6..89a0fa21e7b6b 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -296,7 +296,7 @@ const registry = Registry.as(ConfigurationExtensions.Con }, 'workbench.editor.revealIfOpenInActiveGroup': { 'type': 'boolean', - 'markdownDescription': localize('revealIfOpenInActiveGroup', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies to `#workbench.editor.revealIfOpen#` is set."), + 'markdownDescription': localize('revealIfOpenInActiveGroup', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies when `#workbench.editor.revealIfOpen#` is set."), 'default': false }, 'workbench.editor.mouseBackForwardToNavigate': { From bcba64973de5a7d2df0ec73b9576a0aa527f2927 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sat, 17 Feb 2024 23:27:39 +0000 Subject: [PATCH 4/9] Extract `isActiveGroupLocked` --- src/vs/workbench/services/editor/common/editorGroupFinder.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index a8d65efaa0623..1e25bdb777e77 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -152,10 +152,11 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer const [groupWithInput, typedEditor] = groupAndEditorWithInput; const revealIfOpenInActiveGroup = configurationService.getValue('workbench.editor.revealIfOpenInActiveGroup'); + const isActiveGroupLocked = isGroupLockedForEditor(editorGroupService.activeGroup, editor); // If workbench.editor.revealIfOpenInActiveGroup is set, // move editor to active group unless it is locked - if (revealIfOpen && revealIfOpenInActiveGroup && !isGroupLockedForEditor(editorGroupService.activeGroup, editor)) { + if (revealIfOpen && revealIfOpenInActiveGroup && !isActiveGroupLocked) { groupWithInput.moveEditor(typedEditor, editorGroupService.activeGroup); group = editorGroupService.activeGroup; } From 06b61d14034c2b9019fd3c3a08157986cf46c6e1 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Thu, 7 Mar 2024 20:21:09 +0000 Subject: [PATCH 5/9] Rename `revealIfOpenInActiveGroup` to `moveToActiveGroupIfOpen` --- src/vs/workbench/browser/parts/editor/editor.ts | 4 ++-- src/vs/workbench/browser/workbench.contribution.ts | 4 ++-- src/vs/workbench/common/editor.ts | 2 +- .../workbench/services/editor/common/editorGroupFinder.ts | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/editor.ts b/src/vs/workbench/browser/parts/editor/editor.ts index c8538c883ea4c..444dc6367a158 100644 --- a/src/vs/workbench/browser/parts/editor/editor.ts +++ b/src/vs/workbench/browser/parts/editor/editor.ts @@ -63,7 +63,7 @@ export const DEFAULT_EDITOR_PART_OPTIONS: IEditorPartOptions = { restoreViewState: true, splitInGroupLayout: 'horizontal', revealIfOpen: false, - revealIfOpenInActiveGroup: false, + moveToActiveGroupIfOpen: false, // Properties that are Objects have to be defined as getters // to ensure no consumer modifies the default values get limit(): IEditorPartLimitOptions { return { enabled: false, value: 10, perEditorGroup: false, excludeDirty: false }; }, @@ -131,7 +131,7 @@ function validateEditorPartOptions(options: IEditorPartOptions): IEditorPartOpti 'closeOnFileDelete': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['closeOnFileDelete']), 'closeEmptyGroups': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['closeEmptyGroups']), 'revealIfOpen': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['revealIfOpen']), - 'revealIfOpenInActiveGroup': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['revealIfOpenInActiveGroup']), + 'moveToActiveGroupIfOpen': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['moveToActiveGroupIfOpen']), 'mouseBackForwardToNavigate': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['mouseBackForwardToNavigate']), 'restoreViewState': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['restoreViewState']), 'splitOnDragAndDrop': new BooleanVerifier(DEFAULT_EDITOR_PART_OPTIONS['splitOnDragAndDrop']), diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index 045917948b082..91f651f1c3a53 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -294,9 +294,9 @@ const registry = Registry.as(ConfigurationExtensions.Con 'description': localize('revealIfOpen', "Controls whether an editor is revealed in any of the visible groups if opened. If disabled, an editor will prefer to open in the currently active editor group. If enabled, an already opened editor will be revealed instead of opened again in the currently active editor group. Note that there are some cases where this setting is ignored, such as when forcing an editor to open in a specific group or to the side of the currently active group."), 'default': false }, - 'workbench.editor.revealIfOpenInActiveGroup': { + 'workbench.editor.moveToActiveGroupIfOpen': { 'type': 'boolean', - 'markdownDescription': localize('revealIfOpenInActiveGroup', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies when `#workbench.editor.revealIfOpen#` is set."), + 'markdownDescription': localize('moveToActiveGroupIfOpen', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies when `#workbench.editor.revealIfOpen#` is set."), 'default': false }, 'workbench.editor.mouseBackForwardToNavigate': { diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index bd79c716a34d9..37d79dc6311a9 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -1188,7 +1188,7 @@ interface IEditorPartConfiguration { closeEmptyGroups?: boolean; autoLockGroups?: Set; revealIfOpen?: boolean; - revealIfOpenInActiveGroup?: boolean; + moveToActiveGroupIfOpen?: boolean; mouseBackForwardToNavigate?: boolean; labelFormat?: 'default' | 'short' | 'medium' | 'long'; restoreViewState?: boolean; diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index 1e25bdb777e77..7d9bf1db38b71 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -151,12 +151,12 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer if (groupAndEditorWithInput) { const [groupWithInput, typedEditor] = groupAndEditorWithInput; - const revealIfOpenInActiveGroup = configurationService.getValue('workbench.editor.revealIfOpenInActiveGroup'); + const moveToActiveGroupIfOpen = configurationService.getValue('workbench.editor.moveToActiveGroupIfOpen'); const isActiveGroupLocked = isGroupLockedForEditor(editorGroupService.activeGroup, editor); - // If workbench.editor.revealIfOpenInActiveGroup is set, + // If workbench.editor.moveToActiveGroupIfOpen is set, // move editor to active group unless it is locked - if (revealIfOpen && revealIfOpenInActiveGroup && !isActiveGroupLocked) { + if (revealIfOpen && moveToActiveGroupIfOpen && !isActiveGroupLocked) { groupWithInput.moveEditor(typedEditor, editorGroupService.activeGroup); group = editorGroupService.activeGroup; } From 8920fdac3609bd52646498e45da729cac224f616 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sun, 21 Apr 2024 16:47:34 +0000 Subject: [PATCH 6/9] Move logic to `editorResolverService` --- .../editor/browser/editorResolverService.ts | 55 ++++++++++++------- .../editor/common/editorGroupFinder.ts | 2 +- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/services/editor/browser/editorResolverService.ts b/src/vs/workbench/services/editor/browser/editorResolverService.ts index d18ee039799a7..fc1497bab0e86 100644 --- a/src/vs/workbench/services/editor/browser/editorResolverService.ts +++ b/src/vs/workbench/services/editor/browser/editorResolverService.ts @@ -65,7 +65,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver @ITelemetryService private readonly telemetryService: ITelemetryService, @IStorageService private readonly storageService: IStorageService, @IExtensionService private readonly extensionService: IExtensionService, - @ILogService private readonly logService: ILogService + @ILogService private readonly logService: ILogService, ) { super(); // Read in the cache on statup @@ -502,11 +502,14 @@ 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 except one and move the new one into the group - const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource; - if (singleEditorPerResource) { + const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource ?? false; + const revealIfOpen = this.configurationService.getValue('workbench.editor.revealIfOpen'); + + if (singleEditorPerResource || revealIfOpen) { const existingEditors = this.findExistingEditorsForResource(resource, selectedEditor.editorInfo.id); + const shouldCloseOtherEditors = singleEditorPerResource; if (existingEditors.length) { - const editor = await this.moveExistingEditorForResource(existingEditors, group); + const editor = await this.moveExistingEditorForResource(existingEditors, group, shouldCloseOtherEditors); if (editor) { return { editor, options }; } else { @@ -529,30 +532,44 @@ export class EditorResolverService extends Disposable implements IEditorResolver } /** - * Moves the first existing editor for a resource to the target group unless already opened there. - * Additionally will close any other editors that are open for that resource and viewtype besides the first one found - * @param resource The resource of the editor - * @param viewType the viewtype of the editor - * @param targetGroup The group to move it to - * @returns The moved editor input or `undefined` if the editor could not be moved + * Moves the first existing editor for a resource to the target group unless one is already opened there. + * Optionally will close any other editors that are open for that resource and viewtype + * @param existingEditorsForResource All the editors for a resource (must be non-empty) + * @param targetGroup The group to move it the first editor to + * @param shouldCloseOtherEditors If `true`, attempt to close all other editors + * @returns The moved or already opened editor input or `undefined` if the editor could not be moved */ private async moveExistingEditorForResource( existingEditorsForResource: Array<{ editor: EditorInput; group: IEditorGroup }>, targetGroup: IEditorGroup, + shouldCloseOtherEditors: boolean, ): Promise { - const editorToUse = existingEditorsForResource[0]; - - // We should only have one editor but if there are multiple we close the others - for (const { editor, group } of existingEditorsForResource) { - if (editor !== editorToUse.editor) { - const closed = await group.closeEditor(editor); - if (!closed) { - return; + let editorToUse: { editor: EditorInput; group: IEditorGroup } | undefined = undefined; + + // Prefer an editor that is already in the target group + for (const editor of existingEditorsForResource) { + if (targetGroup.id === editor.group.id) { + editorToUse = editor; + break; + } + } + + if (editorToUse === undefined) { + editorToUse = existingEditorsForResource[0]; + } + + if (shouldCloseOtherEditors) { + for (const { editor, group } of existingEditorsForResource) { + if (editor !== editorToUse.editor) { + const closed = await group.closeEditor(editor); + if (!closed) { + return; + } } } } - // Move the editor already opened to the target group + // Move the already opened editor to the target group if it is outside it if (targetGroup.id !== editorToUse.group.id) { const moved = editorToUse.group.moveEditor(editorToUse.editor, targetGroup); if (!moved) { diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index 7d9bf1db38b71..15542f4651905 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -114,7 +114,7 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer // We also try to reveal an editor if it has the `Singleton` capability which // indicates that the same editor cannot be opened across groups. if (!group) { - const revealIfOpen = configurationService.getValue('workbench.editor.revealIfOpen'); + const revealIfOpen = false; // configurationService.getValue('workbench.editor.revealIfOpen'); if (options?.revealIfOpened || revealIfOpen || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { let groupAndEditorWithInputActive: [IEditorGroup, EditorInput] | undefined = undefined; From 3d7ff09e568f34c1264d95227e5dbb94e0e4e6c0 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sun, 21 Apr 2024 17:03:01 +0000 Subject: [PATCH 7/9] Respect editor group locks --- src/vs/workbench/browser/workbench.contribution.ts | 2 +- .../workbench/services/editor/browser/editorResolverService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index a4b1d77ef0faf..26ac7677f0ff8 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -325,7 +325,7 @@ const registry = Registry.as(ConfigurationExtensions.Con }, 'workbench.editor.moveToActiveGroupIfOpen': { 'type': 'boolean', - 'markdownDescription': localize('moveToActiveGroupIfOpen', "Moves already opened editors to the active editor group when they are revealed. Note that there are some cases where this setting is ignored, such as when the currently active group is locked. Only applies when `#workbench.editor.revealIfOpen#` is set."), + 'markdownDescription': localize('moveToActiveGroupIfOpen', "Moves already opened editors to the active editor group when they are revealed. Note that this setting is ignored when the currently active group is locked."), 'default': false }, 'workbench.editor.mouseBackForwardToNavigate': { diff --git a/src/vs/workbench/services/editor/browser/editorResolverService.ts b/src/vs/workbench/services/editor/browser/editorResolverService.ts index fc1497bab0e86..826e687a219bf 100644 --- a/src/vs/workbench/services/editor/browser/editorResolverService.ts +++ b/src/vs/workbench/services/editor/browser/editorResolverService.ts @@ -505,7 +505,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource ?? false; const revealIfOpen = this.configurationService.getValue('workbench.editor.revealIfOpen'); - if (singleEditorPerResource || revealIfOpen) { + if (singleEditorPerResource || (revealIfOpen && !group.isLocked)) { const existingEditors = this.findExistingEditorsForResource(resource, selectedEditor.editorInfo.id); const shouldCloseOtherEditors = singleEditorPerResource; if (existingEditors.length) { From 0610a88493db59803cf1814213154b8031e76436 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sun, 21 Apr 2024 17:07:49 +0000 Subject: [PATCH 8/9] Revert `editorGroupFinder.ts` --- .../editor/common/editorGroupFinder.ts | 49 ++++--------------- 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index 15542f4651905..bcae9ca0e589c 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -114,57 +114,28 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer // We also try to reveal an editor if it has the `Singleton` capability which // indicates that the same editor cannot be opened across groups. if (!group) { - const revealIfOpen = false; // configurationService.getValue('workbench.editor.revealIfOpen'); - - if (options?.revealIfOpened || revealIfOpen || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { - let groupAndEditorWithInputActive: [IEditorGroup, EditorInput] | undefined = undefined; - let groupAndEditorWithInputOpened: [IEditorGroup, EditorInput] | undefined = undefined; + if (options?.revealIfOpened || configurationService.getValue('workbench.editor.revealIfOpen') || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { + let groupWithInputActive: IEditorGroup | undefined = undefined; + let groupWithInputOpened: IEditorGroup | undefined = undefined; for (const group of groupsByLastActive) { - let typedEditor: EditorInput | undefined = undefined; - - for (const groupTypedEditor of group.editors) { - if (groupTypedEditor.matches(editor)) { - typedEditor = groupTypedEditor; - break; - } - } - - if (typedEditor) { - if (!groupAndEditorWithInputOpened) { - groupAndEditorWithInputOpened = [group, typedEditor]; + if (isOpened(group, editor)) { + if (!groupWithInputOpened) { + groupWithInputOpened = group; } - if (!groupAndEditorWithInputActive && group.isActive(editor)) { - groupAndEditorWithInputActive = [group, typedEditor]; + if (!groupWithInputActive && group.isActive(editor)) { + groupWithInputActive = group; } } - if (groupAndEditorWithInputOpened && groupAndEditorWithInputActive) { + if (groupWithInputOpened && groupWithInputActive) { break; // we found all groups we wanted } } // Prefer a target group where the input is visible - const groupAndEditorWithInput = groupAndEditorWithInputActive || groupAndEditorWithInputOpened; - - if (groupAndEditorWithInput) { - const [groupWithInput, typedEditor] = groupAndEditorWithInput; - - const moveToActiveGroupIfOpen = configurationService.getValue('workbench.editor.moveToActiveGroupIfOpen'); - const isActiveGroupLocked = isGroupLockedForEditor(editorGroupService.activeGroup, editor); - - // If workbench.editor.moveToActiveGroupIfOpen is set, - // move editor to active group unless it is locked - if (revealIfOpen && moveToActiveGroupIfOpen && !isActiveGroupLocked) { - groupWithInput.moveEditor(typedEditor, editorGroupService.activeGroup); - group = editorGroupService.activeGroup; - } - - else { - group = groupWithInput; - } - } + group = groupWithInputActive || groupWithInputOpened; } } } From c199868c162147985cc61ce874383770f0277c29 Mon Sep 17 00:00:00 2001 From: Jonathan Watson <23344719+jonathanjameswatson@users.noreply.github.com> Date: Sun, 21 Apr 2024 17:27:54 +0000 Subject: [PATCH 9/9] Allow `moveToActiveGroupIfOpen` and `revealIfOpen` --- .../services/editor/browser/editorResolverService.ts | 9 +++++---- .../services/editor/common/editorGroupFinder.ts | 5 ++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/services/editor/browser/editorResolverService.ts b/src/vs/workbench/services/editor/browser/editorResolverService.ts index 826e687a219bf..3c596f4295f4e 100644 --- a/src/vs/workbench/services/editor/browser/editorResolverService.ts +++ b/src/vs/workbench/services/editor/browser/editorResolverService.ts @@ -65,7 +65,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver @ITelemetryService private readonly telemetryService: ITelemetryService, @IStorageService private readonly storageService: IStorageService, @IExtensionService private readonly extensionService: IExtensionService, - @ILogService private readonly logService: ILogService, + @ILogService private readonly logService: ILogService ) { super(); // Read in the cache on statup @@ -501,11 +501,12 @@ export class EditorResolverService extends Disposable implements IEditorResolver throw new Error(`Undefined resource on non untitled editor input.`); } - // If the editor states it can only be opened once per resource we must close all existing ones except one and move the new one into the group const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource ?? false; - const revealIfOpen = this.configurationService.getValue('workbench.editor.revealIfOpen'); + const moveToActiveGroupIfOpen = this.configurationService.getValue('workbench.editor.moveToActiveGroupIfOpen'); - if (singleEditorPerResource || (revealIfOpen && !group.isLocked)) { + // If the editor states it can only be opened once per resource we must close all existing ones except one and move the new one into the group + // We also move the active editor if moveToActiveGroupIfOpen is enabled, although in this case we don't close other editors + if (singleEditorPerResource || (moveToActiveGroupIfOpen && !group.isLocked)) { const existingEditors = this.findExistingEditorsForResource(resource, selectedEditor.editorInfo.id); const shouldCloseOtherEditors = singleEditorPerResource; if (existingEditors.length) { diff --git a/src/vs/workbench/services/editor/common/editorGroupFinder.ts b/src/vs/workbench/services/editor/common/editorGroupFinder.ts index bcae9ca0e589c..5697d670784de 100644 --- a/src/vs/workbench/services/editor/common/editorGroupFinder.ts +++ b/src/vs/workbench/services/editor/common/editorGroupFinder.ts @@ -109,12 +109,15 @@ function doFindGroup(input: EditorInputWithOptions | IUntypedEditorInput, prefer } } + // moveToActiveGroupIfOpen disables revealIfOpen here and performs a superset of its behavior in editorResolverService + const revealIfOpen = !configurationService.getValue('workbench.editor.moveToActiveGroupIfOpen') && configurationService.getValue('workbench.editor.revealIfOpen'); + // Respect option to reveal an editor if it is open (not necessarily visible) // Still prefer to reveal an editor in a group where the editor is active though. // We also try to reveal an editor if it has the `Singleton` capability which // indicates that the same editor cannot be opened across groups. if (!group) { - if (options?.revealIfOpened || configurationService.getValue('workbench.editor.revealIfOpen') || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { + if (options?.revealIfOpened || revealIfOpen || (isEditorInput(editor) && editor.hasCapability(EditorInputCapabilities.Singleton))) { let groupWithInputActive: IEditorGroup | undefined = undefined; let groupWithInputOpened: IEditorGroup | undefined = undefined;