editors - tweak options around group activation more

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

@@ -82,17 +82,33 @@ export interface IResourceInput extends IBaseResourceInput {
 export interface IEditorOptions {
 
 	/**
-	 * Tells the editor to not receive keyboard focus when the editor is being opened. This
-	 * will also prevent the group the editor opens in to become active. This can be overridden
-	 * via the `forceActive` option.
+	 * Tells the editor to not receive keyboard focus when the editor is being opened.
 	 *
-	 * By default, the editor will receive keyboard focus on open.
+	 * Will also not activate the group the editor opens in unless the group is already the active one.
+	 * This behaviour can be overridden via the `forceActive` option.
 	 */
 	readonly preserveFocus?: boolean;
 
 	/**
-	 * Tells the group the editor opens in to become active even if either `preserveFocus: true`
-	 * or `inactive: true` are specified.
+	 * This option is only relevant if an editor is opened into a group that is not active. In
+	 * most cases opening an editor into an inactive group will cause it to become active. With
+	 * `preserveActive: true` in combination with `preserveFocus: true` an editor can be opened
+	 * while keeping the current group active.
+	 *
+	 * Other options like `preserveFocus`, `inactive` and `forceActive` can have an impact on
+	 * whether a group gets activated or not.
+	 *
+	 * Note: `forceActive: true` will always win over `preserveActive: true`.
+	 */
+	readonly preserveActive?: boolean;
+
+	/**
+	 * This option is only relevant if an editor is opened into a group that is not active. In
+	 * most cases opening an editor into an inactive group will cause it to become active. With
+	 * `forceActive` the inactive group will become active independent of other options such as
+	 * `preserveFocus` or `inactive`.
+	 *
+	 * Note: `forceActive: true` will always win over `preserveActive: true`.
 	 */
 	readonly forceActive?: boolean;
 
@@ -132,8 +148,10 @@ export interface IEditorOptions {
 
 	/**
 	 * An active editor that is opened will show its contents directly. Set to true to open an editor
-	 * in the background. This will also prevent the group the editor opens in to become active. This
-	 * can be overridden via the `forceActive` option.
+	 * in the background without loading its contents.
+	 *
+	 * Will also not activate the group the editor opens in unless the group is already the active one.
+	 * This behaviour can be overridden via the `forceActive` option.
 	 */
 	readonly inactive?: boolean;
 

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

@@ -835,10 +835,13 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		let restoreGroup = false;
 
 		if (options && options.forceActive) {
-			// Always respect option to force activate an editor group.
+			// First, respect option to force activate an editor group.
 			activateGroup = true;
+		} else if (options && options.preserveActive) {
+			// Second, respect option to preserve active editor group.
+			activateGroup = false;
 		} else if (openEditorOptions.active) {
-			// Otherwise, we only activate/restore an editor which is
+			// Third, we only activate/restore an editor which is
 			// opening as active editor.
 			// If preserveFocus is enabled, we only restore but never
 			// activate the group.

src/vs/workbench/browser/parts/editor/textDiffEditor.ts

@@ -172,6 +172,12 @@ export class TextDiffEditor extends BaseTextEditor implements ITextDiffEditor {
 				modifiedInput.setForceOpenAsBinary();
 			}
 
+			// Make sure to not steal away the currently active group
+			// because we are triggering another openEditor() call
+			// and do not control the initial intent that resulted
+			// in us now opening as binary.
+			options.overwrite({ preserveActive: true, forceActive: false });
+
 			this.editorService.openEditor(binaryDiffInput, options, this.group);
 
 			return true;

src/vs/workbench/common/editor.ts

@@ -713,17 +713,33 @@ export class EditorOptions implements IEditorOptions {
 	}
 
 	/**
-	 * Tells the editor to not receive keyboard focus when the editor is being opened. This
-	 * will also prevent the group the editor opens in to become active. This can be overridden
-	 * via the `forceActive` option.
+	 * Tells the editor to not receive keyboard focus when the editor is being opened.
 	 *
-	 * By default, the editor will receive keyboard focus on open.
+	 * Will also not activate the group the editor opens in unless the group is already the active one.
+	 * This behaviour can be overridden via the `forceActive` option.
 	 */
 	preserveFocus: boolean | undefined;
 
 	/**
-	 * Tells the group the editor opens in to become active even if either `preserveFocus: true`
-	 * or `inactive: true` are specified.
+	 * This option is only relevant if an editor is opened into a group that is not active. In
+	 * most cases opening an editor into an inactive group will cause it to become active. With
+	 * `preserveActive: true` in combination with `preserveFocus: true` an editor can be opened
+	 * while keeping the current group active.
+	 *
+	 * Other options like `preserveFocus`, `inactive` and `forceActive` can have an impact on
+	 * whether a group gets activated or not.
+	 *
+	 * Note: `forceActive: true` will always win over `preserveActive: true`.
+	 */
+	preserveActive: boolean | undefined;
+
+	/**
+	 * This option is only relevant if an editor is opened into a group that is not active. In
+	 * most cases opening an editor into an inactive group will cause it to become active. With
+	 * `forceActive` the inactive group will become active independent of other options such as
+	 * `preserveFocus` or `inactive`.
+	 *
+	 * Note: `forceActive: true` will always win over `preserveActive: true`.
 	 */
 	forceActive: boolean | undefined;
 
@@ -757,8 +773,10 @@ export class EditorOptions implements IEditorOptions {
 
 	/**
 	 * An active editor that is opened will show its contents directly. Set to true to open an editor
-	 * in the background. This will also prevent the group the editor opens in to become active. This
-	 * can be overridden via the `forceActive` option.
+	 * in the background without loading its contents.
+	 *
+	 * Will also not activate the group the editor opens in unless the group is already the active one.
+	 * This behaviour can be overridden via the `forceActive` option.
 	 */
 	inactive: boolean | undefined;
 
@@ -788,6 +806,10 @@ export class EditorOptions implements IEditorOptions {
 			this.preserveFocus = options.preserveFocus;
 		}
 
+		if (typeof options.preserveActive === 'boolean') {
+			this.preserveActive = options.preserveActive;
+		}
+
 		if (typeof options.forceActive === 'boolean') {
 			this.forceActive = options.forceActive;
 		}

src/vs/workbench/contrib/files/browser/editors/fileEditorTracker.ts

@@ -326,7 +326,7 @@ export class FileEditorTracker extends Disposable implements IWorkbenchContribut
 
 			// Binary editor that should reload from event
 			if (resource && editor.input && isBinaryEditor && (e.contains(resource, FileChangeType.UPDATED) || e.contains(resource, FileChangeType.ADDED))) {
-				this.editorService.openEditor(editor.input, { forceReload: true, preserveFocus: true }, editor.group);
+				this.editorService.openEditor(editor.input, { forceReload: true, preserveFocus: true, preserveActive: true }, editor.group);
 			}
 		});
 	}

src/vs/workbench/contrib/files/browser/editors/textFileEditor.ts

@@ -225,6 +225,13 @@ export class TextFileEditor extends BaseTextEditor {
 
 	private openAsBinary(input: FileEditorInput, options: EditorOptions): void {
 		input.setForceOpenAsBinary();
+
+		// Make sure to not steal away the currently active group
+		// because we are triggering another openEditor() call
+		// and do not control the initial intent that resulted
+		// in us now opening as binary.
+		options.overwrite({ preserveActive: true, forceActive: false });
+
 		this.editorService.openEditor(input, options, this.group);
 	}
 

src/vs/workbench/contrib/files/browser/fileCommands.ts

@@ -225,23 +225,23 @@ async function saveAll(saveAllArguments: any, editorService: IEditorService, unt
 	// Store some properties per untitled file to restore later after save is completed
 	const groupIdToUntitledResourceInput = new Map<number, IResourceInput[]>();
 
-	editorGroupService.groups.forEach(g => {
-		const activeEditorResource = g.activeEditor && g.activeEditor.getResource();
-		g.editors.forEach(e => {
+	editorGroupService.groups.forEach(group => {
+		const activeEditorResource = group.activeEditor && group.activeEditor.getResource();
+		group.editors.forEach(e => {
 			const resource = e.getResource();
 			if (resource && untitledEditorService.isDirty(resource)) {
-				if (!groupIdToUntitledResourceInput.has(g.id)) {
-					groupIdToUntitledResourceInput.set(g.id, []);
+				if (!groupIdToUntitledResourceInput.has(group.id)) {
+					groupIdToUntitledResourceInput.set(group.id, []);
 				}
 
-				groupIdToUntitledResourceInput.get(g.id)!.push({
+				groupIdToUntitledResourceInput.get(group.id)!.push({
 					encoding: untitledEditorService.getEncoding(resource),
 					resource,
 					options: {
 						inactive: activeEditorResource ? activeEditorResource.toString() !== resource.toString() : true,
 						pinned: true,
 						preserveFocus: true,
-						index: g.getIndexOfEditor(e)
+						index: group.getIndexOfEditor(e)
 					}
 				});
 			}

src/vs/workbench/contrib/files/browser/views/explorerView.ts

@@ -241,8 +241,7 @@ export class ExplorerView extends ViewletPanel {
 			const activeFile = this.getActiveFile();
 			if (!activeFile && !focused[0].isDirectory) {
 				// Open the focused element in the editor if there is currently no file opened #67708
-				this.editorService.openEditor({ resource: focused[0].resource, options: { preserveFocus: true, revealIfVisible: true } })
-					.then(undefined, onUnexpectedError);
+				this.editorService.openEditor({ resource: focused[0].resource, options: { preserveFocus: true, revealIfVisible: true } });
 			}
 		}
 	}

src/vs/workbench/contrib/files/common/dirtyFilesTracker.ts

@@ -15,7 +15,7 @@ import { URI } from 'vs/base/common/uri';
 import { IActivityService, NumberBadge } from 'vs/workbench/services/activity/common/activity';
 import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
 import * as arrays from 'vs/base/common/arrays';
-import { IEditorService, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService';
+import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 
 export class DirtyFilesTracker extends Disposable implements IWorkbenchContribution {
 	private isDocumentedEdited: boolean;
@@ -88,7 +88,6 @@ export class DirtyFilesTracker extends Disposable implements IWorkbenchContribut
 	}
 
 	private doOpenDirtyResources(resources: URI[]): void {
-		const activeEditor = this.editorService.activeControl;
 
 		// Open
 		this.editorService.openEditors(resources.map(resource => {
@@ -96,7 +95,7 @@ export class DirtyFilesTracker extends Disposable implements IWorkbenchContribut
 				resource,
 				options: { inactive: true, pinned: true, preserveFocus: true }
 			};
-		}), activeEditor ? activeEditor.group : ACTIVE_GROUP);
+		}));
 	}
 
 	private onTextFilesSaved(e: TextFileModelChangeEvent[]): void {

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

@@ -250,18 +250,23 @@ export class EditorService extends Disposable implements EditorServiceImpl {
 		}
 
 		if (typedEditor && resolvedGroup) {
-			// 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 &&			// never for inactive editors
-				typedOptions.preserveFocus &&						// only if preserveFocus
-				typeof typedOptions.forceActive !== 'boolean' &&	// only if forceActive is not already defined (either true or false)
-				candidateGroup !== SIDE_GROUP						// never for the SIDE_GROUP
+				this.editorGroupService.activeGroup !== resolvedGroup && 	// only if target group is not already active
+				typedOptions && !typedOptions.inactive &&					// never for inactive editors
+				typedOptions.preserveFocus &&								// only if preserveFocus
+				typeof typedOptions.forceActive !== 'boolean' &&			// only if forceActive is not already defined (either true or false)
+				typeof typedOptions.preserveActive !== 'boolean' &&			// only if preserveActive is not already defined (either true or false)
+				candidateGroup !== SIDE_GROUP								// never for the SIDE_GROUP
 			) {
+				// If the resolved group is not the active one, we typically
+				// want the group to become active. There are a few cases
+				// where we stay away from encorcing this, e.g. if the caller
+				// is already providing `forceActive` or `preserveActive`.
+				//
+				// Specifically for historic reasons we do not activate a
+				// group is it is opened as `SIDE_GROUP` with `preserveFocus:true`.
+				// 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.
 				typedOptions.overwrite({ forceActive: true });
 			}
 

src/vs/workbench/services/preferences/browser/preferencesService.ts

@@ -364,7 +364,7 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 					const activeEditorGroup = this.editorGroupService.activeGroup;
 					const sideEditorGroup = this.editorGroupService.addGroup(activeEditorGroup.id, GroupDirection.RIGHT);
 					return Promise.all([
-						this.editorService.openEditor({ resource: this.defaultSettingsRawResource, options: { pinned: true, preserveFocus: true, revealIfOpened: true }, label: nls.localize('defaultSettings', "Default Settings"), description: '' }),
+						this.editorService.openEditor({ resource: this.defaultSettingsRawResource, options: { pinned: true, preserveFocus: true, preserveActive: true, revealIfOpened: true }, label: nls.localize('defaultSettings', "Default Settings"), description: '' }),
 						this.editorService.openEditor(editableSettingsEditorInput, { pinned: true, revealIfOpened: true }, sideEditorGroup.id)
 					]).then(([defaultEditor, editor]) => withNullAsUndefined(editor));
 				} else {

src/vs/workbench/services/textfile/common/textFileService.ts

@@ -685,7 +685,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
 	protected async promptForPath(resource: URI, defaultUri: URI, availableFileSystems?: string[]): Promise<URI | undefined> {
 
 		// Help user to find a name for the file by opening it first
-		await this.editorService.openEditor({ resource, options: { revealIfOpened: true, preserveFocus: true, } });
+		await this.editorService.openEditor({ resource, options: { revealIfOpened: true, preserveFocus: true } });
 
 		return this.fileDialogService.pickFileToSave(this.getSaveDialogOptions(defaultUri, availableFileSystems));
 	}