From e02ccd0715c8d8ada53eac2c83160e90f66d2b53 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 26 Aug 2019 11:32:19 +0200 Subject: [PATCH] fix #79798 --- .../contrib/files/browser/explorerViewlet.ts | 10 +-- .../files/browser/views/openEditorsView.ts | 6 +- .../contrib/search/browser/searchView.ts | 6 -- .../services/editor/browser/editorService.ts | 75 ++++++++++++++++--- .../editor/test/browser/editorService.test.ts | 2 +- 5 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/vs/workbench/contrib/files/browser/explorerViewlet.ts b/src/vs/workbench/contrib/files/browser/explorerViewlet.ts index c9713c80989..d85d35b9bcf 100644 --- a/src/vs/workbench/contrib/files/browser/explorerViewlet.ts +++ b/src/vs/workbench/contrib/files/browser/explorerViewlet.ts @@ -26,10 +26,9 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; import { DelegatingEditorService } from 'vs/workbench/services/editor/browser/editorService'; -import { IEditorGroup, IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; +import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { IEditorOptions } from 'vs/platform/editor/common/editor'; -import { IEditorInput, IEditor } from 'vs/workbench/common/editor'; +import { IEditor } from 'vs/workbench/common/editor'; import { ViewletPanel } from 'vs/workbench/browser/parts/views/panelViewlet'; import { KeyChord, KeyMod, KeyCode } from 'vs/base/common/keyCodes'; import { Registry } from 'vs/platform/registry/common/platform'; @@ -158,7 +157,6 @@ export class ExplorerViewlet extends ViewContainerViewlet { @ITelemetryService telemetryService: ITelemetryService, @IWorkspaceContextService protected contextService: IWorkspaceContextService, @IStorageService protected storageService: IStorageService, - @IEditorService private readonly editorService: IEditorService, @IEditorGroupsService private readonly editorGroupService: IEditorGroupsService, @IConfigurationService configurationService: IConfigurationService, @IInstantiationService protected instantiationService: IInstantiationService, @@ -187,7 +185,7 @@ export class ExplorerViewlet extends ViewContainerViewlet { // We try to be smart and only use the delay if we recognize that the user action is likely to cause // a new entry in the opened editors view. const delegatingEditorService = this.instantiationService.createInstance(DelegatingEditorService); - delegatingEditorService.setEditorOpenHandler(async (group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions): Promise => { + delegatingEditorService.setEditorOpenHandler(async (delegate, group, editor, options): Promise => { let openEditorsView = this.getOpenEditorsView(); if (openEditorsView) { let delay = 0; @@ -205,7 +203,7 @@ export class ExplorerViewlet extends ViewContainerViewlet { let openedEditor: IEditor | undefined; try { - openedEditor = await this.editorService.openEditor(editor, options, group); + openedEditor = await delegate(group, editor, options); } catch (error) { // ignore } finally { diff --git a/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts b/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts index 1aa98445d91..362a5add5ed 100644 --- a/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts +++ b/src/vs/workbench/contrib/files/browser/views/openEditorsView.ts @@ -357,11 +357,7 @@ export class OpenEditorsView extends ViewletPanel { if (!preserveActivateGroup) { this.editorGroupService.activateGroup(element.group); // needed for https://github.com/Microsoft/vscode/issues/6672 } - this.editorService.openEditor(element.editor, options, options.sideBySide ? SIDE_GROUP : element.group).then(editor => { - if (editor && !preserveActivateGroup && editor.group) { - this.editorGroupService.activateGroup(editor.group); - } - }); + this.editorService.openEditor(element.editor, options, options.sideBySide ? SIDE_GROUP : element.group); } } diff --git a/src/vs/workbench/contrib/search/browser/searchView.ts b/src/vs/workbench/contrib/search/browser/searchView.ts index 33e16e9f22d..975a14cf7b7 100644 --- a/src/vs/workbench/contrib/search/browser/searchView.ts +++ b/src/vs/workbench/contrib/search/browser/searchView.ts @@ -52,7 +52,6 @@ import { IReplaceService } from 'vs/workbench/contrib/search/common/replace'; import { getOutOfWorkspaceEditorResources } from 'vs/workbench/contrib/search/common/search'; import { FileMatch, FileMatchOrMatch, FolderMatch, IChangeEvent, ISearchWorkbenchService, Match, RenderableMatch, searchMatchComparer, SearchModel, SearchResult, BaseFolderMatch } from 'vs/workbench/contrib/search/common/searchModel'; import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; -import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IPreferencesService, ISettingsEditorOptions } from 'vs/workbench/services/preferences/common/preferences'; import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService'; import { relativePath } from 'vs/base/common/resources'; @@ -148,7 +147,6 @@ export class SearchView extends ViewletPanel { @IPreferencesService private readonly preferencesService: IPreferencesService, @IThemeService protected themeService: IThemeService, @ISearchHistoryService private readonly searchHistoryService: ISearchHistoryService, - @IEditorGroupsService private readonly editorGroupsService: IEditorGroupsService, @IContextMenuService contextMenuService: IContextMenuService, @IMenuService private readonly menuService: IMenuService, @IAccessibilityService private readonly accessibilityService: IAccessibilityService, @@ -1570,10 +1568,6 @@ export class SearchView extends ViewletPanel { } else { this.viewModel.searchResult.rangeHighlightDecorations.removeHighlightRange(); } - - if (editor) { - this.editorGroupsService.activateGroup(editor.group!); - } }, errors.onUnexpectedError); } diff --git a/src/vs/workbench/services/editor/browser/editorService.ts b/src/vs/workbench/services/editor/browser/editorService.ts index 78422237936..4e64d758145 100644 --- a/src/vs/workbench/services/editor/browser/editorService.ts +++ b/src/vs/workbench/services/editor/browser/editorService.ts @@ -222,28 +222,59 @@ export class EditorService extends Disposable implements EditorServiceImpl { openEditor(editor: IResourceDiffInput, group?: OpenInEditorGroup): Promise; openEditor(editor: IResourceSideBySideInput, group?: OpenInEditorGroup): Promise; async openEditor(editor: IEditorInput | IResourceEditor, optionsOrGroup?: IEditorOptions | ITextEditorOptions | OpenInEditorGroup, group?: OpenInEditorGroup): Promise { + let resolvedGroup: IEditorGroup | undefined; + let candidateGroup: OpenInEditorGroup | undefined; + + let typedEditor: IEditorInput | undefined; + let typedOptions: IEditorOptions | undefined; // Typed Editor Support if (editor instanceof EditorInput) { - const editorOptions = this.toOptions(optionsOrGroup as IEditorOptions); - const targetGroup = this.findTargetGroup(editor, editorOptions, group); + typedEditor = editor; + typedOptions = this.toOptions(optionsOrGroup as IEditorOptions); - return this.doOpenEditor(targetGroup, editor, editorOptions); + candidateGroup = group; + resolvedGroup = this.findTargetGroup(typedEditor, typedOptions, candidateGroup); } // Untyped Text Editor Support - const textInput = editor; - const typedInput = this.createInput(textInput); - if (typedInput) { - const editorOptions = TextEditorOptions.from(textInput); - const targetGroup = this.findTargetGroup(typedInput, editorOptions, optionsOrGroup as IEditorGroup | GroupIdentifier); + else { + const textInput = editor; + typedEditor = this.createInput(textInput); + if (typedEditor) { + typedOptions = TextEditorOptions.from(textInput); - return this.doOpenEditor(targetGroup, typedInput, editorOptions); + candidateGroup = optionsOrGroup as OpenInEditorGroup; + resolvedGroup = this.findTargetGroup(typedEditor, typedOptions, candidateGroup); + } + } + + if (typedEditor && resolvedGroup) { + const control = await this.doOpenEditor(resolvedGroup, typedEditor, typedOptions); + + this.ensureGroupActive(resolvedGroup, candidateGroup); + + return control; } return undefined; } + private ensureGroupActive(resolvedGroup: IEditorGroup, candidateGroup?: OpenInEditorGroup): void { + + // Ensure we activate the group the editor opens in unless already active. Typically + // an editor always opens in the active group, but there are some cases where the + // target group is not the active one. If `preserveFocus: true` we do not activate + // the target group and as such have to do this manually. + // There is one exception: opening to the side with `preserveFocus: true` will keep + // the current behaviour for historic reasons. The scenario is that repeated Alt-clicking + // of files in the explorer always open into the same side group and not cause a group + // to be created each time. + if (this.editorGroupService.activeGroup !== resolvedGroup && candidateGroup !== SIDE_GROUP) { + this.editorGroupService.activateGroup(resolvedGroup); + } + } + protected async doOpenEditor(group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions): Promise { return withNullAsUndefined(await group.openEditor(editor, options)); } @@ -377,14 +408,23 @@ export class EditorService extends Disposable implements EditorServiceImpl { }); } - // Open in targets + // Open in target groups const result: Promise[] = []; + let firstGroup: IEditorGroup | undefined; mapGroupToEditors.forEach((editorsWithOptions, group) => { + if (!firstGroup) { + firstGroup = group; + } + result.push(group.openEditors(editorsWithOptions)); }); const openedEditors = await Promise.all(result); + if (firstGroup) { + this.ensureGroupActive(firstGroup, group); + } + return coalesce(openedEditors); } @@ -625,7 +665,12 @@ export class EditorService extends Disposable implements EditorServiceImpl { } export interface IEditorOpenHandler { - (group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions | ITextEditorOptions): Promise; + ( + delegate: (group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions) => Promise, + group: IEditorGroup, + editor: IEditorInput, + options?: IEditorOptions | ITextEditorOptions + ): Promise; } /** @@ -662,7 +707,13 @@ export class DelegatingEditorService extends EditorService { return super.doOpenEditor(group, editor, options); } - const control = await this.editorOpenHandler(group, editor, options); + const control = await this.editorOpenHandler( + (group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions) => super.doOpenEditor(group, editor, options), + group, + editor, + options + ); + if (control) { return control; // the opening was handled, so return early } diff --git a/src/vs/workbench/services/editor/test/browser/editorService.test.ts b/src/vs/workbench/services/editor/test/browser/editorService.test.ts index 712b8f922b4..d2fa3c6025b 100644 --- a/src/vs/workbench/services/editor/test/browser/editorService.test.ts +++ b/src/vs/workbench/services/editor/test/browser/editorService.test.ts @@ -330,7 +330,7 @@ suite('EditorService', () => { const inp = instantiationService.createInstance(ResourceEditorInput, 'name', 'description', URI.parse('my://resource-delegate'), undefined); const delegate = instantiationService.createInstance(DelegatingEditorService); - delegate.setEditorOpenHandler((group: IEditorGroup, input: IEditorInput, options?: EditorOptions) => { + delegate.setEditorOpenHandler((delegate, group: IEditorGroup, input: IEditorInput, options?: EditorOptions) => { assert.strictEqual(input, inp); done(); -- GitLab