refine API, add closeTextDocument call

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

@@ -122,11 +122,8 @@ suite('workspace-namespace', () => {
 	test('registerTextDocumentContentProvider, simple', function() {
 
 		let registration = workspace.registerTextDocumentContentProvider('foo', {
-			open(uri) {
+			provideTextDocumentContent(uri) {
 				return uri.toString();
-			},
-			close() {
-				// nothing
 			}
 		});
 
@@ -143,29 +140,26 @@ suite('workspace-namespace', () => {
 
 		// built-in
 		assert.throws(function() {
-			workspace.registerTextDocumentContentProvider('untitled', { open() { return null; }, close() { } });
+			workspace.registerTextDocumentContentProvider('untitled', { provideTextDocumentContent() { return null; } });
 		});
 		// built-in
 		assert.throws(function() {
-			workspace.registerTextDocumentContentProvider('file', { open() { return null; }, close() { } });
+			workspace.registerTextDocumentContentProvider('file', { provideTextDocumentContent() { return null; } });
 		});
 
 		// duplicate registration
 		let registration = workspace.registerTextDocumentContentProvider('foo', {
-			open(uri) {
+			provideTextDocumentContent(uri) {
 				return uri.toString();
-			},
-			close() {
-				// nothing
 			}
 		});
 		assert.throws(function() {
-			workspace.registerTextDocumentContentProvider('foo', { open() { return null; }, close() { } });
+			workspace.registerTextDocumentContentProvider('foo', { provideTextDocumentContent() { return null; } });
 		});
 
 		// unregister & register
 		registration.dispose();
-		registration = workspace.registerTextDocumentContentProvider('foo', { open() { return null; }, close() { } });
+		registration = workspace.registerTextDocumentContentProvider('foo', { provideTextDocumentContent() { return null; } });
 		registration.dispose();
 
 		// missing scheme
@@ -178,13 +172,9 @@ suite('workspace-namespace', () => {
 
 	test('registerTextDocumentContentProvider, show virtual document', function() {
 
-		// duplicate registration
 		let registration = workspace.registerTextDocumentContentProvider('foo', {
-			open(uri) {
+			provideTextDocumentContent(uri) {
 				return 'I am virtual';
-			},
-			close() {
-				// nothing
 			}
 		});
 
@@ -198,6 +188,77 @@ suite('workspace-namespace', () => {
 		});
 	});
 
+	test('registerTextDocumentContentProvider, open/close document 1/2', function() {
+
+		let registration = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				return 'I am virtual';
+			}
+		});
+
+		const uri = Uri.parse('foo://testing/path');
+
+		return workspace.openTextDocument(uri).then(doc => {
+
+			assert.ok(workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString()));
+
+			return workspace.closeTextDocument(doc).then(value => {
+				assert.ok(value);
+				assert.ok(!workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString()));
+				registration.dispose();
+			});
+		});
+	});
+
+	test('registerTextDocumentContentProvider, open/close document 2/2', function() {
+
+		let registration = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				return 'I am virtual';
+			}
+		});
+
+		const uri = Uri.parse('foo://testing/path');
+
+		return workspace.openTextDocument(uri).then(first => {
+
+			assert.ok(workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString()));
+
+			return workspace.openTextDocument(uri).then(second => {
+				registration.dispose();
+				assert.ok(first === second);
+
+				return workspace.closeTextDocument(first).then(value => {
+					assert.ok(value);
+
+					// should we reference count or not?
+					// assert.ok(workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString()));
+				});
+			});
+		});
+	});
+
+	test('registerTextDocumentContentProvider, open/open document', function() {
+
+		let callCount = 0;
+		let registration = workspace.registerTextDocumentContentProvider('foo', {
+			provideTextDocumentContent(uri) {
+				callCount += 1;
+				return 'I am virtual';
+			}
+		});
+
+		const uri = Uri.parse('foo://testing/path');
+
+		return Promise.all([workspace.openTextDocument(uri), workspace.openTextDocument(uri)]).then(docs => {
+			let [first, second] = docs;
+			assert.ok(first === second);
+			assert.ok(workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString()));
+			assert.equal(callCount, 1);
+			registration.dispose();
+		});
+	});
+
 	test('findFiles', () => {
 		return workspace.findFiles('*.js', null).then((res) => {
 			assert.equal(res.length, 1);

src/vs/vscode.d.ts

@@ -1025,12 +1025,13 @@ declare namespace vscode {
 		onDidChange?: Event<Uri>;
 
 		/**
+		 * Provide textual content for a given uri.
 		 *
+		 * @param uri An uri which scheme matches the scheme this provider was [registered](#workspace.registerTextDocumentContentProvider) for.
+		 * @param token A cancellation token.
+		 * @return A string or a thenable that resolves to such.
 		 */
-		open(uri: Uri, token: CancellationToken): string | Thenable<string>;
-
-		// todo@joh make this optional?
-		close(uri: Uri, token: CancellationToken): any | Thenable<any>;
+		provideTextDocumentContent(uri: Uri, token: CancellationToken): string | Thenable<string>;
 	}
 
 	/**
@@ -3020,6 +3021,11 @@ declare namespace vscode {
 		 */
 		export function openTextDocument(fileName: string): Thenable<TextDocument>;
 
+		/**
+		 *
+		 */
+		export function closeTextDocument(document: TextDocument): Thenable<boolean>;
+
 		/**
 		 *
 		 */

src/vs/workbench/api/common/extHost.api.impl.ts

@@ -255,6 +255,9 @@ export class ExtHostAPIImplementation {
 			openTextDocument(uriOrFileName:vscode.Uri | string) {
 				return pluginHostDocuments.openDocument(uriOrFileName);
 			},
+			closeTextDocument(document: vscode.TextDocument) {
+				return pluginHostDocuments.closeDocument(document);
+			},
 			registerTextDocumentContentProvider(scheme: string, provider: vscode.TextDocumentContentProvider) {
 				return pluginHostDocuments.registerTextDocumentContentProvider(scheme, provider);
 			},

src/vs/workbench/api/common/extHostDocuments.ts

@@ -66,6 +66,7 @@ export class ExtHostModelService {
 	public onDidSaveDocument: Event<BaseTextDocument>;
 
 	private _documents: { [modelUri: string]: ExtHostDocument; };
+	private _loadingDocuments: { [modelUri: string]: TPromise<ExtHostDocument> };
 	private _documentContentProviders: { [scheme: string]: vscode.TextDocumentContentProvider };
 
 	private _proxy: MainThreadDocuments;
@@ -86,6 +87,7 @@ export class ExtHostModelService {
 		this.onDidSaveDocument = this._onDidSaveDocumentEventEmitter.event;
 
 		this._documents = Object.create(null);
+		this._loadingDocuments = Object.create(null);
 		this._documentContentProviders = Object.create(null);
 	}
 
@@ -119,12 +121,26 @@ export class ExtHostModelService {
 		if (cached) {
 			return TPromise.as(cached);
 		}
-		return this._proxy._tryOpenDocument(uri).then(() => {
+
+		let promise = this._loadingDocuments[uri.toString()];
+		if (promise) {
+			return promise;
+		}
+
+		return this._loadingDocuments[uri.toString()] = this._proxy._tryOpenDocument(uri).then(() => {
+			delete this._loadingDocuments[uri.toString()];
 			return this._documents[uri.toString()];
+		}, err => {
+			delete this._loadingDocuments[uri.toString()];
+			return TPromise.wrapError(err);
 		});
 	}
 
-	registerTextDocumentContentProvider(scheme: string, provider: vscode.TextDocumentContentProvider): vscode.Disposable {
+	public closeDocument(document: vscode.TextDocument): TPromise<boolean> {
+		return this._proxy._tryCloseDocument(<URI> document.uri);
+	}
+
+	public registerTextDocumentContentProvider(scheme: string, provider: vscode.TextDocumentContentProvider): vscode.Disposable {
 		if (scheme === 'file' || scheme === 'untitled' || this._documentContentProviders[scheme]) {
 			throw new Error(`scheme '${scheme}' already registered`);
 		}
@@ -132,21 +148,17 @@ export class ExtHostModelService {
 		return new Disposable(() => delete this._documentContentProviders[scheme]);
 	}
 
-	$openTextDocumentContent(uri: URI): TPromise<string> {
+	$provideTextDocumentContent(uri: URI): TPromise<string> {
 		const provider = this._documentContentProviders[uri.scheme];
 		if (!provider) {
 			return TPromise.wrapError<string>(`unsupported uri-scheme: ${uri.scheme}`);
 		}
-		// todo@joh protected for !string results, slow provider etc
-		return asWinJsPromise(token => provider.open(uri, token));
-	}
-
-	$closeTextDocumentContent(uri: URI): TPromise<any> {
-		const provider = this._documentContentProviders[uri.scheme];
-		if (!provider) {
-			return TPromise.wrapError<string>(`unsupported uri-scheme: ${uri.scheme}`);
-		}
-		return asWinJsPromise(token => provider.close(uri, token));
+		return asWinJsPromise(token => provider.provideTextDocumentContent(uri, token)).then(value => {
+			if (typeof value !== 'string') {
+				return TPromise.wrapError('received illegal value from text document provider');
+			}
+			return value;
+		});
 	}
 
 	public _acceptModelAdd(data:IModelAddedData): void {
@@ -731,10 +743,7 @@ export class MainThreadDocuments {
 			return TPromise.as(true);
 		}
 
-		return this._proxy.$openTextDocumentContent(uri).then(value => {
-			if (typeof value !== 'string') {
-				return TPromise.wrapError<boolean>('illegal value');
-			}
+		return this._proxy.$provideTextDocumentContent(uri).then(value => {
 			const firstLineText = value.substr(0, 1 + value.search(/\r?\n/));
 			const mode = this._modeService.getOrCreateModeByFilenameOrFirstLine(uri.fsPath, firstLineText);
 			return this._modelService.createModel(value, mode, uri);
@@ -743,4 +752,9 @@ export class MainThreadDocuments {
 			return true;
 		});
 	}
+
+	_tryCloseDocument(uri: URI): TPromise<boolean> {
+		this._modelService.destroyModel(uri);
+		return TPromise.as(true);
+	}
 }