Clean up some editor input debt and lifecycle issues (#24439)

extensions/vscode-api-tests/src/editor.test.ts

@@ -128,12 +128,18 @@ suite('editor tests', () => {
 			});
 
 			return Promise.all([
-				commands.executeCommand('workbench.action.closeAllEditors'),
+				delay(800).then(() => commands.executeCommand('workbench.action.closeAllEditors')), // TODO@Ben TODO@Joh this delay is a hack
 				p
 			]).then(() => undefined);
 		});
 	});
 
+	function delay(time) {
+		return new Promise(function (fulfill) {
+			setTimeout(fulfill, time);
+		});
+	}
+
 	test('issue #20867: vscode.window.visibleTextEditors returns closed document 2/2', () => {
 
 		const file10Path = join(workspace.rootPath || '', './10linefile.ts');
@@ -165,8 +171,11 @@ suite('editor tests', () => {
 
 			// hide doesn't what it means because it triggers a close event and because it
 			// detached the editor. For this test that's what we want.
-			editors[0].hide();
-			return p;
+			delay(800).then(() => { // TODO@Ben TODO@Joh this delay is a hack
+				editors[0].hide();
+
+				return p;
+			});
 		});
 	});
 

extensions/vscode-api-tests/src/workspace.test.ts

@@ -60,7 +60,7 @@ suite('workspace-namespace', () => {
 
 	test('openTextDocument', () => {
 		let len = workspace.textDocuments.length;
-		return workspace.openTextDocument(join(workspace.rootPath || '', './far.js')).then(doc => {
+		return workspace.openTextDocument(join(workspace.rootPath || '', './simple.txt')).then(doc => {
 			assert.ok(doc);
 			assert.equal(workspace.textDocuments.length, len + 1);
 		});

extensions/vscode-api-tests/testWorkspace/simple.txt

@@ -0,0 +1 @@
+Just a simple file...

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

@@ -69,6 +69,29 @@ export interface IResourceInput extends IBaseResourceInput {
 	encoding?: string;
 }
 
+export interface IUntitledResourceInput extends IBaseResourceInput {
+
+	/**
+	 * Optional resource. If the resource is not provided a new untitled file is created.
+	 */
+	resource?: URI;
+
+	/**
+	 * Optional file path. Using the file resource will associate the file to the untitled resource.
+	 */
+	filePath?: string;
+
+	/**
+	 * Optional language of the untitled resource.
+	 */
+	language?: string;
+
+	/**
+	 * Optional contents of the untitled resource.
+	 */
+	contents?: string;
+}
+
 export interface IResourceDiffInput extends IBaseResourceInput {
 
 	/**

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

@@ -11,7 +11,7 @@ import types = require('vs/base/common/types');
 import { Builder } from 'vs/base/browser/builder';
 import { Registry } from 'vs/platform/platform';
 import { Panel } from 'vs/workbench/browser/panel';
-import { EditorInput, IFileEditorInput, EditorOptions, IEditorDescriptor, IEditorInputFactory, IEditorRegistry, Extensions } from 'vs/workbench/common/editor';
+import { EditorInput, EditorOptions, IEditorDescriptor, IEditorInputFactory, IEditorRegistry, Extensions, IFileInputFactory } from 'vs/workbench/common/editor';
 import { IEditor, Position, POSITIONS } from 'vs/platform/editor/common/editor';
 import { IInstantiationService, IConstructorSignature0 } from 'vs/platform/instantiation/common/instantiation';
 import { SyncDescriptor, AsyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
@@ -160,7 +160,7 @@ const INPUT_DESCRIPTORS_PROPERTY = '__$inputDescriptors';
 class EditorRegistry implements IEditorRegistry {
 	private editors: EditorDescriptor[];
 	private instantiationService: IInstantiationService;
-	private defaultFileInputDescriptor: AsyncDescriptor<IFileEditorInput>;
+	private fileInputFactory: IFileInputFactory;
 	private editorInputFactoryConstructors: { [editorInputId: string]: IConstructorSignature0<IEditorInputFactory> } = Object.create(null);
 	private editorInputFactoryInstances: { [editorInputId: string]: IEditorInputFactory } = Object.create(null);
 
@@ -283,12 +283,12 @@ class EditorRegistry implements IEditorRegistry {
 		return inputClasses;
 	}
 
-	public registerDefaultFileInput(editorInputDescriptor: AsyncDescriptor<IFileEditorInput>): void {
-		this.defaultFileInputDescriptor = editorInputDescriptor;
+	public registerFileInputFactory(factory: IFileInputFactory): void {
+		this.fileInputFactory = factory;
 	}
 
-	public getDefaultFileInput(): AsyncDescriptor<IFileEditorInput> {
-		return this.defaultFileInputDescriptor;
+	public getFileInputFactory(): IFileInputFactory {
+		return this.fileInputFactory;
 	}
 
 	public registerEditorInputFactory(editorInputId: string, ctor: IConstructorSignature0<IEditorInputFactory>): void {

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

@@ -8,7 +8,6 @@ import { Registry } from 'vs/platform/platform';
 import nls = require('vs/nls');
 import URI from 'vs/base/common/uri';
 import { Action, IAction } from 'vs/base/common/actions';
-import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
 import { IEditorQuickOpenEntry, IQuickOpenRegistry, Extensions as QuickOpenExtensions, QuickOpenHandlerDescriptor } from 'vs/workbench/browser/quickopen';
 import { StatusbarItemDescriptor, StatusbarAlignment, IStatusbarRegistry, Extensions as StatusExtensions } from 'vs/workbench/browser/parts/statusbar/statusbar';
 import { EditorDescriptor } from 'vs/workbench/browser/parts/editor/baseEditor';
@@ -37,6 +36,7 @@ import {
 	NAVIGATE_IN_GROUP_TWO_PREFIX, ShowEditorsInGroupThreeAction, NAVIGATE_IN_GROUP_THREE_PREFIX, FocusLastEditorInStackAction, OpenNextRecentlyUsedEditorInGroupAction, MoveEditorToPreviousGroupAction, MoveEditorToNextGroupAction, MoveEditorLeftInGroupAction, ClearRecentFilesAction
 } from 'vs/workbench/browser/parts/editor/editorActions';
 import * as editorCommands from 'vs/workbench/browser/parts/editor/editorCommands';
+import { IWorkbenchEditorService } from "vs/workbench/services/editor/common/editorService";
 
 // Register String Editor
 Registry.as<IEditorRegistry>(EditorExtensions.Editors).registerEditor(
@@ -101,7 +101,6 @@ interface ISerializedUntitledEditorInput {
 class UntitledEditorInputFactory implements IEditorInputFactory {
 
 	constructor(
-		@IUntitledEditorService private untitledEditorService: IUntitledEditorService,
 		@ITextFileService private textFileService: ITextFileService
 	) {
 	}
@@ -127,10 +126,15 @@ class UntitledEditorInputFactory implements IEditorInputFactory {
 		return JSON.stringify(serialized);
 	}
 
-	public deserialize(instantiationService: IInstantiationService, serializedEditorInput: string): EditorInput {
-		const deserialized: ISerializedUntitledEditorInput = JSON.parse(serializedEditorInput);
+	public deserialize(instantiationService: IInstantiationService, serializedEditorInput: string): UntitledEditorInput {
+		return instantiationService.invokeFunction<UntitledEditorInput>(accessor => {
+			const deserialized: ISerializedUntitledEditorInput = JSON.parse(serializedEditorInput);
+			const resource = !!deserialized.resourceJSON ? URI.revive(deserialized.resourceJSON) : URI.parse(deserialized.resource);
+			const filePath = resource.scheme === 'file' ? resource.fsPath : void 0;
+			const language = deserialized.modeId;
 
-		return this.untitledEditorService.createOrGet(!!deserialized.resourceJSON ? URI.revive(deserialized.resourceJSON) : URI.parse(deserialized.resource), deserialized.modeId);
+			return accessor.get(IWorkbenchEditorService).createInput({ resource, filePath, language }) as UntitledEditorInput;
+		});
 	}
 }
 

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

@@ -14,7 +14,7 @@ import { isMacintosh } from 'vs/base/common/platform';
 import { MIME_BINARY } from 'vs/base/common/mime';
 import { shorten } from 'vs/base/common/labels';
 import { ActionRunner, IAction } from 'vs/base/common/actions';
-import { Position, IEditorInput, Verbosity } from 'vs/platform/editor/common/editor';
+import { Position, IEditorInput, Verbosity, IUntitledResourceInput } from 'vs/platform/editor/common/editor';
 import { IEditorGroup, toResource } from 'vs/workbench/common/editor';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyCode } from 'vs/base/common/keyCodes';
@@ -23,7 +23,6 @@ import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar';
 import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
 import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService';
-import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
 import { IMessageService } from 'vs/platform/message/common/message';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
@@ -66,7 +65,6 @@ export class TabsTitleControl extends TitleControl {
 		@IInstantiationService instantiationService: IInstantiationService,
 		@IWorkbenchEditorService editorService: IWorkbenchEditorService,
 		@IEditorGroupService editorGroupService: IEditorGroupService,
-		@IUntitledEditorService private untitledEditorService: IUntitledEditorService,
 		@IContextKeyService contextKeyService: IContextKeyService,
 		@IKeybindingService keybindingService: IKeybindingService,
 		@ITelemetryService telemetryService: ITelemetryService,
@@ -143,7 +141,7 @@ export class TabsTitleControl extends TitleControl {
 
 				const group = this.context;
 				if (group) {
-					this.editorService.openEditor(this.untitledEditorService.createOrGet(), { pinned: true, index: group.count /* always at the end */ }).done(null, errors.onUnexpectedError); // untitled are always pinned
+					this.editorService.openEditor({ options: { pinned: true, index: group.count /* always at the end */ } } as IUntitledResourceInput).done(null, errors.onUnexpectedError); // untitled are always pinned
 				}
 			}
 		}));

