Skip to content

Commit daaf263

Browse files
author
Benjamin Pasero
committed
better fix for microsoft#79798 
1 parent 3cb1da6 commit daaf263

4 files changed

Lines changed: 105 additions & 91 deletions

File tree

‎src/vs/platform/editor/common/editor.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ export interface IEditorOptions {
8787
*/
8888
readonly preserveFocus?: boolean;
8989

90+
/**
91+
* Tells the group the editor opens in to become active. By default, an editor group will not
92+
* become active if either `preserveFocus: true` or `inactive: true`.
93+
*/
94+
readonly forceActive?: boolean;
95+
9096
/**
9197
* Tells the editor to reload the editor input in the editor even if it is identical to the one
9298
* already showing. By default, the editor will not reload the input if it is identical to the

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -831,19 +831,28 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
831831
openEditorOptions.active = true;
832832
}
833833

834-
if (openEditorOptions.active) {
835-
// Set group active unless we are instructed to preserveFocus. Always
836-
// make sure to restore a minimized group though in order to fix
837-
// https://github.com/microsoft/vscode/issues/79633
838-
//
839-
// Do this before we open the editor in the group to prevent a false
840-
// active editor change event before the editor is loaded
841-
// (see https://github.com/Microsoft/vscode/issues/51679)
842-
if (options && options.preserveFocus) {
843-
this.accessor.restoreGroup(this);
844-
} else {
845-
this.accessor.activateGroup(this);
846-
}
834+
let activateGroup = false;
835+
let restoreGroup = false;
836+
837+
if (options && options.forceActive) {
838+
// Always respect option to force activate an editor group.
839+
activateGroup = true;
840+
} else if (openEditorOptions.active) {
841+
// Otherwise, we only activate/restore an editor which is
842+
// opening as active editor.
843+
// If preserveFocus is enabled, we only restore but never
844+
// activate the group.
845+
activateGroup = !options || !options.preserveFocus;
846+
restoreGroup = !activateGroup;
847+
}
848+
849+
// Do this before we open the editor in the group to prevent a false
850+
// active editor change event before the editor is loaded
851+
// (see https://github.com/Microsoft/vscode/issues/51679)
852+
if (activateGroup) {
853+
this.accessor.activateGroup(this);
854+
} else if (restoreGroup) {
855+
this.accessor.restoreGroup(this);
847856
}
848857

849858
// Actually move the editor if a specific index is provided and we figure

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

Lines changed: 64 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -707,15 +707,7 @@ export class EditorOptions implements IEditorOptions {
707707
*/
708708
static create(settings: IEditorOptions): EditorOptions {
709709
const options = new EditorOptions();
710-
711-
options.preserveFocus = settings.preserveFocus;
712-
options.forceReload = settings.forceReload;
713-
options.revealIfVisible = settings.revealIfVisible;
714-
options.revealIfOpened = settings.revealIfOpened;
715-
options.pinned = settings.pinned;
716-
options.index = settings.index;
717-
options.inactive = settings.inactive;
718-
options.ignoreError = settings.ignoreError;
710+
options.overwrite(settings);
719711

720712
return options;
721713
}
@@ -726,6 +718,12 @@ export class EditorOptions implements IEditorOptions {
726718
*/
727719
preserveFocus: boolean | undefined;
728720

721+
/**
722+
* Tells the group the editor opens in to become active. By default, an editor group will not
723+
* become active if either `preserveFocus: true` or `inactive: true`.
724+
*/
725+
forceActive: boolean | undefined;
726+
729727
/**
730728
* Tells the editor to reload the editor input in the editor even if it is identical to the one
731729
* 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 {
765763
* message as needed. By default, an error will be presented as notification if opening was not possible.
766764
*/
767765
ignoreError: boolean | undefined;
766+
767+
/**
768+
* Overwrites option values from the provided bag.
769+
*/
770+
overwrite(options: IEditorOptions): EditorOptions {
771+
if (options.forceReload) {
772+
this.forceReload = true;
773+
}
774+
775+
if (options.revealIfVisible) {
776+
this.revealIfVisible = true;
777+
}
778+
779+
if (options.revealIfOpened) {
780+
this.revealIfOpened = true;
781+
}
782+
783+
if (options.preserveFocus) {
784+
this.preserveFocus = true;
785+
}
786+
787+
if (options.forceActive) {
788+
this.forceActive = true;
789+
}
790+
791+
if (options.pinned) {
792+
this.pinned = true;
793+
}
794+
795+
if (options.inactive) {
796+
this.inactive = true;
797+
}
798+
799+
if (options.ignoreError) {
800+
this.ignoreError = true;
801+
}
802+
803+
if (typeof options.index === 'number') {
804+
this.index = options.index;
805+
}
806+
807+
return this;
808+
}
768809
}
769810

770811
/**
@@ -792,53 +833,31 @@ export class TextEditorOptions extends EditorOptions {
792833
*/
793834
static create(options: ITextEditorOptions = Object.create(null)): TextEditorOptions {
794835
const textEditorOptions = new TextEditorOptions();
836+
textEditorOptions.overwrite(options);
837+
838+
return textEditorOptions;
839+
}
840+
841+
/**
842+
* Overwrites option values from the provided bag.
843+
*/
844+
overwrite(options: ITextEditorOptions): TextEditorOptions {
845+
super.overwrite(options);
795846

796847
if (options.selection) {
797848
const selection = options.selection;
798-
textEditorOptions.selection(selection.startLineNumber, selection.startColumn, selection.endLineNumber, selection.endColumn);
849+
this.selection(selection.startLineNumber, selection.startColumn, selection.endLineNumber, selection.endColumn);
799850
}
800851

801852
if (options.viewState) {
802-
textEditorOptions.editorViewState = options.viewState as IEditorViewState;
803-
}
804-
805-
if (options.forceReload) {
806-
textEditorOptions.forceReload = true;
807-
}
808-
809-
if (options.revealIfVisible) {
810-
textEditorOptions.revealIfVisible = true;
811-
}
812-
813-
if (options.revealIfOpened) {
814-
textEditorOptions.revealIfOpened = true;
815-
}
816-
817-
if (options.preserveFocus) {
818-
textEditorOptions.preserveFocus = true;
853+
this.editorViewState = options.viewState as IEditorViewState;
819854
}
820855

821856
if (options.revealInCenterIfOutsideViewport) {
822-
textEditorOptions.revealInCenterIfOutsideViewport = true;
823-
}
824-
825-
if (options.pinned) {
826-
textEditorOptions.pinned = true;
827-
}
828-
829-
if (options.inactive) {
830-
textEditorOptions.inactive = true;
857+
this.revealInCenterIfOutsideViewport = true;
831858
}
832859

833-
if (options.ignoreError) {
834-
textEditorOptions.ignoreError = true;
835-
}
836-
837-
if (typeof options.index === 'number') {
838-
textEditorOptions.index = options.index;
839-
}
840-
841-
return textEditorOptions;
860+
return this;
842861
}
843862

844863
/**

‎src/vs/workbench/services/editor/browser/editorService.ts‎

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ export class EditorService extends Disposable implements EditorServiceImpl {
225225
let resolvedGroup: IEditorGroup | undefined;
226226
let candidateGroup: OpenInEditorGroup | undefined;
227227

228-
let typedEditor: IEditorInput | undefined;
229-
let typedOptions: IEditorOptions | undefined;
228+
let typedEditor: EditorInput | undefined;
229+
let typedOptions: EditorOptions | undefined;
230230

231231
// Typed Editor Support
232232
if (editor instanceof EditorInput) {
@@ -250,31 +250,22 @@ export class EditorService extends Disposable implements EditorServiceImpl {
250250
}
251251

252252
if (typedEditor && resolvedGroup) {
253-
const control = await this.doOpenEditor(resolvedGroup, typedEditor, typedOptions);
254-
255-
this.ensureGroupActive(resolvedGroup, candidateGroup);
253+
// Unless the editor opens as inactive editor or we are instructed to open a side group,
254+
// ensure that the group gets activated even if preserveFocus: true.
255+
//
256+
// Not enforcing this for side groups supports a historic scenario we have: repeated
257+
// Alt-clicking of files in the explorer always open into the same side group and not
258+
// cause a group to be created each time.
259+
if (typedOptions && !typedOptions.inactive && typedOptions.preserveFocus && candidateGroup !== SIDE_GROUP) {
260+
typedOptions.overwrite({ forceActive: true });
261+
}
256262

257-
return control;
263+
return this.doOpenEditor(resolvedGroup, typedEditor, typedOptions);
258264
}
259265

260266
return undefined;
261267
}
262268

263-
private ensureGroupActive(resolvedGroup: IEditorGroup, candidateGroup?: OpenInEditorGroup): void {
264-
265-
// Ensure we activate the group the editor opens in unless already active. Typically
266-
// an editor always opens in the active group, but there are some cases where the
267-
// target group is not the active one. If `preserveFocus: true` we do not activate
268-
// the target group and as such have to do this manually.
269-
// There is one exception: opening to the side with `preserveFocus: true` will keep
270-
// the current behaviour for historic reasons. The scenario is that repeated Alt-clicking
271-
// of files in the explorer always open into the same side group and not cause a group
272-
// to be created each time.
273-
if (this.editorGroupService.activeGroup !== resolvedGroup && candidateGroup !== SIDE_GROUP) {
274-
this.editorGroupService.activateGroup(resolvedGroup);
275-
}
276-
}
277-
278269
protected async doOpenEditor(group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions): Promise<IEditor | undefined> {
279270
return withNullAsUndefined(await group.openEditor(editor, options));
280271
}
@@ -410,22 +401,11 @@ export class EditorService extends Disposable implements EditorServiceImpl {
410401

411402
// Open in target groups
412403
const result: Promise<IEditor | null>[] = [];
413-
let firstGroup: IEditorGroup | undefined;
414404
mapGroupToEditors.forEach((editorsWithOptions, group) => {
415-
if (!firstGroup) {
416-
firstGroup = group;
417-
}
418-
419405
result.push(group.openEditors(editorsWithOptions));
420406
});
421407

422-
const openedEditors = await Promise.all(result);
423-
424-
if (firstGroup) {
425-
this.ensureGroupActive(firstGroup, group);
426-
}
427-
428-
return coalesce(openedEditors);
408+
return coalesce(await Promise.all(result));
429409
}
430410

431411
//#endregion

0 commit comments

Comments
 (0)