From daaf263d00e3e9ba10dab439ab357e2cb8eee84b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 26 Aug 2019 13:14:37 +0200 Subject: [PATCH] better fix for #79798 --- src/vs/platform/editor/common/editor.ts | 6 + .../browser/parts/editor/editorGroupView.ts | 35 +++--- src/vs/workbench/common/editor.ts | 109 ++++++++++-------- .../services/editor/browser/editorService.ts | 46 +++----- 4 files changed, 105 insertions(+), 91 deletions(-) diff --git a/src/vs/platform/editor/common/editor.ts b/src/vs/platform/editor/common/editor.ts index c061f0eea6a..a2bafd91dca 100644 --- a/src/vs/platform/editor/common/editor.ts +++ b/src/vs/platform/editor/common/editor.ts @@ -87,6 +87,12 @@ export interface IEditorOptions { */ readonly preserveFocus?: boolean; + /** + * Tells the group the editor opens in to become active. By default, an editor group will not + * become active if either `preserveFocus: true` or `inactive: true`. + */ + readonly forceActive?: boolean; + /** * Tells the editor to reload the editor input in the editor even if it is identical to the one * already showing. By default, the editor will not reload the input if it is identical to the diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index fb1e07119c7..d9df1ba5b50 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -831,19 +831,28 @@ export class EditorGroupView extends Themable implements IEditorGroupView { openEditorOptions.active = true; } - if (openEditorOptions.active) { - // Set group active unless we are instructed to preserveFocus. Always - // make sure to restore a minimized group though in order to fix - // https://github.com/microsoft/vscode/issues/79633 - // - // Do this before we open the editor in the group to prevent a false - // active editor change event before the editor is loaded - // (see https://github.com/Microsoft/vscode/issues/51679) - if (options && options.preserveFocus) { - this.accessor.restoreGroup(this); - } else { - this.accessor.activateGroup(this); - } + let activateGroup = false; + let restoreGroup = false; + + if (options && options.forceActive) { + // Always respect option to force activate an editor group. + activateGroup = true; + } else if (openEditorOptions.active) { + // Otherwise, we only activate/restore an editor which is + // opening as active editor. + // If preserveFocus is enabled, we only restore but never + // activate the group. + activateGroup = !options || !options.preserveFocus; + restoreGroup = !activateGroup; + } + + // Do this before we open the editor in the group to prevent a false + // active editor change event before the editor is loaded + // (see https://github.com/Microsoft/vscode/issues/51679) + if (activateGroup) { + this.accessor.activateGroup(this); + } else if (restoreGroup) { + this.accessor.restoreGroup(this); } // Actually move the editor if a specific index is provided and we figure diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index 97f5aa1e79d..3b2647c3c97 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -707,15 +707,7 @@ export class EditorOptions implements IEditorOptions { */ static create(settings: IEditorOptions): EditorOptions { const options = new EditorOptions(); - - options.preserveFocus = settings.preserveFocus; - options.forceReload = settings.forceReload; - options.revealIfVisible = settings.revealIfVisible; - options.revealIfOpened = settings.revealIfOpened; - options.pinned = settings.pinned; - options.index = settings.index; - options.inactive = settings.inactive; - options.ignoreError = settings.ignoreError; + options.overwrite(settings); return options; } @@ -726,6 +718,12 @@ export class EditorOptions implements IEditorOptions { */ preserveFocus: boolean | undefined; + /** + * Tells the group the editor opens in to become active. By default, an editor group will not + * become active if either `preserveFocus: true` or `inactive: true`. + */ + forceActive: boolean | undefined; + /** * Tells the editor to reload the editor input in the editor even if it is identical to the one * already showing. By default, the editor will not reload the input if it is identical to the @@ -765,6 +763,49 @@ export class EditorOptions implements IEditorOptions { * message as needed. By default, an error will be presented as notification if opening was not possible. */ ignoreError: boolean | undefined; + + /** + * Overwrites option values from the provided bag. + */ + overwrite(options: IEditorOptions): EditorOptions { + if (options.forceReload) { + this.forceReload = true; + } + + if (options.revealIfVisible) { + this.revealIfVisible = true; + } + + if (options.revealIfOpened) { + this.revealIfOpened = true; + } + + if (options.preserveFocus) { + this.preserveFocus = true; + } + + if (options.forceActive) { + this.forceActive = true; + } + + if (options.pinned) { + this.pinned = true; + } + + if (options.inactive) { + this.inactive = true; + } + + if (options.ignoreError) { + this.ignoreError = true; + } + + if (typeof options.index === 'number') { + this.index = options.index; + } + + return this; + } } /** @@ -792,53 +833,31 @@ export class TextEditorOptions extends EditorOptions { */ static create(options: ITextEditorOptions = Object.create(null)): TextEditorOptions { const textEditorOptions = new TextEditorOptions(); + textEditorOptions.overwrite(options); + + return textEditorOptions; + } + + /** + * Overwrites option values from the provided bag. + */ + overwrite(options: ITextEditorOptions): TextEditorOptions { + super.overwrite(options); if (options.selection) { const selection = options.selection; - textEditorOptions.selection(selection.startLineNumber, selection.startColumn, selection.endLineNumber, selection.endColumn); + this.selection(selection.startLineNumber, selection.startColumn, selection.endLineNumber, selection.endColumn); } if (options.viewState) { - textEditorOptions.editorViewState = options.viewState as IEditorViewState; - } - - if (options.forceReload) { - textEditorOptions.forceReload = true; - } - - if (options.revealIfVisible) { - textEditorOptions.revealIfVisible = true; - } - - if (options.revealIfOpened) { - textEditorOptions.revealIfOpened = true; - } - - if (options.preserveFocus) { - textEditorOptions.preserveFocus = true; + this.editorViewState = options.viewState as IEditorViewState; } if (options.revealInCenterIfOutsideViewport) { - textEditorOptions.revealInCenterIfOutsideViewport = true; - } - - if (options.pinned) { - textEditorOptions.pinned = true; - } - - if (options.inactive) { - textEditorOptions.inactive = true; + this.revealInCenterIfOutsideViewport = true; } - if (options.ignoreError) { - textEditorOptions.ignoreError = true; - } - - if (typeof options.index === 'number') { - textEditorOptions.index = options.index; - } - - return textEditorOptions; + return this; } /** diff --git a/src/vs/workbench/services/editor/browser/editorService.ts b/src/vs/workbench/services/editor/browser/editorService.ts index 4e64d758145..20b9aac8069 100644 --- a/src/vs/workbench/services/editor/browser/editorService.ts +++ b/src/vs/workbench/services/editor/browser/editorService.ts @@ -225,8 +225,8 @@ export class EditorService extends Disposable implements EditorServiceImpl { let resolvedGroup: IEditorGroup | undefined; let candidateGroup: OpenInEditorGroup | undefined; - let typedEditor: IEditorInput | undefined; - let typedOptions: IEditorOptions | undefined; + let typedEditor: EditorInput | undefined; + let typedOptions: EditorOptions | undefined; // Typed Editor Support if (editor instanceof EditorInput) { @@ -250,31 +250,22 @@ export class EditorService extends Disposable implements EditorServiceImpl { } if (typedEditor && resolvedGroup) { - const control = await this.doOpenEditor(resolvedGroup, typedEditor, typedOptions); - - this.ensureGroupActive(resolvedGroup, candidateGroup); + // Unless the editor opens as inactive editor or we are instructed to open a side group, + // ensure that the group gets activated even if preserveFocus: true. + // + // Not enforcing this for side groups supports a historic scenario we have: 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 (typedOptions && !typedOptions.inactive && typedOptions.preserveFocus && candidateGroup !== SIDE_GROUP) { + typedOptions.overwrite({ forceActive: true }); + } - return control; + return this.doOpenEditor(resolvedGroup, typedEditor, typedOptions); } 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)); } @@ -410,22 +401,11 @@ export class EditorService extends Disposable implements EditorServiceImpl { // 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); + return coalesce(await Promise.all(result)); } //#endregion -- GitLab