src/vs/workbench/common/editor.ts

@@ -14,7 +14,7 @@ import { IEditor, ICommonCodeEditor, IEditorViewState, IEditorOptions as ICodeEd
 import { IEditorInput, IEditorModel, IEditorOptions, ITextEditorOptions, IBaseResourceInput, Position, Verbosity } from 'vs/platform/editor/common/editor';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IEditorGroupService } from 'vs/workbench/services/group/common/groupService';
-import { SyncDescriptor, AsyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
+import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
 import { IInstantiationService, IConstructorSignature0 } from 'vs/platform/instantiation/common/instantiation';
 import { RawContextKey } from 'vs/platform/contextkey/common/contextkey';
 
@@ -49,6 +49,10 @@ export const TEXT_DIFF_EDITOR_ID = 'workbench.editors.textDiffEditor';
  */
 export const BINARY_DIFF_EDITOR_ID = 'workbench.editors.binaryResourceDiffEditor';
 
+export interface IFileInputFactory {
+	createFileInput(resource: URI, encoding: string, instantiationService: IInstantiationService): IFileEditorInput;
+}
+
 export interface IEditorRegistry {
 
 	/**
@@ -79,20 +83,14 @@ export interface IEditorRegistry {
 	getEditors(): IEditorDescriptor[];
 
 	/**
-	 * Registers the default input to be used for files in the workbench.
-	 *
-	 * @param editorInputDescriptor a descriptor that resolves to an instance of EditorInput that
-	 * should be used to handle file inputs.
+	 * Registers the file input factory to use for file inputs.
 	 */
-	registerDefaultFileInput(editorInputDescriptor: AsyncDescriptor<IFileEditorInput>): void;
+	registerFileInputFactory(factory: IFileInputFactory): void;
 
 	/**
-	 * Returns a descriptor of the default input to be used for files in the workbench.
-	 *
-	 * @return a descriptor that resolves to an instance of EditorInput that should be used to handle
-	 * file inputs.
+	 * Returns the file input factory to use for file inputs.
 	 */
-	getDefaultFileInput(): AsyncDescriptor<IFileEditorInput>;
+	getFileInputFactory(): IFileInputFactory;
 
 	/**
 	 * Registers a editor input factory for the given editor input to the registry. An editor input factory
@@ -329,11 +327,6 @@ export interface IFileEditorInput extends IEditorInput, IEncodingSupport {
 	 */
 	getResource(): URI;
 
-	/**
-	 * Sets the absolute file resource URI this input is about.
-	 */
-	setResource(resource: URI): void;
-
 	/**
 	 * Sets the preferred encodingt to use for this input.
 	 */
@@ -851,7 +844,6 @@ export interface IEditorStacksModel {
 	next(jumpGroups: boolean, cycleAtEnd?: boolean): IEditorIdentifier;
 	previous(jumpGroups: boolean, cycleAtStart?: boolean): IEditorIdentifier;
 
-	isOpen(editor: IEditorInput): boolean;
 	isOpen(resource: URI): boolean;
 
 	toString(): string;
@@ -869,8 +861,7 @@ export interface IEditorGroup {
 	getEditor(resource: URI): IEditorInput;
 	indexOf(editor: IEditorInput): number;
 
-	contains(editor: IEditorInput): boolean;
-	contains(resource: URI): boolean;
+	contains(editorOrResource: IEditorInput | URI): boolean;
 
 	getEditors(mru?: boolean): IEditorInput[];
 	isActive(editor: IEditorInput): boolean;

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

@@ -587,14 +587,12 @@ export class EditorGroup implements IEditorGroup {
 		return -1;
 	}
 
-	public contains(candidate: EditorInput): boolean;
-	public contains(resource: URI): boolean;
-	public contains(arg1: any): boolean {
-		if (arg1 instanceof EditorInput) {
-			return this.indexOf(arg1) >= 0;
+	public contains(editorOrResource: EditorInput | URI): boolean {
+		if (editorOrResource instanceof EditorInput) {
+			return this.indexOf(editorOrResource) >= 0;
 		}
 
-		const counter = this.mapResourceToEditorCount.get(arg1);
+		const counter = this.mapResourceToEditorCount.get(editorOrResource);
 
 		return typeof counter === 'number' && counter > 0;
 	}
@@ -1174,32 +1172,28 @@ export class EditorStacksModel implements IEditorStacksModel {
 
 	private handleOnEditorClosed(event: GroupEvent): void {
 		const editor = event.editor;
+		const editorsToClose = [editor];
 
-		// Close the editor when it is no longer open in any group
-		if (!this.isOpen(editor)) {
-			editor.close();
+		// Include both sides of side by side editors when being closed and not opened multiple times
+		if (editor instanceof SideBySideEditorInput && !this.isOpen(editor)) {
+			editorsToClose.push(editor.master, editor.details);
+		}
 
-			// Also take care of side by side editor inputs that wrap around 2 editors
-			if (editor instanceof SideBySideEditorInput) {
-				[editor.master, editor.details].forEach(editor => {
-					if (!this.isOpen(editor)) {
-						editor.close();
-					}
-				});
+		// Close the editor when it is no longer open in any group including diff editors
+		editorsToClose.forEach(editorToClose => {
+			const resource = toResource(editorToClose); // prefer resource to not close right-hand side editors of a diff editor
+			if (!this.isOpen(resource || editorToClose)) {
+				editorToClose.close();
 			}
-		}
+		});
 	}
 
-	public isOpen(resource: URI): boolean;
-	public isOpen(editor: EditorInput): boolean;
-	public isOpen(arg1: any): boolean {
-		return this._groups.some(group => group.contains(arg1));
+	public isOpen(editorOrResource: URI | EditorInput): boolean {
+		return this._groups.some(group => group.contains(editorOrResource));
 	}
 
-	public count(resource: URI): number;
-	public count(editor: EditorInput): number;
-	public count(arg1: any): number {
-		return this._groups.filter(group => group.contains(arg1)).length;
+	public count(editor: EditorInput): number {
+		return this._groups.filter(group => group.contains(editor)).length;
 	}
 
 	private onShutdown(): void {