editor - ensure contract of openEditor()/openEditors() is respected

These methods return an IEditor as a result but even if the opening fails or the editor was not opened as active.

The fix is to check for the result of the open call and only return the editor if the editor was opened actually.

This restores the behaviour we had before GRID.

fixes #64767

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

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { GroupIdentifier, IWorkbenchEditorConfiguration, IWorkbenchEditorPartConfiguration, EditorOptions, TextEditorOptions, IEditorInput, IEditorIdentifier, IEditorCloseEvent } from 'vs/workbench/common/editor';
+import { GroupIdentifier, IWorkbenchEditorConfiguration, IWorkbenchEditorPartConfiguration, EditorOptions, TextEditorOptions, IEditorInput, IEditorIdentifier, IEditorCloseEvent, IEditor } from 'vs/workbench/common/editor';
 import { EditorGroup } from 'vs/workbench/common/editor/editorGroup';
 import { IEditorGroup, GroupDirection, IAddGroupOptions, IMergeGroupOptions, GroupsOrder, IEditorGroupsService } from 'vs/workbench/services/group/common/editorGroupsService';
 import { IDisposable } from 'vs/base/common/lifecycle';
@@ -74,9 +74,9 @@ export interface IEditorOpeningEvent extends IEditorIdentifier {
 	 * that will be executed instead. By returning another editor promise
 	 * it is possible to override the opening with another editor. It is ok
 	 * to return a promise that resolves to NULL to prevent the opening
-	 * altogether.
+	 * alltogether.
 	 */
-	prevent(callback: () => Thenable<any>): void;
+	prevent(callback: () => Thenable<IEditor>): void;
 }
 
 export interface IEditorGroupsAccessor {

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

@@ -6,7 +6,7 @@
 import 'vs/css!./media/editorgroupview';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { EditorGroup, IEditorOpenOptions, EditorCloseEvent, ISerializedEditorGroup, isSerializedEditorGroup } from 'vs/workbench/common/editor/editorGroup';
-import { EditorInput, EditorOptions, GroupIdentifier, ConfirmResult, SideBySideEditorInput, CloseDirection, IEditorCloseEvent, EditorGroupActiveEditorDirtyContext } from 'vs/workbench/common/editor';
+import { EditorInput, EditorOptions, GroupIdentifier, ConfirmResult, SideBySideEditorInput, CloseDirection, IEditorCloseEvent, EditorGroupActiveEditorDirtyContext, IEditor } from 'vs/workbench/common/editor';
 import { Event, Emitter, Relay } from 'vs/base/common/event';
 import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
 import { addClass, addClasses, Dimension, trackFocus, toggleClass, removeClass, addDisposableListener, EventType, EventHelper, findParentWithClass, clearNode, isAncestor } from 'vs/base/browser/dom';
@@ -725,11 +725,11 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 
 	//#region openEditor()
 
-	openEditor(editor: EditorInput, options?: EditorOptions): TPromise<void> {
+	openEditor(editor: EditorInput, options?: EditorOptions): TPromise<IEditor> {
 
 		// Guard against invalid inputs
 		if (!editor) {
-			return TPromise.as(void 0);
+			return TPromise.as(null);
 		}
 
 		// Editor opening event allows for prevention
@@ -744,7 +744,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		return this.doOpenEditor(editor, options);
 	}
 
-	private doOpenEditor(editor: EditorInput, options?: EditorOptions): TPromise<void> {
+	private doOpenEditor(editor: EditorInput, options?: EditorOptions): TPromise<IEditor> {
 
 		// Determine options
 		const openEditorOptions: IEditorOpenOptions = {
@@ -785,10 +785,10 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		return this.doShowEditor(editor, openEditorOptions.active, options);
 	}
 
-	private doShowEditor(editor: EditorInput, active: boolean, options?: EditorOptions): TPromise<void> {
+	private doShowEditor(editor: EditorInput, active: boolean, options?: EditorOptions): TPromise<IEditor> {
 
 		// Show in editor control if the active editor changed
-		let openEditorPromise: TPromise<void>;
+		let openEditorPromise: TPromise<IEditor>;
 		if (active) {
 			openEditorPromise = this.editorControl.openEditor(editor, options).then(result => {
 
@@ -796,13 +796,17 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 				if (result.editorChanged) {
 					this._onDidGroupChange.fire({ kind: GroupChangeKind.EDITOR_ACTIVE, editor });
 				}
+
+				return result.control;
 			}, error => {
 
 				// Handle errors but do not bubble them up
 				this.doHandleOpenEditorError(error, editor, options);
+
+				return null; // error: return NULL as result to signal this
 			});
 		} else {
-			openEditorPromise = TPromise.as(void 0);
+			openEditorPromise = TPromise.as(null); // inactive: return NULL as result to signal this
 		}
 
 		// Show in title control after editor control because some actions depend on it
@@ -844,17 +848,21 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 
 	//#region openEditors()
 
-	openEditors(editors: { editor: EditorInput, options?: EditorOptions }[]): TPromise<void> {
+	openEditors(editors: { editor: EditorInput, options?: EditorOptions }[]): TPromise<IEditor> {
 		if (!editors.length) {
-			return TPromise.as(void 0);
+			return TPromise.as(null);
 		}
 
 		// Do not modify original array
 		editors = editors.slice(0);
 
+		let result: IEditor;
+
 		// Use the first editor as active editor
 		const { editor, options } = editors.shift();
-		return this.openEditor(editor, options).then(() => {
+		return this.openEditor(editor, options).then(activeEditor => {
+			result = activeEditor; // this can be NULL if the opening failed
+
 			const startingIndex = this.getIndexOfEditor(editor) + 1;
 
 			// Open the other ones inactive
@@ -864,8 +872,12 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 				adjustedEditorOptions.pinned = true;
 				adjustedEditorOptions.index = startingIndex + index;
 
-				return this.openEditor(editor, adjustedEditorOptions);
-			})).then(() => void 0);
+				return this.openEditor(editor, adjustedEditorOptions).then(activeEditor => {
+					if (!result) {
+						result = activeEditor; // only take if the first editor opening failed
+					}
+				});
+			})).then(() => result);
 		});
 	}
 
@@ -1316,7 +1328,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 			// Forward to title control
 			this.titleAreaControl.closeEditor(activeReplacement.editor);
 
-			return openEditorResult;
+			return openEditorResult.then(() => void 0);
 		}
 
 		return TPromise.as(void 0);
@@ -1408,7 +1420,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 }
 
 class EditorOpeningEvent implements IEditorOpeningEvent {
-	private override: () => TPromise<any>;
+	private override: () => TPromise<IEditor>;
 
 	constructor(
 		private _group: GroupIdentifier,
@@ -1429,11 +1441,11 @@ class EditorOpeningEvent implements IEditorOpeningEvent {
 		return this._options;
 	}
 
-	prevent(callback: () => TPromise<any>): void {
+	prevent(callback: () => TPromise<IEditor>): void {
 		this.override = callback;
 	}
 
-	isPrevented(): () => TPromise<any> {
+	isPrevented(): () => TPromise<IEditor> {
 		return this.override;
 	}
 }

src/vs/workbench/electron-browser/window.ts

@@ -410,8 +410,8 @@ export class ElectronWindow extends Themable {
 		}
 	}
 
-	private openResources(resources: (IResourceInput | IUntitledResourceInput)[], diffMode: boolean): Thenable<any> {
-		return this.lifecycleService.when(LifecyclePhase.Ready).then(() => {
+	private openResources(resources: (IResourceInput | IUntitledResourceInput)[], diffMode: boolean): void {
+		this.lifecycleService.when(LifecyclePhase.Ready).then(() => {
 
 			// In diffMode we open 2 resources as diff
 			if (diffMode && resources.length === 2) {

src/vs/workbench/parts/comments/electron-browser/commentsPanel.ts

@@ -216,10 +216,12 @@ export class CommentsPanel extends Panel {
 					selection: range
 				}
 			}, sideBySide ? SIDE_GROUP : ACTIVE_GROUP).then(editor => {
-				const control = editor.getControl();
-				if (threadToReveal && isCodeEditor(control)) {
-					const controller = ReviewController.get(control);
-					controller.revealCommentThread(threadToReveal, commentToReveal.commentId, true);
+				if (editor) {
+					const control = editor.getControl();
+					if (threadToReveal && isCodeEditor(control)) {
+						const controller = ReviewController.get(control);
+						controller.revealCommentThread(threadToReveal, commentToReveal.commentId, true);
+					}
 				}
 			});
 		}

src/vs/workbench/parts/debug/browser/breakpointsView.ts

@@ -146,9 +146,11 @@ export class BreakpointsView extends ViewletPanel {
 			actions.push(new Action('workbench.action.debug.openEditorAndEditBreakpoint', nls.localize('editBreakpoint', "Edit {0}...", breakpointType), undefined, true, () => {
 				if (element instanceof Breakpoint) {
 					return openBreakpointSource(element, false, false, this.debugService, this.editorService).then(editor => {
-						const codeEditor = editor.getControl();
-						if (isCodeEditor(codeEditor)) {
-							codeEditor.getContribution<IDebugEditorContribution>(EDITOR_CONTRIBUTION_ID).showBreakpointWidget(element.lineNumber, element.column);
+						if (editor) {
+							const codeEditor = editor.getControl();
+							if (isCodeEditor(codeEditor)) {
+								codeEditor.getContribution<IDebugEditorContribution>(EDITOR_CONTRIBUTION_ID).showBreakpointWidget(element.lineNumber, element.column);
+							}
 						}
 					});
 				}

src/vs/workbench/parts/debug/browser/linkDetector.ts

@@ -5,7 +5,6 @@
 
 import { URI as uri } from 'vs/base/common/uri';
 import { isMacintosh } from 'vs/base/common/platform';
-import * as errors from 'vs/base/common/errors';
 import { IMouseEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent';
 import * as nls from 'vs/nls';
 import { IEditorService, SIDE_GROUP, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService';
@@ -106,6 +105,6 @@ export class LinkDetector {
 					startColumn: column
 				}
 			}
-		}, group).then(null, errors.onUnexpectedError);
+		}, group);
 	}
 }

src/vs/workbench/parts/debug/electron-browser/debugService.ts

@@ -783,7 +783,7 @@ export class DebugService implements IDebugService {
 		}
 
 		if (stackFrame) {
-			stackFrame.openInEditor(this.editorService, true).then(undefined, errors.onUnexpectedError);
+			stackFrame.openInEditor(this.editorService, true);
 			aria.alert(nls.localize('debuggingPaused', "Debugging paused {0}, {1} {2}", thread.stoppedDetails ? `, reason ${thread.stoppedDetails.reason}` : '', stackFrame.source ? stackFrame.source.name : '', stackFrame.range.startLineNumber));
 		}
 

src/vs/workbench/parts/files/electron-browser/fileActions.ts

@@ -1120,7 +1120,7 @@ export class GlobalCompareResourcesAction extends Action {
 		if (activeResource) {
 
 			// Compare with next editor that opens
-			const toDispose = this.editorService.overrideOpenEditor((editor, options, group) => {
+			const toDispose = this.editorService.overrideOpenEditor(editor => {
 
 				// Only once!
 				toDispose.dispose();

src/vs/workbench/parts/files/electron-browser/views/openEditorsView.ts

@@ -377,7 +377,7 @@ export class OpenEditorsView extends ViewletPanel {
 				this.editorGroupService.activateGroup(element.groupId); // needed for https://github.com/Microsoft/vscode/issues/6672
 			}
 			this.editorService.openEditor(element.editor, options, options.sideBySide ? SIDE_GROUP : ACTIVE_GROUP).then(editor => {
-				if (!preserveActivateGroup) {
+				if (editor && !preserveActivateGroup) {
 					this.editorGroupService.activateGroup(editor.group);
 				}
 			});

src/vs/workbench/parts/search/browser/replaceService.ts

@@ -130,9 +130,11 @@ export class ReplaceService implements IReplaceService {
 				disposable.dispose();
 			});
 			this.updateReplacePreview(fileMatch).then(() => {
-				let editorControl = editor.getControl();
-				if (element instanceof Match) {
-					editorControl.revealLineInCenter(element.range().startLineNumber, ScrollType.Immediate);
+				if (editor) {
+					let editorControl = editor.getControl();
+					if (element instanceof Match) {
+						editorControl.revealLineInCenter(element.range().startLineNumber, ScrollType.Immediate);
+					}
 				}
 			});
 		}, errors.onUnexpectedError);

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

@@ -244,7 +244,7 @@ export class EditorService extends Disposable implements EditorServiceImpl {
 	}
 
 	protected doOpenEditor(group: IEditorGroup, editor: IEditorInput, options?: IEditorOptions): TPromise<IEditor> {
-		return group.openEditor(editor, options).then(() => group.activeControl);
+		return group.openEditor(editor, options);
 	}
 
 	private findTargetGroup(input: IEditorInput, options?: IEditorOptions, group?: IEditorGroup | GroupIdentifier | SIDE_GROUP_TYPE | ACTIVE_GROUP_TYPE): IEditorGroup {
@@ -363,7 +363,7 @@ export class EditorService extends Disposable implements EditorServiceImpl {
 		// Open in targets
 		const result: TPromise<IEditor>[] = [];
 		mapGroupToEditors.forEach((editorsWithOptions, group) => {
-			result.push((group.openEditors(editorsWithOptions)).then(() => group.activeControl));
+			result.push(group.openEditors(editorsWithOptions));
 		});
 
 		return TPromise.join(result);

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

@@ -37,7 +37,7 @@ export interface IOpenEditorOverride {
 	 * If defined, will prevent the opening of an editor and replace the resulting
 	 * promise with the provided promise for the openEditor() call.
 	 */
-	override?: TPromise<any>;
+	override?: TPromise<IEditor>;
 }
 
 export interface IEditorService {
@@ -111,6 +111,9 @@ export interface IEditorService {
 	 * @param group the target group. If unspecified, the editor will open in the currently
 	 * active group. Use `SIDE_GROUP_TYPE` to open the editor in a new editor group to the side
 	 * of the currently active group.
+	 *
+	 * @returns the editor that opened or NULL if the operation failed or the editor was not
+	 * opened to be active.
 	 */
 	openEditor(editor: IEditorInput, options?: IEditorOptions | ITextEditorOptions, group?: IEditorGroup | GroupIdentifier | SIDE_GROUP_TYPE | ACTIVE_GROUP_TYPE): TPromise<IEditor>;
 	openEditor(editor: IResourceInput | IUntitledResourceInput, group?: IEditorGroup | GroupIdentifier | SIDE_GROUP_TYPE | ACTIVE_GROUP_TYPE): TPromise<ITextEditor>;
@@ -124,6 +127,9 @@ export interface IEditorService {
 	 * @param group the target group. If unspecified, the editor will open in the currently
 	 * active group. Use `SIDE_GROUP_TYPE` to open the editor in a new editor group to the side
 	 * of the currently active group.
+	 *
+	 * @returns the editors that opened. The array can be empty or have less elements for editors
+	 * that failed to open or were instructed to open as inactive.
 	 */
 	openEditors(editors: IEditorInputWithOptions[], group?: IEditorGroup | GroupIdentifier | SIDE_GROUP_TYPE | ACTIVE_GROUP_TYPE): TPromise<ReadonlyArray<IEditor>>;
 	openEditors(editors: IResourceEditor[], group?: IEditorGroup | GroupIdentifier | SIDE_GROUP_TYPE | ACTIVE_GROUP_TYPE): TPromise<ReadonlyArray<IEditor>>;

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

@@ -26,11 +26,18 @@ import { Registry } from 'vs/platform/registry/common/platform';
 import { FileEditorInput } from 'vs/workbench/parts/files/common/editors/fileEditorInput';
 import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorInput';
 import { EditorServiceImpl } from 'vs/workbench/browser/parts/editor/editor';
+import { CancellationToken } from 'vs/base/common/cancellation';
 
 export class TestEditorControl extends BaseEditor {
 
 	constructor(@ITelemetryService telemetryService: ITelemetryService) { super('MyTestEditorForEditorService', NullTelemetryService, new TestThemeService(), new TestStorageService()); }
 
+	setInput(input: EditorInput, options: EditorOptions, token: CancellationToken): Thenable<void> {
+		super.setInput(input, options, token);
+
+		return input.resolve().then(() => void 0);
+	}
+
 	getId(): string { return 'MyTestEditorForEditorService'; }
 	layout(): void { }
 	createEditor(): any { }
@@ -38,16 +45,20 @@ export class TestEditorControl extends BaseEditor {
 
 export class TestEditorInput extends EditorInput implements IFileEditorInput {
 	public gotDisposed: boolean;
+	private fails: boolean;
 	constructor(private resource: URI) { super(); }
 
 	getTypeId() { return 'testEditorInputForEditorService'; }
-	resolve(): Thenable<IEditorModel> { return Promise.resolve(null); }
+	resolve(): Thenable<IEditorModel> { return !this.fails ? Promise.resolve(null) : Promise.reject(new Error('fails')); }
 	matches(other: TestEditorInput): boolean { return other && other.resource && this.resource.toString() === other.resource.toString() && other instanceof TestEditorInput; }
 	setEncoding(encoding: string) { }
 	getEncoding(): string { return null; }
 	setPreferredEncoding(encoding: string) { }
 	getResource(): URI { return this.resource; }
 	setForceOpenAsBinary(): void { }
+	setFailToOpen(): void {
+		this.fails = true;
+	}
 	dispose(): void {
 		super.dispose();
 		this.gotDisposed = true;
@@ -386,6 +397,7 @@ suite('Editor service', () => {
 
 		// 1.) open, open same, open other, close
 		let editor = await service.openEditor(input, { pinned: true });
+		const group = editor.group;
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -397,11 +409,11 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		await editor.group.closeEditor(otherInput);
+		await group.closeEditor(otherInput);
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		await editor.group.closeEditor(input);
+		await group.closeEditor(input);
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -414,7 +426,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeEditor(input);
+		await group.closeEditor(input);
 
 		// 3.) open, open inactive, close
 		editor = await service.openEditor(input, { pinned: true });
@@ -425,7 +437,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -438,11 +450,11 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeEditor(otherInput);
+		await group.closeEditor(otherInput);
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -463,7 +475,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -484,7 +496,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -501,7 +513,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		editor.group.focus();
+		group.focus();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(false);
 
@@ -509,7 +521,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(true);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -522,11 +534,11 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		editor.group.moveEditor(otherInput, editor.group, { index: 0 });
+		group.moveEditor(otherInput, group, { index: 0 });
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(false);
 
-		await editor.group.closeAllEditors();
+		await group.closeAllEditors();
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -543,7 +555,7 @@ suite('Editor service', () => {
 		assertActiveEditorChangedEvent(true);
 		assertVisibleEditorsChangedEvent(true);
 
-		await editor.group.closeEditor(input);
+		await group.closeEditor(input);
 		assertActiveEditorChangedEvent(false);
 		assertVisibleEditorsChangedEvent(true);
 
@@ -551,6 +563,34 @@ suite('Editor service', () => {
 		activeEditorChangeListener.dispose();
 		visibleEditorChangeListener.dispose();
 	});
+
+	test('openEditor returns NULL when opening fails or is inactive', async function () {
+		const partInstantiator = workbenchInstantiationService();
+
+		const part = partInstantiator.createInstance(EditorPart, 'id', false);
+		part.create(document.createElement('div'));
+		part.layout(new Dimension(400, 300));
+
+		const testInstantiationService = partInstantiator.createChild(new ServiceCollection([IEditorGroupsService, part]));
+
+		const service: EditorServiceImpl = testInstantiationService.createInstance(EditorService);
+
+		const input = testInstantiationService.createInstance(TestEditorInput, URI.parse('my://resource-active'));
+		const otherInput = testInstantiationService.createInstance(TestEditorInput, URI.parse('my://resource2-inactive'));
+		const failingInput = testInstantiationService.createInstance(TestEditorInput, URI.parse('my://resource3-failing'));
+		failingInput.setFailToOpen();
+
+		await part.whenRestored;
+
+		let editor = await service.openEditor(input, { pinned: true });
+		assert.ok(editor);
+
+		let otherEditor = await service.openEditor(otherInput, { inactive: true });
+		assert.ok(!otherEditor);
+
+		let failingEditor = await service.openEditor(failingInput);
+		assert.ok(!failingEditor);
+	});
 });
 
 function toResource(path: string) {

src/vs/workbench/services/group/common/editorGroupsService.ts

@@ -377,18 +377,20 @@ export interface IEditorGroup {
 	/**
 	 * Open an editor in this group.
 	 *
-	 * @returns a promise that is resolved when the active editor (if any)
-	 * has finished loading
+	 * @returns a promise that resolves around an IEditor instance unless
+	 * the call failed, or the editor was not opened as active editor.
 	 */
-	openEditor(editor: IEditorInput, options?: IEditorOptions | ITextEditorOptions): TPromise<void>;
+	openEditor(editor: IEditorInput, options?: IEditorOptions | ITextEditorOptions): TPromise<IEditor>;
 
 	/**
 	 * Opens editors in this group.
 	 *
-	 * @returns a promise that is resolved when the active editor (if any)
-	 * has finished loading
+	 * @returns a promise that resolves around an IEditor instance unless
+	 * the call failed, or the editor was not opened as active editor. Since
+	 * a group can only ever have one active editor, even if many editors are
+	 * opened, the result will only be one editor.
 	 */
-	openEditors(editors: IEditorInputWithOptions[]): TPromise<void>;
+	openEditors(editors: IEditorInputWithOptions[]): TPromise<IEditor>;
 
 	/**
 	 * Find out if the provided editor is opened in the group.

src/vs/workbench/services/history/electron-browser/history.ts

@@ -30,6 +30,7 @@ import { EditorServiceImpl } from 'vs/workbench/browser/parts/editor/editor';
 import { IPartService } from 'vs/workbench/services/part/common/partService';
 import { IContextKeyService, RawContextKey, IContextKey } from 'vs/platform/contextkey/common/contextkey';
 import { coalesce } from 'vs/base/common/arrays';
+import { always } from 'vs/base/common/async';
 
 /**
  * Stores the selection & view state of an editor and allows to compare it to other selection states.
@@ -402,13 +403,7 @@ export class HistoryService extends Disposable implements IHistoryService {
 	private navigate(acrossEditors?: boolean): void {
 		this.navigatingInStack = true;
 
-		this.doNavigate(this.stack[this.index], !acrossEditors).then(() => {
-			this.navigatingInStack = false;
-		}, error => {
-			this.navigatingInStack = false;
-
-			onUnexpectedError(error);
-		});
+		always(this.doNavigate(this.stack[this.index], !acrossEditors), () => this.navigatingInStack = false);
 	}
 
 	private doNavigate(location: IStackEntry, withSelection: boolean): Thenable<IBaseEditor> {

src/vs/workbench/test/workbenchTestServices.ts

@@ -649,12 +649,12 @@ export class TestEditorGroup implements IEditorGroupView {
 		return -1;
 	}
 
-	openEditor(_editor: IEditorInput, _options?: IEditorOptions): TPromise<void> {
-		return TPromise.as(void 0);
+	openEditor(_editor: IEditorInput, _options?: IEditorOptions): TPromise<IEditor> {
+		return TPromise.as(null);
 	}
 
-	openEditors(_editors: IEditorInputWithOptions[]): TPromise<void> {
-		return TPromise.as(void 0);
+	openEditors(_editors: IEditorInputWithOptions[]): TPromise<IEditor> {
+		return TPromise.as(null);
 	}
 
 	isOpened(_editor: IEditorInput): boolean {