From 1ecf01b9c71e76160c96f8b2dd89752b11f785ca Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 29 Jan 2021 12:12:06 +0100 Subject: [PATCH 01/11] mark rpc protocol and proxies with symbols and try to find them from an API test (with success...) --- .../src/singlefolder-tests/rpc.test.ts | 53 +++++++++++++++++++ .../services/extensions/common/rpcProtocol.ts | 14 +++-- 2 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts new file mode 100644 index 0000000000000..f75da5a79a93a --- /dev/null +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -0,0 +1,53 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +suite('vscode', function () { + + test('rpc protocol, proxies not reachable', function () { + + const symProxy = Symbol.for('rpcProxy'); + const symProtocol = Symbol.for('rpcProtocol'); + + const proxyPaths: string[] = []; + const rpcPaths: string[] = []; + + function walk(obj: any, path: string, seen: Set) { + if (!obj) { + return; + } + if (typeof obj !== 'object' && typeof obj !== 'function') { + return; + } + if (seen.has(obj)) { + return; + } + seen.add(obj); + + if (obj[symProtocol]) { + rpcPaths.push(`PROTOCOL via ${path}`); + } + if (obj[symProxy]) { + proxyPaths.push(`PROXY '${obj[symProxy]}' via ${path}`); + } + + for (const key in obj) { + walk(obj[key], `${path}.${String(key)}`, seen); + } + } + + try { + walk(vscode, 'vscode', new Set()); + } catch (err) { + assert.fail(err); + } + assert.strictEqual(rpcPaths.length, 0); + + // assert.strictEqual(proxyPaths.length, 0); // proxies are accessible... + }); + +}); diff --git a/src/vs/workbench/services/extensions/common/rpcProtocol.ts b/src/vs/workbench/services/extensions/common/rpcProtocol.ts index 199ea6e15ef44..a9fef74ecec25 100644 --- a/src/vs/workbench/services/extensions/common/rpcProtocol.ts +++ b/src/vs/workbench/services/extensions/common/rpcProtocol.ts @@ -60,8 +60,13 @@ export interface IRPCProtocolLogger { const noop = () => { }; +const _RPCProtocolSymbol = Symbol.for('rpcProtocol'); +const _RPCProxySymbol = Symbol.for('rpcProxy'); + export class RPCProtocol extends Disposable implements IRPCProtocol { + [_RPCProtocolSymbol] = true; + private static readonly UNRESPONSIVE_TIME = 3 * 1000; // 3s private readonly _onDidChangeResponsiveState: Emitter = this._register(new Emitter()); @@ -182,14 +187,14 @@ export class RPCProtocol extends Disposable implements IRPCProtocol { } public getProxy(identifier: ProxyIdentifier): T { - const rpcId = identifier.nid; + const { nid: rpcId, sid } = identifier; if (!this._proxies[rpcId]) { - this._proxies[rpcId] = this._createProxy(rpcId); + this._proxies[rpcId] = this._createProxy(rpcId, sid); } return this._proxies[rpcId]; } - private _createProxy(rpcId: number): T { + private _createProxy(rpcId: number, debugName: string): T { let handler = { get: (target: any, name: PropertyKey) => { if (typeof name === 'string' && !target[name] && name.charCodeAt(0) === CharCode.DollarSign) { @@ -197,6 +202,9 @@ export class RPCProtocol extends Disposable implements IRPCProtocol { return this._remoteCall(rpcId, name, myArgs); }; } + if (name === _RPCProxySymbol) { + return debugName; + } return target[name]; } }; From 4cfd5f850806fd293d5ca6e988c612a5514f93b3 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 29 Jan 2021 12:30:02 +0100 Subject: [PATCH 02/11] clipboard changes --- .../src/singlefolder-tests/rpc.test.ts | 4 ++-- .../workbench/api/common/extHost.api.impl.ts | 4 +--- .../workbench/api/common/extHostClipboard.ts | 24 +++++++++---------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index f75da5a79a93a..7e0d622120feb 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -45,9 +45,9 @@ suite('vscode', function () { } catch (err) { assert.fail(err); } - assert.strictEqual(rpcPaths.length, 0); + assert.strictEqual(rpcPaths.length, 0, rpcPaths.join('\n')); - // assert.strictEqual(proxyPaths.length, 0); // proxies are accessible... + // assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); // proxies are accessible... }); }); diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 2da02381a46d2..a575b13d7c7e7 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -288,9 +288,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I get appName() { return initData.environment.appName; }, get appRoot() { return initData.environment.appRoot?.fsPath ?? ''; }, get uriScheme() { return initData.environment.appUriScheme; }, - get clipboard(): vscode.Clipboard { - return extHostClipboard; - }, + get clipboard(): vscode.Clipboard { return extHostClipboard.value; }, get shell() { return extHostTerminalService.getDefaultShell(false, configProvider); }, diff --git a/src/vs/workbench/api/common/extHostClipboard.ts b/src/vs/workbench/api/common/extHostClipboard.ts index 2983f22bd5b9c..f8665dacbedce 100644 --- a/src/vs/workbench/api/common/extHostClipboard.ts +++ b/src/vs/workbench/api/common/extHostClipboard.ts @@ -3,22 +3,22 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IMainContext, MainContext, MainThreadClipboardShape } from 'vs/workbench/api/common/extHost.protocol'; +import { IMainContext, MainContext } from 'vs/workbench/api/common/extHost.protocol'; import type * as vscode from 'vscode'; -export class ExtHostClipboard implements vscode.Clipboard { +export class ExtHostClipboard { - private readonly _proxy: MainThreadClipboardShape; + readonly value: vscode.Clipboard; constructor(mainContext: IMainContext) { - this._proxy = mainContext.getProxy(MainContext.MainThreadClipboard); - } - - readText(): Promise { - return this._proxy.$readText(); - } - - writeText(value: string): Promise { - return this._proxy.$writeText(value); + const proxy = mainContext.getProxy(MainContext.MainThreadClipboard); + this.value = Object.freeze({ + readText() { + return proxy.$readText(); + }, + writeText(value: string) { + return proxy.$writeText(value); + } + }); } } From 3114b1c4c52c9efb624c2ba2c9cb1a0367650860 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 29 Jan 2021 13:03:42 +0100 Subject: [PATCH 03/11] hide rpc proxies --- .../src/singlefolder-tests/rpc.test.ts | 3 +- .../workbench/api/common/extHost.api.impl.ts | 20 ++--- .../api/common/extHostDebugService.ts | 22 +++--- .../api/common/extHostFileSystemConsumer.ts | 74 ++++++++++--------- .../api/common/extHostStoragePaths.ts | 6 +- 5 files changed, 63 insertions(+), 62 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index 7e0d622120feb..b7c4c7c2abe49 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -46,8 +46,7 @@ suite('vscode', function () { assert.fail(err); } assert.strictEqual(rpcPaths.length, 0, rpcPaths.join('\n')); - - // assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); // proxies are accessible... + assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); }); }); diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index a575b13d7c7e7..a32d5b80013de 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -829,7 +829,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I return extHostFileSystem.registerFileSystemProvider(extension.identifier, scheme, provider, options); }, get fs() { - return extHostConsumerFileSystem; + return extHostConsumerFileSystem.value; }, registerFileSearchProvider: (scheme: string, provider: vscode.FileSearchProvider) => { checkProposedApiEnabled(extension); @@ -1290,9 +1290,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I class Extension implements vscode.Extension { - private _extensionService: IExtHostExtensionService; - private _originExtensionId: ExtensionIdentifier; - private _identifier: ExtensionIdentifier; + #extensionService: IExtHostExtensionService; + #originExtensionId: ExtensionIdentifier; + #identifier: ExtensionIdentifier; readonly id: string; readonly extensionUri: URI; @@ -1301,9 +1301,9 @@ class Extension implements vscode.Extension { readonly extensionKind: vscode.ExtensionKind; constructor(extensionService: IExtHostExtensionService, originExtensionId: ExtensionIdentifier, description: IExtensionDescription, kind: extHostTypes.ExtensionKind) { - this._extensionService = extensionService; - this._originExtensionId = originExtensionId; - this._identifier = description.identifier; + this.#extensionService = extensionService; + this.#originExtensionId = originExtensionId; + this.#identifier = description.identifier; this.id = description.identifier.value; this.extensionUri = description.extensionLocation; this.extensionPath = path.normalize(originalFSPath(description.extensionLocation)); @@ -1312,17 +1312,17 @@ class Extension implements vscode.Extension { } get isActive(): boolean { - return this._extensionService.isActivated(this._identifier); + return this.#extensionService.isActivated(this.#identifier); } get exports(): T { if (this.packageJSON.api === 'none') { return undefined!; // Strict nulloverride - Public api } - return this._extensionService.getExtensionExports(this._identifier); + return this.#extensionService.getExtensionExports(this.#identifier); } activate(): Thenable { - return this._extensionService.activateByIdWithErrors(this._identifier, { startup: false, extensionId: this._originExtensionId, activationEvent: 'api' }).then(() => this.exports); + return this.#extensionService.activateByIdWithErrors(this.#identifier, { startup: false, extensionId: this.#originExtensionId, activationEvent: 'api' }).then(() => this.exports); } } diff --git a/src/vs/workbench/api/common/extHostDebugService.ts b/src/vs/workbench/api/common/extHostDebugService.ts index d639431a13efc..c1bbd4806c8bf 100644 --- a/src/vs/workbench/api/common/extHostDebugService.ts +++ b/src/vs/workbench/api/common/extHostDebugService.ts @@ -89,7 +89,7 @@ export abstract class ExtHostDebugServiceBase implements IExtHostDebugService, E get onDidReceiveDebugSessionCustomEvent(): Event { return this._onDidReceiveDebugSessionCustomEvent.event; } private _activeDebugConsole: ExtHostDebugConsole; - get activeDebugConsole(): ExtHostDebugConsole { return this._activeDebugConsole; } + get activeDebugConsole(): vscode.DebugConsole { return this._activeDebugConsole.value; } private _breakpoints: Map; private _breakpointEventsActive: boolean; @@ -911,20 +911,20 @@ export class ExtHostDebugSession implements vscode.DebugSession { } } -export class ExtHostDebugConsole implements vscode.DebugConsole { +export class ExtHostDebugConsole { - private _debugServiceProxy: MainThreadDebugServiceShape; + readonly value: vscode.DebugConsole; constructor(proxy: MainThreadDebugServiceShape) { - this._debugServiceProxy = proxy; - } - - append(value: string): void { - this._debugServiceProxy.$appendDebugConsole(value); - } - appendLine(value: string): void { - this.append(value + '\n'); + this.value = Object.freeze({ + append(value: string): void { + proxy.$appendDebugConsole(value); + }, + appendLine(value: string): void { + this.append(value + '\n'); + } + }); } } diff --git a/src/vs/workbench/api/common/extHostFileSystemConsumer.ts b/src/vs/workbench/api/common/extHostFileSystemConsumer.ts index 5bbd185c77588..46e331f4c8313 100644 --- a/src/vs/workbench/api/common/extHostFileSystemConsumer.ts +++ b/src/vs/workbench/api/common/extHostFileSystemConsumer.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { MainThreadFileSystemShape, MainContext } from './extHost.protocol'; +import { MainContext } from './extHost.protocol'; import * as vscode from 'vscode'; import * as files from 'vs/platform/files/common/files'; import { FileSystemError } from 'vs/workbench/api/common/extHostTypes'; @@ -12,49 +12,51 @@ import { createDecorator } from 'vs/platform/instantiation/common/instantiation' import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService'; import { IExtHostFileSystemInfo } from 'vs/workbench/api/common/extHostFileSystemInfo'; -export class ExtHostConsumerFileSystem implements vscode.FileSystem { +export class ExtHostConsumerFileSystem { readonly _serviceBrand: undefined; - private readonly _proxy: MainThreadFileSystemShape; + readonly value: vscode.FileSystem; constructor( @IExtHostRpcService extHostRpc: IExtHostRpcService, - @IExtHostFileSystemInfo private readonly _fileSystemInfo: IExtHostFileSystemInfo, + @IExtHostFileSystemInfo fileSystemInfo: IExtHostFileSystemInfo, ) { - this._proxy = extHostRpc.getProxy(MainContext.MainThreadFileSystem); - } + const proxy = extHostRpc.getProxy(MainContext.MainThreadFileSystem); - stat(uri: vscode.Uri): Promise { - return this._proxy.$stat(uri).catch(ExtHostConsumerFileSystem._handleError); - } - readDirectory(uri: vscode.Uri): Promise<[string, vscode.FileType][]> { - return this._proxy.$readdir(uri).catch(ExtHostConsumerFileSystem._handleError); - } - createDirectory(uri: vscode.Uri): Promise { - return this._proxy.$mkdir(uri).catch(ExtHostConsumerFileSystem._handleError); - } - async readFile(uri: vscode.Uri): Promise { - return this._proxy.$readFile(uri).then(buff => buff.buffer).catch(ExtHostConsumerFileSystem._handleError); - } - writeFile(uri: vscode.Uri, content: Uint8Array): Promise { - return this._proxy.$writeFile(uri, VSBuffer.wrap(content)).catch(ExtHostConsumerFileSystem._handleError); - } - delete(uri: vscode.Uri, options?: { recursive?: boolean; useTrash?: boolean; }): Promise { - return this._proxy.$delete(uri, { ...{ recursive: false, useTrash: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); - } - rename(oldUri: vscode.Uri, newUri: vscode.Uri, options?: { overwrite?: boolean; }): Promise { - return this._proxy.$rename(oldUri, newUri, { ...{ overwrite: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); - } - copy(source: vscode.Uri, destination: vscode.Uri, options?: { overwrite?: boolean; }): Promise { - return this._proxy.$copy(source, destination, { ...{ overwrite: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); - } - isWritableFileSystem(scheme: string): boolean | undefined { - const capabilities = this._fileSystemInfo.getCapabilities(scheme); - if (typeof capabilities === 'number') { - return !(capabilities & files.FileSystemProviderCapabilities.Readonly); - } - return undefined; + this.value = Object.freeze({ + stat(uri: vscode.Uri): Promise { + return proxy.$stat(uri).catch(ExtHostConsumerFileSystem._handleError); + }, + readDirectory(uri: vscode.Uri): Promise<[string, vscode.FileType][]> { + return proxy.$readdir(uri).catch(ExtHostConsumerFileSystem._handleError); + }, + createDirectory(uri: vscode.Uri): Promise { + return proxy.$mkdir(uri).catch(ExtHostConsumerFileSystem._handleError); + }, + async readFile(uri: vscode.Uri): Promise { + return proxy.$readFile(uri).then(buff => buff.buffer).catch(ExtHostConsumerFileSystem._handleError); + }, + writeFile(uri: vscode.Uri, content: Uint8Array): Promise { + return proxy.$writeFile(uri, VSBuffer.wrap(content)).catch(ExtHostConsumerFileSystem._handleError); + }, + delete(uri: vscode.Uri, options?: { recursive?: boolean; useTrash?: boolean; }): Promise { + return proxy.$delete(uri, { ...{ recursive: false, useTrash: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); + }, + rename(oldUri: vscode.Uri, newUri: vscode.Uri, options?: { overwrite?: boolean; }): Promise { + return proxy.$rename(oldUri, newUri, { ...{ overwrite: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); + }, + copy(source: vscode.Uri, destination: vscode.Uri, options?: { overwrite?: boolean; }): Promise { + return proxy.$copy(source, destination, { ...{ overwrite: false }, ...options }).catch(ExtHostConsumerFileSystem._handleError); + }, + isWritableFileSystem(scheme: string): boolean | undefined { + const capabilities = fileSystemInfo.getCapabilities(scheme); + if (typeof capabilities === 'number') { + return !(capabilities & files.FileSystemProviderCapabilities.Readonly); + } + return undefined; + } + }); } private static _handleError(err: any): never { diff --git a/src/vs/workbench/api/common/extHostStoragePaths.ts b/src/vs/workbench/api/common/extHostStoragePaths.ts index 34dc47ea63564..45dc6abf5edaf 100644 --- a/src/vs/workbench/api/common/extHostStoragePaths.ts +++ b/src/vs/workbench/api/common/extHostStoragePaths.ts @@ -48,7 +48,7 @@ export class ExtensionStoragePaths implements IExtensionStoragePaths { const storageUri = URI.joinPath(this._environment.workspaceStorageHome, storageName); try { - await this._extHostFileSystem.stat(storageUri); + await this._extHostFileSystem.value.stat(storageUri); this._logService.trace('[ExtHostStorage] storage dir already exists', storageUri); return storageUri; } catch { @@ -57,8 +57,8 @@ export class ExtensionStoragePaths implements IExtensionStoragePaths { try { this._logService.trace('[ExtHostStorage] creating dir and metadata-file', storageUri); - await this._extHostFileSystem.createDirectory(storageUri); - await this._extHostFileSystem.writeFile( + await this._extHostFileSystem.value.createDirectory(storageUri); + await this._extHostFileSystem.value.writeFile( URI.joinPath(storageUri, 'meta.json'), new TextEncoder().encode(JSON.stringify({ id: this._workspace.id, From 930b5686ffa042fdbc37fb9a7c3e800d786bca99 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 29 Jan 2021 13:08:41 +0100 Subject: [PATCH 04/11] add todo --- .../vscode-api-tests/src/singlefolder-tests/rpc.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index b7c4c7c2abe49..9a688165d86f4 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -47,6 +47,10 @@ suite('vscode', function () { } assert.strictEqual(rpcPaths.length, 0, rpcPaths.join('\n')); assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); + + // todo@jrieken + // this should be run after/before each test because some objects are short lived, + // like notebook-editor, documents etc. }); }); From 46cf57b2646c33fee17806fb92a8438980c07b4c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 29 Jan 2021 16:33:25 +0100 Subject: [PATCH 05/11] a bunch of failing tests --- .../src/singlefolder-tests/commands.test.ts | 3 ++ .../singlefolder-tests/configuration.test.ts | 3 ++ .../src/singlefolder-tests/debug.test.ts | 4 +- .../src/singlefolder-tests/editor.test.ts | 7 ++- .../src/singlefolder-tests/env.test.ts | 3 ++ .../src/singlefolder-tests/languages.test.ts | 4 +- .../src/singlefolder-tests/quickInput.test.ts | 7 ++- .../src/singlefolder-tests/rpc.test.ts | 49 ++----------------- .../src/singlefolder-tests/terminal.test.ts | 2 + .../src/singlefolder-tests/types.test.ts | 3 ++ .../src/singlefolder-tests/webview.test.ts | 4 +- .../src/singlefolder-tests/window.test.ts | 31 ++++++------ .../workspace.event.test.ts | 7 ++- .../singlefolder-tests/workspace.fs.test.ts | 3 ++ .../workspace.tasks.test.ts | 2 + .../src/singlefolder-tests/workspace.test.ts | 7 ++- extensions/vscode-api-tests/src/utils.ts | 41 ++++++++++++++++ 17 files changed, 106 insertions(+), 74 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/commands.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/commands.test.ts index 1d62a12c170f1..f9f971bdbdab3 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/commands.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/commands.test.ts @@ -7,9 +7,12 @@ import 'mocha'; import * as assert from 'assert'; import { join } from 'path'; import { commands, workspace, window, Uri, Range, Position, ViewColumn } from 'vscode'; +import { assertNoRpc } from '../utils'; suite('vscode API - commands', () => { + teardown(assertNoRpc); + test('getCommands', function (done) { let p1 = commands.getCommands().then(commands => { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/configuration.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/configuration.test.ts index 0b6f13fb01fcb..0a9ddf7aa2d61 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/configuration.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/configuration.test.ts @@ -6,9 +6,12 @@ import 'mocha'; import * as assert from 'assert'; import * as vscode from 'vscode'; +import { assertNoRpc } from '../utils'; suite('vscode API - configuration', () => { + teardown(assertNoRpc); + test('configurations, language defaults', function () { const defaultLanguageSettings = vscode.workspace.getConfiguration().get('[abcLang]'); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts index ec976ff53341c..145c2ee3bce88 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts @@ -5,11 +5,13 @@ import * as assert from 'assert'; import { debug, workspace, Disposable, commands, window } from 'vscode'; -import { disposeAll } from '../utils'; +import { assertNoRpc, disposeAll } from '../utils'; import { basename } from 'path'; suite('vscode API - debug', function () { + teardown(assertNoRpc); + test('breakpoints', async function () { assert.equal(debug.breakpoints.length, 0); let onDidChangeBreakpointsCounter = 0; diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts index b5a0a79166bb5..ef16bbe1c61a2 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts @@ -5,11 +5,14 @@ import * as assert from 'assert'; import { workspace, window, Position, Range, commands, TextEditor, TextDocument, TextEditorCursorStyle, TextEditorLineNumbersStyle, SnippetString, Selection, Uri, env } from 'vscode'; -import { createRandomFile, deleteFile, closeAllEditors } from '../utils'; +import { createRandomFile, deleteFile, closeAllEditors, assertNoRpc } from '../utils'; suite('vscode API - editors', () => { - teardown(closeAllEditors); + teardown(async function () { + assertNoRpc(); + await closeAllEditors(); + }); function withRandomFileEditor(initialContents: string, run: (editor: TextEditor, doc: TextDocument) => Thenable): Thenable { return createRandomFile(initialContents).then(file => { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/env.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/env.test.ts index 7bc2f75973fea..e3f0106fe1084 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/env.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/env.test.ts @@ -5,9 +5,12 @@ import * as assert from 'assert'; import { env, extensions, ExtensionKind, UIKind, Uri } from 'vscode'; +import { assertNoRpc } from '../utils'; suite('vscode API - env', () => { + teardown(assertNoRpc); + test('env is set', function () { assert.equal(typeof env.language, 'string'); assert.equal(typeof env.appRoot, 'string'); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts index a59841f6689c8..0553a4444dbe7 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts @@ -6,10 +6,12 @@ import * as assert from 'assert'; import { join } from 'path'; import * as vscode from 'vscode'; -import { createRandomFile, testFs } from '../utils'; +import { assertNoRpc, createRandomFile, testFs } from '../utils'; suite('vscode API - languages', () => { + teardown(assertNoRpc); + const isWindows = process.platform === 'win32'; function positionToString(p: vscode.Position) { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts index 55fbe0655a7da..701c947426bc5 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/quickInput.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { window, commands } from 'vscode'; -import { closeAllEditors } from '../utils'; +import { assertNoRpc, closeAllEditors } from '../utils'; interface QuickPickExpected { events: string[]; @@ -20,7 +20,10 @@ interface QuickPickExpected { suite('vscode API - quick input', function () { - teardown(closeAllEditors); + teardown(async function () { + assertNoRpc(); + await closeAllEditors(); + }); test('createQuickPick, select second', function (_done) { let done = (err?: any) => { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index 9a688165d86f4..596a7f9746f3a 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -3,54 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as assert from 'assert'; -import * as vscode from 'vscode'; +import { assertNoRpc } from '../utils'; suite('vscode', function () { - test('rpc protocol, proxies not reachable', function () { - - const symProxy = Symbol.for('rpcProxy'); - const symProtocol = Symbol.for('rpcProtocol'); - - const proxyPaths: string[] = []; - const rpcPaths: string[] = []; - - function walk(obj: any, path: string, seen: Set) { - if (!obj) { - return; - } - if (typeof obj !== 'object' && typeof obj !== 'function') { - return; - } - if (seen.has(obj)) { - return; - } - seen.add(obj); - - if (obj[symProtocol]) { - rpcPaths.push(`PROTOCOL via ${path}`); - } - if (obj[symProxy]) { - proxyPaths.push(`PROXY '${obj[symProxy]}' via ${path}`); - } - - for (const key in obj) { - walk(obj[key], `${path}.${String(key)}`, seen); - } - } - - try { - walk(vscode, 'vscode', new Set()); - } catch (err) { - assert.fail(err); - } - assert.strictEqual(rpcPaths.length, 0, rpcPaths.join('\n')); - assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); - - // todo@jrieken - // this should be run after/before each test because some objects are short lived, - // like notebook-editor, documents etc. + test('no rpc', function () { + assertNoRpc(); }); - }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index 01232a99c8c97..9cede7605b47d 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -5,6 +5,7 @@ import { window, Pseudoterminal, EventEmitter, TerminalDimensions, workspace, ConfigurationTarget, Disposable, UIKind, env, EnvironmentVariableMutatorType, EnvironmentVariableMutator, extensions, ExtensionContext, TerminalOptions, ExtensionTerminalOptions } from 'vscode'; import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert'; +import { assertNoRpc } from '../utils'; // Disable terminal tests: // - Web https://github.com/microsoft/vscode/issues/92826 @@ -30,6 +31,7 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert'; let disposables: Disposable[] = []; teardown(() => { + assertNoRpc(); disposables.forEach(d => d.dispose()); disposables.length = 0; }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/types.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/types.test.ts index 53265b35e99b9..7143b40fa0c2a 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/types.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/types.test.ts @@ -6,9 +6,12 @@ import 'mocha'; import * as assert from 'assert'; import * as vscode from 'vscode'; +import { assertNoRpc } from '../utils'; suite('vscode API - types', () => { + teardown(assertNoRpc); + test('static properties, es5 compat class', function () { assert.ok(vscode.ThemeIcon.File instanceof vscode.ThemeIcon); assert.ok(vscode.ThemeIcon.Folder instanceof vscode.ThemeIcon); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/webview.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/webview.test.ts index 83357278c5d1c..14947da78e2ca 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/webview.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/webview.test.ts @@ -7,7 +7,7 @@ import * as assert from 'assert'; import 'mocha'; import * as os from 'os'; import * as vscode from 'vscode'; -import { closeAllEditors, delay, disposeAll } from '../utils'; +import { assertNoRpc, closeAllEditors, delay, disposeAll } from '../utils'; const webviewId = 'myWebview'; @@ -26,8 +26,8 @@ suite.skip('vscode API - webview', () => { } teardown(async () => { + assertNoRpc(); await closeAllEditors(); - disposeAll(disposables); }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/window.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/window.test.ts index 89f3dbb725951..65fd89d3b96a8 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/window.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/window.test.ts @@ -6,12 +6,15 @@ import * as assert from 'assert'; import { workspace, window, commands, ViewColumn, TextEditorViewColumnChangeEvent, Uri, Selection, Position, CancellationTokenSource, TextEditorSelectionChangeKind, QuickPickItem, TextEditor } from 'vscode'; import { join } from 'path'; -import { closeAllEditors, pathEquals, createRandomFile } from '../utils'; +import { closeAllEditors, pathEquals, createRandomFile, assertNoRpc } from '../utils'; suite('vscode API - window', () => { - teardown(closeAllEditors); + teardown(async function () { + assertNoRpc(); + await closeAllEditors(); + }); test('editor, active text editor', async () => { const doc = await workspace.openTextDocument(join(workspace.rootPath || '', './far.js')); @@ -429,8 +432,8 @@ suite('vscode API - window', () => { }); test('showQuickPick, select first two', async function () { - const label = 'showQuickPick, select first two'; - let i = 0; + // const label = 'showQuickPick, select first two'; + // let i = 0; const resolves: ((value: string) => void)[] = []; let done: () => void; const unexpected = new Promise((resolve, reject) => { @@ -442,26 +445,26 @@ suite('vscode API - window', () => { canPickMany: true }); const first = new Promise(resolve => resolves.push(resolve)); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); await new Promise(resolve => setTimeout(resolve, 100)); // Allow UI to update. - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); await commands.executeCommand('workbench.action.quickOpenSelectNext'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); assert.equal(await first, 'eins'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); await commands.executeCommand('workbench.action.quickPickManyToggle'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); const second = new Promise(resolve => resolves.push(resolve)); await commands.executeCommand('workbench.action.quickOpenSelectNext'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); assert.equal(await second, 'zwei'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); await commands.executeCommand('workbench.action.quickPickManyToggle'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); await commands.executeCommand('workbench.action.acceptSelectedQuickOpenItem'); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); assert.deepStrictEqual(await picks, ['eins', 'zwei']); - console.log(`${label}: ${++i}`); + // console.log(`${label}: ${++i}`); done!(); return unexpected; }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.event.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.event.test.ts index e192c63c0abef..e3f0c13dd2de3 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.event.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.event.test.ts @@ -5,16 +5,15 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; -import { createRandomFile, withLogDisabled } from '../utils'; +import { assertNoRpc, createRandomFile, disposeAll, withLogDisabled } from '../utils'; suite('vscode API - workspace events', () => { const disposables: vscode.Disposable[] = []; teardown(() => { - for (const dispo of disposables) { - dispo.dispose(); - } + assertNoRpc(); + disposeAll(disposables); disposables.length = 0; }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.fs.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.fs.test.ts index 2eb21a4c1f9f5..193ab54be61da 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.fs.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.fs.test.ts @@ -6,6 +6,7 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; import { posix } from 'path'; +import { assertNoRpc } from '../utils'; suite('vscode API - workspace-fs', () => { @@ -15,6 +16,8 @@ suite('vscode API - workspace-fs', () => { root = vscode.workspace.workspaceFolders![0]!.uri; }); + teardown(assertNoRpc); + test('fs.stat', async function () { const stat = await vscode.workspace.fs.stat(root); assert.equal(stat.type, vscode.FileType.Directory); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts index c831e6d4306e4..d6761c7344443 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts @@ -5,6 +5,7 @@ import * as assert from 'assert'; import { window, tasks, Disposable, TaskDefinition, Task, EventEmitter, CustomExecution, Pseudoterminal, TaskScope, commands, env, UIKind, ShellExecution, TaskExecution, Terminal, Event } from 'vscode'; +import { assertNoRpc } from '../utils'; // Disable tasks tests: // - Web https://github.com/microsoft/vscode/issues/90528 @@ -14,6 +15,7 @@ import { window, tasks, Disposable, TaskDefinition, Task, EventEmitter, CustomEx let disposables: Disposable[] = []; teardown(() => { + assertNoRpc(); disposables.forEach(d => d.dispose()); disposables.length = 0; }); diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts index b3241e6d384b1..ace2479b34313 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts @@ -5,14 +5,17 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; -import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs, delay, withLogDisabled, revertAllDirty } from '../utils'; +import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs, delay, withLogDisabled, revertAllDirty, assertNoRpc } from '../utils'; import { join, posix, basename } from 'path'; import * as fs from 'fs'; import { TestFS } from '../memfs'; suite('vscode API - workspace', () => { - teardown(closeAllEditors); + teardown(async function () { + assertNoRpc(); + await closeAllEditors(); + }); test('MarkdownString', function () { let md = new vscode.MarkdownString(); diff --git a/extensions/vscode-api-tests/src/utils.ts b/extensions/vscode-api-tests/src/utils.ts index 392d0be84612b..f35acafde7d9a 100644 --- a/extensions/vscode-api-tests/src/utils.ts +++ b/extensions/vscode-api-tests/src/utils.ts @@ -72,3 +72,44 @@ export function withLogDisabled(runnable: () => Promise): () => Promise) { + if (!obj) { + return; + } + if (typeof obj !== 'object' && typeof obj !== 'function') { + return; + } + if (seen.has(obj)) { + return; + } + seen.add(obj); + + if (obj[symProtocol]) { + rpcPaths.push(`PROTOCOL via ${path}`); + } + if (obj[symProxy]) { + proxyPaths.push(`PROXY '${obj[symProxy]}' via ${path}`); + } + + for (const key in obj) { + walk(obj[key], `${path}.${String(key)}`, seen); + } + } + + try { + walk(vscode, 'vscode', new Set()); + } catch (err) { + assert.fail(err); + } + assert.strictEqual(rpcPaths.length, 0, rpcPaths.join('\n')); + assert.strictEqual(proxyPaths.length, 0, proxyPaths.join('\n')); // happens... +} From 82c629eb3a4360f4ba5a47668e7e973839b5f6e8 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 1 Feb 2021 11:23:37 +0100 Subject: [PATCH 06/11] hide tasks in TaskExecution --- src/vs/workbench/api/common/extHostTask.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTask.ts b/src/vs/workbench/api/common/extHostTask.ts index 9b0753a57090a..35e824bad1dd3 100644 --- a/src/vs/workbench/api/common/extHostTask.ts +++ b/src/vs/workbench/api/common/extHostTask.ts @@ -341,7 +341,10 @@ export namespace TaskFilterDTO { class TaskExecutionImpl implements vscode.TaskExecution { - constructor(private readonly _tasks: ExtHostTaskBase, readonly _id: string, private readonly _task: vscode.Task) { + readonly #tasks: ExtHostTaskBase; + + constructor(tasks: ExtHostTaskBase, readonly _id: string, private readonly _task: vscode.Task) { + this.#tasks = tasks; } public get task(): vscode.Task { @@ -349,7 +352,7 @@ class TaskExecutionImpl implements vscode.TaskExecution { } public terminate(): void { - this._tasks.terminateTask(this); + this.#tasks.terminateTask(this); } public fireDidStartProcess(value: tasks.TaskProcessStartedDTO): void { From a60beb9d7a8ade758f62dd12fa3e4461955554eb Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 1 Feb 2021 11:24:30 +0100 Subject: [PATCH 07/11] don't leak proxies in editor land, also remove indentSize property which isn't API --- .../workbench/api/browser/mainThreadEditor.ts | 9 - .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 - .../workbench/api/common/extHostCodeInsets.ts | 6 +- .../api/common/extHostDocumentsAndEditors.ts | 23 +- .../workbench/api/common/extHostTextEditor.ts | 430 ++++++++---------- .../api/common/extHostTextEditors.ts | 23 +- .../browser/api/extHostTextEditor.test.ts | 207 ++------- 8 files changed, 257 insertions(+), 445 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadEditor.ts b/src/vs/workbench/api/browser/mainThreadEditor.ts index f805b7f9cbe21..37d4d9326d40c 100644 --- a/src/vs/workbench/api/browser/mainThreadEditor.ts +++ b/src/vs/workbench/api/browser/mainThreadEditor.ts @@ -79,7 +79,6 @@ export class MainThreadTextEditorProperties { return { insertSpaces: modelOptions.insertSpaces, tabSize: modelOptions.tabSize, - indentSize: modelOptions.indentSize, cursorStyle: cursorStyle, lineNumbers: lineNumbers }; @@ -146,7 +145,6 @@ export class MainThreadTextEditorProperties { } return ( a.tabSize === b.tabSize - && a.indentSize === b.indentSize && a.insertSpaces === b.insertSpaces && a.cursorStyle === b.cursorStyle && a.lineNumbers === b.lineNumbers @@ -377,13 +375,6 @@ export class MainThreadTextEditor { if (typeof newConfiguration.tabSize !== 'undefined') { newOpts.tabSize = newConfiguration.tabSize; } - if (typeof newConfiguration.indentSize !== 'undefined') { - if (newConfiguration.indentSize === 'tabSize') { - newOpts.indentSize = newOpts.tabSize || creationOpts.tabSize; - } else { - newOpts.indentSize = newConfiguration.indentSize; - } - } this._model.updateOptions(newOpts); } diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index a32d5b80013de..a21a1a708b05c 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -262,7 +262,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I registerDiffInformationCommand: (id: string, callback: (diff: vscode.LineChange[], ...args: any[]) => any, thisArg?: any): vscode.Disposable => { checkProposedApiEnabled(extension); return extHostCommands.registerCommand(true, id, async (...args: any[]): Promise => { - const activeTextEditor = extHostEditors.getActiveTextEditor(); + const activeTextEditor = extHostDocumentsAndEditors.activeEditor(true); if (!activeTextEditor) { extHostLogService.warn('Cannot execute ' + id + ' because there is no active text editor.'); return undefined; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index be7a14f885943..1be08c2c007bd 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -237,7 +237,6 @@ export interface MainThreadDocumentsShape extends IDisposable { export interface ITextEditorConfigurationUpdate { tabSize?: number | 'auto'; - indentSize?: number | 'tabSize'; insertSpaces?: boolean | 'auto'; cursorStyle?: TextEditorCursorStyle; lineNumbers?: RenderLineNumbersType; @@ -245,7 +244,6 @@ export interface ITextEditorConfigurationUpdate { export interface IResolvedTextEditorConfiguration { tabSize: number; - indentSize: number; insertSpaces: boolean; cursorStyle: TextEditorCursorStyle; lineNumbers: RenderLineNumbersType; diff --git a/src/vs/workbench/api/common/extHostCodeInsets.ts b/src/vs/workbench/api/common/extHostCodeInsets.ts index 138e4950ae2ed..7db034cd81f90 100644 --- a/src/vs/workbench/api/common/extHostCodeInsets.ts +++ b/src/vs/workbench/api/common/extHostCodeInsets.ts @@ -44,8 +44,8 @@ export class ExtHostEditorInsets implements ExtHostEditorInsetsShape { createWebviewEditorInset(editor: vscode.TextEditor, line: number, height: number, options: vscode.WebviewOptions | undefined, extension: IExtensionDescription): vscode.WebviewEditorInset { let apiEditor: ExtHostTextEditor | undefined; - for (const candidate of this._editors.getVisibleTextEditors()) { - if (candidate === editor) { + for (const candidate of this._editors.getVisibleTextEditors(true)) { + if (candidate.value === editor) { apiEditor = candidate; break; } @@ -121,7 +121,7 @@ export class ExtHostEditorInsets implements ExtHostEditorInsetsShape { } }; - this._proxy.$createEditorInset(handle, apiEditor.id, apiEditor.document.uri, line + 1, height, options || {}, extension.identifier, extension.extensionLocation); + this._proxy.$createEditorInset(handle, apiEditor.id, apiEditor.value.document.uri, line + 1, height, options || {}, extension.identifier, extension.extensionLocation); this._insets.set(handle, { editor, inset, onDidReceiveMessage }); return inset; diff --git a/src/vs/workbench/api/common/extHostDocumentsAndEditors.ts b/src/vs/workbench/api/common/extHostDocumentsAndEditors.ts index e378b49a198d4..bd80952ee5321 100644 --- a/src/vs/workbench/api/common/extHostDocumentsAndEditors.ts +++ b/src/vs/workbench/api/common/extHostDocumentsAndEditors.ts @@ -18,6 +18,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import { ResourceMap } from 'vs/base/common/map'; import { Schemas } from 'vs/base/common/network'; import { Iterable } from 'vs/base/common/iterator'; +import { Lazy } from 'vs/base/common/lazy'; class Reference { private _count = 0; @@ -49,13 +50,13 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha private readonly _onDidAddDocuments = new Emitter(); private readonly _onDidRemoveDocuments = new Emitter(); - private readonly _onDidChangeVisibleTextEditors = new Emitter(); - private readonly _onDidChangeActiveTextEditor = new Emitter(); + private readonly _onDidChangeVisibleTextEditors = new Emitter(); + private readonly _onDidChangeActiveTextEditor = new Emitter(); readonly onDidAddDocuments: Event = this._onDidAddDocuments.event; readonly onDidRemoveDocuments: Event = this._onDidRemoveDocuments.event; - readonly onDidChangeVisibleTextEditors: Event = this._onDidChangeVisibleTextEditors.event; - readonly onDidChangeActiveTextEditor: Event = this._onDidChangeActiveTextEditor.event; + readonly onDidChangeVisibleTextEditors: Event = this._onDidChangeVisibleTextEditors.event; + readonly onDidChangeActiveTextEditor: Event = this._onDidChangeActiveTextEditor.event; constructor( @IExtHostRpcService private readonly _extHostRpc: IExtHostRpcService, @@ -135,7 +136,7 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha data.id, this._extHostRpc.getProxy(MainContext.MainThreadTextEditors), this._logService, - documentData, + new Lazy(() => documentData.document), data.selections.map(typeConverters.Selection.to), data.options, data.visibleRanges.map(range => typeConverters.Range.to(range)), @@ -162,7 +163,7 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha } if (delta.removedEditors || delta.addedEditors) { - this._onDidChangeVisibleTextEditors.fire(this.allEditors()); + this._onDidChangeVisibleTextEditors.fire(this.allEditors().map(editor => editor.value)); } if (delta.newActiveEditor !== undefined) { this._onDidChangeActiveTextEditor.fire(this.activeEditor()); @@ -181,11 +182,17 @@ export class ExtHostDocumentsAndEditors implements ExtHostDocumentsAndEditorsSha return this._editors.get(id); } - activeEditor(): ExtHostTextEditor | undefined { + activeEditor(): vscode.TextEditor | undefined; + activeEditor(internal: true): ExtHostTextEditor | undefined; + activeEditor(internal?: true): vscode.TextEditor | ExtHostTextEditor | undefined { if (!this._activeEditorId) { return undefined; + } + const editor = this._editors.get(this._activeEditorId); + if (internal) { + return editor; } else { - return this._editors.get(this._activeEditorId); + return editor?.value; } } diff --git a/src/vs/workbench/api/common/extHostTextEditor.ts b/src/vs/workbench/api/common/extHostTextEditor.ts index 635a5c6c253b0..6e2a3e4e3249e 100644 --- a/src/vs/workbench/api/common/extHostTextEditor.ts +++ b/src/vs/workbench/api/common/extHostTextEditor.ts @@ -10,11 +10,11 @@ import { TextEditorCursorStyle } from 'vs/editor/common/config/editorOptions'; import { IRange } from 'vs/editor/common/core/range'; import { ISingleEditOperation } from 'vs/editor/common/model'; import { IResolvedTextEditorConfiguration, ITextEditorConfigurationUpdate, MainThreadTextEditorsShape } from 'vs/workbench/api/common/extHost.protocol'; -import { ExtHostDocumentData } from 'vs/workbench/api/common/extHostDocumentData'; import * as TypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; import { EndOfLine, Position, Range, Selection, SnippetString, TextEditorLineNumbersStyle, TextEditorRevealType } from 'vs/workbench/api/common/extHostTypes'; import type * as vscode from 'vscode'; import { ILogService } from 'vs/platform/log/common/log'; +import { Lazy } from 'vs/base/common/lazy'; export class TextEditorDecorationType implements vscode.TextEditorDecorationType { @@ -134,36 +134,63 @@ export class TextEditorEdit { } } -export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { +export class ExtHostTextEditorOptions { private _proxy: MainThreadTextEditorsShape; private _id: string; private _logService: ILogService; private _tabSize!: number; - private _indentSize!: number; private _insertSpaces!: boolean; private _cursorStyle!: TextEditorCursorStyle; private _lineNumbers!: TextEditorLineNumbersStyle; + readonly value: vscode.TextEditorOptions; + constructor(proxy: MainThreadTextEditorsShape, id: string, source: IResolvedTextEditorConfiguration, logService: ILogService) { this._proxy = proxy; this._id = id; this._accept(source); this._logService = logService; + + const that = this; + + this.value = { + get tabSize(): number | string { + return that._tabSize; + }, + set tabSize(value: number | string) { + that._setTabSize(value); + }, + get insertSpaces(): boolean | string { + return that._insertSpaces; + }, + set insertSpaces(value: boolean | string) { + that._setInsertSpaces(value); + }, + get cursorStyle(): TextEditorCursorStyle { + return that._cursorStyle; + }, + set cursorStyle(value: TextEditorCursorStyle) { + that._setCursorStyle(value); + }, + get lineNumbers(): TextEditorLineNumbersStyle { + return that._lineNumbers; + }, + set lineNumbers(value: TextEditorLineNumbersStyle) { + that._setLineNumbers(value); + } + }; } public _accept(source: IResolvedTextEditorConfiguration): void { this._tabSize = source.tabSize; - this._indentSize = source.indentSize; this._insertSpaces = source.insertSpaces; this._cursorStyle = source.cursorStyle; this._lineNumbers = TypeConverters.TextEditorLineNumbersStyle.to(source.lineNumbers); } - public get tabSize(): number | string { - return this._tabSize; - } + // --- internal: tabSize private _validateTabSize(value: number | string): number | 'auto' | null { if (value === 'auto') { @@ -183,7 +210,7 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { return null; } - public set tabSize(value: number | string) { + private _setTabSize(value: number | string) { const tabSize = this._validateTabSize(value); if (tabSize === null) { // ignore invalid call @@ -202,50 +229,7 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { })); } - public get indentSize(): number | string { - return this._indentSize; - } - - private _validateIndentSize(value: number | string): number | 'tabSize' | null { - if (value === 'tabSize') { - return 'tabSize'; - } - if (typeof value === 'number') { - const r = Math.floor(value); - return (r > 0 ? r : null); - } - if (typeof value === 'string') { - const r = parseInt(value, 10); - if (isNaN(r)) { - return null; - } - return (r > 0 ? r : null); - } - return null; - } - - public set indentSize(value: number | string) { - const indentSize = this._validateIndentSize(value); - if (indentSize === null) { - // ignore invalid call - return; - } - if (typeof indentSize === 'number') { - if (this._indentSize === indentSize) { - // nothing to do - return; - } - // reflect the new indentSize value immediately - this._indentSize = indentSize; - } - this._warnOnError(this._proxy.$trySetOptions(this._id, { - indentSize: indentSize - })); - } - - public get insertSpaces(): boolean | string { - return this._insertSpaces; - } + // --- internal: insert spaces private _validateInsertSpaces(value: boolean | string): boolean | 'auto' { if (value === 'auto') { @@ -254,7 +238,7 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { return (value === 'false' ? false : Boolean(value)); } - public set insertSpaces(value: boolean | string) { + private _setInsertSpaces(value: boolean | string) { const insertSpaces = this._validateInsertSpaces(value); if (typeof insertSpaces === 'boolean') { if (this._insertSpaces === insertSpaces) { @@ -269,11 +253,9 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { })); } - public get cursorStyle(): TextEditorCursorStyle { - return this._cursorStyle; - } + // --- internal: cursor style - public set cursorStyle(value: TextEditorCursorStyle) { + private _setCursorStyle(value: TextEditorCursorStyle) { if (this._cursorStyle === value) { // nothing to do return; @@ -284,11 +266,9 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { })); } - public get lineNumbers(): TextEditorLineNumbersStyle { - return this._lineNumbers; - } + // --- internal: line number - public set lineNumbers(value: TextEditorLineNumbersStyle) { + private _setLineNumbers(value: TextEditorLineNumbersStyle) { if (this._lineNumbers === value) { // nothing to do return; @@ -368,196 +348,203 @@ export class ExtHostTextEditorOptions implements vscode.TextEditorOptions { } } -export class ExtHostTextEditor implements vscode.TextEditor { - - private readonly _documentData: ExtHostDocumentData; +export class ExtHostTextEditor { private _selections: Selection[]; private _options: ExtHostTextEditorOptions; private _visibleRanges: Range[]; private _viewColumn: vscode.ViewColumn | undefined; private _disposed: boolean = false; - private _hasDecorationsForKey: { [key: string]: boolean; }; + private _hasDecorationsForKey = new Set(); + + readonly value: vscode.TextEditor; constructor( readonly id: string, private readonly _proxy: MainThreadTextEditorsShape, private readonly _logService: ILogService, - document: ExtHostDocumentData, + document: Lazy, selections: Selection[], options: IResolvedTextEditorConfiguration, visibleRanges: Range[], viewColumn: vscode.ViewColumn | undefined ) { - this._documentData = document; this._selections = selections; this._options = new ExtHostTextEditorOptions(this._proxy, this.id, options, _logService); this._visibleRanges = visibleRanges; this._viewColumn = viewColumn; - this._hasDecorationsForKey = Object.create(null); - } - - dispose() { - ok(!this._disposed); - this._disposed = true; - } - show(column: vscode.ViewColumn) { - this._proxy.$tryShowEditor(this.id, TypeConverters.ViewColumn.from(column)); - } - - hide() { - this._proxy.$tryHideEditor(this.id); - } + const that = this; + + this.value = Object.freeze({ + get document(): vscode.TextDocument { + return document.getValue(); + }, + set document(_value) { + throw readonly('document'); + }, + // --- selection + get selection(): Selection { + return that._selections && that._selections[0]; + }, + set selection(value: Selection) { + if (!(value instanceof Selection)) { + throw illegalArgument('selection'); + } + that._selections = [value]; + that._trySetSelection(); + }, + get selections(): Selection[] { + return that._selections; + }, + set selections(value: Selection[]) { + if (!Array.isArray(value) || value.some(a => !(a instanceof Selection))) { + throw illegalArgument('selections'); + } + that._selections = value; + that._trySetSelection(); + }, + // --- visible ranges + get visibleRanges(): Range[] { + return that._visibleRanges; + }, + set visibleRanges(_value: Range[]) { + throw readonly('visibleRanges'); + }, + // --- options + get options(): vscode.TextEditorOptions { + return that._options.value; + }, + set options(value: vscode.TextEditorOptions) { + if (!that._disposed) { + that._options.assign(value); + } + }, + // --- view column + get viewColumn(): vscode.ViewColumn | undefined { + return that._viewColumn; + }, + set viewColumn(_value) { + throw readonly('viewColumn'); + }, + // --- edit + edit(callback: (edit: TextEditorEdit) => void, options: { undoStopBefore: boolean; undoStopAfter: boolean; } = { undoStopBefore: true, undoStopAfter: true }): Promise { + if (that._disposed) { + return Promise.reject(new Error('TextEditor#edit not possible on closed editors')); + } + const edit = new TextEditorEdit(document.getValue(), options); + callback(edit); + return that._applyEdit(edit); + }, + // --- snippet edit + insertSnippet(snippet: SnippetString, where?: Position | readonly Position[] | Range | readonly Range[], options: { undoStopBefore: boolean; undoStopAfter: boolean; } = { undoStopBefore: true, undoStopAfter: true }): Promise { + if (that._disposed) { + return Promise.reject(new Error('TextEditor#insertSnippet not possible on closed editors')); + } + let ranges: IRange[]; - // ---- the document + if (!where || (Array.isArray(where) && where.length === 0)) { + ranges = that._selections.map(range => TypeConverters.Range.from(range)); - get document(): vscode.TextDocument { - return this._documentData.document; - } + } else if (where instanceof Position) { + const { lineNumber, column } = TypeConverters.Position.from(where); + ranges = [{ startLineNumber: lineNumber, startColumn: column, endLineNumber: lineNumber, endColumn: column }]; - set document(value) { - throw readonly('document'); + } else if (where instanceof Range) { + ranges = [TypeConverters.Range.from(where)]; + } else { + ranges = []; + for (const posOrRange of where) { + if (posOrRange instanceof Range) { + ranges.push(TypeConverters.Range.from(posOrRange)); + } else { + const { lineNumber, column } = TypeConverters.Position.from(posOrRange); + ranges.push({ startLineNumber: lineNumber, startColumn: column, endLineNumber: lineNumber, endColumn: column }); + } + } + } + return _proxy.$tryInsertSnippet(id, snippet.value, ranges, options); + }, + setDecorations(decorationType: vscode.TextEditorDecorationType, ranges: Range[] | vscode.DecorationOptions[]): void { + const willBeEmpty = (ranges.length === 0); + if (willBeEmpty && !that._hasDecorationsForKey.has(decorationType.key)) { + // avoid no-op call to the renderer + return; + } + if (willBeEmpty) { + that._hasDecorationsForKey.delete(decorationType.key); + } else { + that._hasDecorationsForKey.add(decorationType.key); + } + that._runOnProxy(() => { + if (TypeConverters.isDecorationOptionsArr(ranges)) { + return _proxy.$trySetDecorations( + id, + decorationType.key, + TypeConverters.fromRangeOrRangeWithMessage(ranges) + ); + } else { + const _ranges: number[] = new Array(4 * ranges.length); + for (let i = 0, len = ranges.length; i < len; i++) { + const range = ranges[i]; + _ranges[4 * i] = range.start.line + 1; + _ranges[4 * i + 1] = range.start.character + 1; + _ranges[4 * i + 2] = range.end.line + 1; + _ranges[4 * i + 3] = range.end.character + 1; + } + return _proxy.$trySetDecorationsFast( + id, + decorationType.key, + _ranges + ); + } + }); + }, + revealRange(range: Range, revealType: vscode.TextEditorRevealType): void { + that._runOnProxy(() => _proxy.$tryRevealRange( + id, + TypeConverters.Range.from(range), + (revealType || TextEditorRevealType.Default) + )); + }, + show(column: vscode.ViewColumn) { + _proxy.$tryShowEditor(id, TypeConverters.ViewColumn.from(column)); + }, + hide() { + _proxy.$tryHideEditor(id); + } + }); } - // ---- options - - get options(): vscode.TextEditorOptions { - return this._options; + dispose() { + ok(!this._disposed); + this._disposed = true; } - set options(value: vscode.TextEditorOptions) { - if (!this._disposed) { - this._options.assign(value); - } - } + // --- incoming: extension host MUST accept what the renderer says _acceptOptions(options: IResolvedTextEditorConfiguration): void { ok(!this._disposed); this._options._accept(options); } - // ---- visible ranges - - get visibleRanges(): Range[] { - return this._visibleRanges; - } - - set visibleRanges(value: Range[]) { - throw readonly('visibleRanges'); - } - _acceptVisibleRanges(value: Range[]): void { ok(!this._disposed); this._visibleRanges = value; } - // ---- view column - - get viewColumn(): vscode.ViewColumn | undefined { - return this._viewColumn; - } - - set viewColumn(value) { - throw readonly('viewColumn'); - } - _acceptViewColumn(value: vscode.ViewColumn) { ok(!this._disposed); this._viewColumn = value; } - // ---- selections - - get selection(): Selection { - return this._selections && this._selections[0]; - } - - set selection(value: Selection) { - if (!(value instanceof Selection)) { - throw illegalArgument('selection'); - } - this._selections = [value]; - this._trySetSelection(); - } - - get selections(): Selection[] { - return this._selections; - } - - set selections(value: Selection[]) { - if (!Array.isArray(value) || value.some(a => !(a instanceof Selection))) { - throw illegalArgument('selections'); - } - this._selections = value; - this._trySetSelection(); - } - - setDecorations(decorationType: vscode.TextEditorDecorationType, ranges: Range[] | vscode.DecorationOptions[]): void { - const willBeEmpty = (ranges.length === 0); - if (willBeEmpty && !this._hasDecorationsForKey[decorationType.key]) { - // avoid no-op call to the renderer - return; - } - if (willBeEmpty) { - delete this._hasDecorationsForKey[decorationType.key]; - } else { - this._hasDecorationsForKey[decorationType.key] = true; - } - this._runOnProxy( - () => { - if (TypeConverters.isDecorationOptionsArr(ranges)) { - return this._proxy.$trySetDecorations( - this.id, - decorationType.key, - TypeConverters.fromRangeOrRangeWithMessage(ranges) - ); - } else { - const _ranges: number[] = new Array(4 * ranges.length); - for (let i = 0, len = ranges.length; i < len; i++) { - const range = ranges[i]; - _ranges[4 * i] = range.start.line + 1; - _ranges[4 * i + 1] = range.start.character + 1; - _ranges[4 * i + 2] = range.end.line + 1; - _ranges[4 * i + 3] = range.end.character + 1; - } - return this._proxy.$trySetDecorationsFast( - this.id, - decorationType.key, - _ranges - ); - } - } - ); - } - - revealRange(range: Range, revealType: vscode.TextEditorRevealType): void { - this._runOnProxy( - () => this._proxy.$tryRevealRange( - this.id, - TypeConverters.Range.from(range), - (revealType || TextEditorRevealType.Default) - ) - ); - } - - private _trySetSelection(): Promise { - const selection = this._selections.map(TypeConverters.Selection.from); - return this._runOnProxy(() => this._proxy.$trySetSelections(this.id, selection)); - } - _acceptSelections(selections: Selection[]): void { ok(!this._disposed); this._selections = selections; } - // ---- editing - - edit(callback: (edit: TextEditorEdit) => void, options: { undoStopBefore: boolean; undoStopAfter: boolean; } = { undoStopBefore: true, undoStopAfter: true }): Promise { - if (this._disposed) { - return Promise.reject(new Error('TextEditor#edit not possible on closed editors')); - } - const edit = new TextEditorEdit(this._documentData.document, options); - callback(edit); - return this._applyEdit(edit); + private async _trySetSelection(): Promise { + const selection = this._selections.map(TypeConverters.Selection.from); + await this._runOnProxy(() => this._proxy.$trySetSelections(this.id, selection)); + return this.value; } private _applyEdit(editBuilder: TextEditorEdit): Promise { @@ -613,44 +600,12 @@ export class ExtHostTextEditor implements vscode.TextEditor { undoStopAfter: editData.undoStopAfter }); } - - insertSnippet(snippet: SnippetString, where?: Position | readonly Position[] | Range | readonly Range[], options: { undoStopBefore: boolean; undoStopAfter: boolean; } = { undoStopBefore: true, undoStopAfter: true }): Promise { - if (this._disposed) { - return Promise.reject(new Error('TextEditor#insertSnippet not possible on closed editors')); - } - let ranges: IRange[]; - - if (!where || (Array.isArray(where) && where.length === 0)) { - ranges = this._selections.map(range => TypeConverters.Range.from(range)); - - } else if (where instanceof Position) { - const { lineNumber, column } = TypeConverters.Position.from(where); - ranges = [{ startLineNumber: lineNumber, startColumn: column, endLineNumber: lineNumber, endColumn: column }]; - - } else if (where instanceof Range) { - ranges = [TypeConverters.Range.from(where)]; - } else { - ranges = []; - for (const posOrRange of where) { - if (posOrRange instanceof Range) { - ranges.push(TypeConverters.Range.from(posOrRange)); - } else { - const { lineNumber, column } = TypeConverters.Position.from(posOrRange); - ranges.push({ startLineNumber: lineNumber, startColumn: column, endLineNumber: lineNumber, endColumn: column }); - } - } - } - - return this._proxy.$tryInsertSnippet(this.id, snippet.value, ranges, options); - } - - // ---- util - private _runOnProxy(callback: () => Promise): Promise { if (this._disposed) { this._logService.warn('TextEditor is closed/disposed'); return Promise.resolve(undefined); } + return callback().then(() => this, err => { if (!(err instanceof Error && err.name === 'DISPOSED')) { this._logService.warn(err); @@ -659,4 +614,3 @@ export class ExtHostTextEditor implements vscode.TextEditor { }); } } - diff --git a/src/vs/workbench/api/common/extHostTextEditors.ts b/src/vs/workbench/api/common/extHostTextEditors.ts index 3e95b2f382efd..b01d113fe2b69 100644 --- a/src/vs/workbench/api/common/extHostTextEditors.ts +++ b/src/vs/workbench/api/common/extHostTextEditors.ts @@ -41,12 +41,17 @@ export class ExtHostEditors implements ExtHostEditorsShape { this._extHostDocumentsAndEditors.onDidChangeActiveTextEditor(e => this._onDidChangeActiveTextEditor.fire(e)); } - getActiveTextEditor(): ExtHostTextEditor | undefined { + getActiveTextEditor(): vscode.TextEditor | undefined { return this._extHostDocumentsAndEditors.activeEditor(); } - getVisibleTextEditors(): vscode.TextEditor[] { - return this._extHostDocumentsAndEditors.allEditors(); + getVisibleTextEditors(): vscode.TextEditor[]; + getVisibleTextEditors(internal: true): ExtHostTextEditor[]; + getVisibleTextEditors(internal?: true): ExtHostTextEditor[] | vscode.TextEditor[] { + const editors = this._extHostDocumentsAndEditors.allEditors(); + return internal + ? editors + : editors.map(editor => editor.value); } showTextDocument(document: vscode.TextDocument, column: vscode.ViewColumn, preserveFocus: boolean): Promise; @@ -75,7 +80,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { const editorId = await this._proxy.$tryShowTextDocument(document.uri, options); const editor = editorId && this._extHostDocumentsAndEditors.getEditor(editorId); if (editor) { - return editor; + return editor.value; } // we have no editor... having an id means that we had an editor // on the main side and that it isn't the current editor anymore... @@ -114,7 +119,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { // (2) fire change events if (data.options) { this._onDidChangeTextEditorOptions.fire({ - textEditor: textEditor, + textEditor: textEditor.value, options: { ...data.options, lineNumbers: TypeConverters.TextEditorLineNumbersStyle.to(data.options.lineNumbers) } }); } @@ -122,7 +127,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { const kind = TextEditorSelectionChangeKind.fromValue(data.selections.source); const selections = data.selections.selections.map(TypeConverters.Selection.to); this._onDidChangeTextEditorSelection.fire({ - textEditor, + textEditor: textEditor.value, selections, kind }); @@ -130,7 +135,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { if (data.visibleRanges) { const visibleRanges = arrays.coalesce(data.visibleRanges.map(TypeConverters.Range.to)); this._onDidChangeTextEditorVisibleRanges.fire({ - textEditor, + textEditor: textEditor.value, visibleRanges }); } @@ -143,9 +148,9 @@ export class ExtHostEditors implements ExtHostEditorsShape { throw new Error('Unknown text editor'); } const viewColumn = TypeConverters.ViewColumn.to(data[id]); - if (textEditor.viewColumn !== viewColumn) { + if (textEditor.value.viewColumn !== viewColumn) { textEditor._acceptViewColumn(viewColumn); - this._onDidChangeTextEditorViewColumn.fire({ textEditor, viewColumn }); + this._onDidChangeTextEditorViewColumn.fire({ textEditor: textEditor.value, viewColumn }); } } } diff --git a/src/vs/workbench/test/browser/api/extHostTextEditor.test.ts b/src/vs/workbench/test/browser/api/extHostTextEditor.test.ts index 6e59c522f2ab0..ca9a7f7999daf 100644 --- a/src/vs/workbench/test/browser/api/extHostTextEditor.test.ts +++ b/src/vs/workbench/test/browser/api/extHostTextEditor.test.ts @@ -11,6 +11,7 @@ import { ExtHostDocumentData } from 'vs/workbench/api/common/extHostDocumentData import { URI } from 'vs/base/common/uri'; import { mock } from 'vs/base/test/common/mock'; import { NullLogService } from 'vs/platform/log/common/log'; +import { Lazy } from 'vs/base/common/lazy'; suite('ExtHostTextEditor', () => { @@ -20,21 +21,21 @@ suite('ExtHostTextEditor', () => { ], '\n', 1, 'text', false); setup(() => { - editor = new ExtHostTextEditor('fake', null!, new NullLogService(), doc, [], { cursorStyle: 0, insertSpaces: true, lineNumbers: 1, tabSize: 4, indentSize: 4 }, [], 1); + editor = new ExtHostTextEditor('fake', null!, new NullLogService(), new Lazy(() => doc.document), [], { cursorStyle: 0, insertSpaces: true, lineNumbers: 1, tabSize: 4 }, [], 1); }); test('disposed editor', () => { - assert.ok(editor.document); + assert.ok(editor.value.document); editor._acceptViewColumn(3); - assert.strictEqual(3, editor.viewColumn); + assert.strictEqual(3, editor.value.viewColumn); editor.dispose(); assert.throws(() => editor._acceptViewColumn(2)); - assert.strictEqual(3, editor.viewColumn); + assert.strictEqual(3, editor.value.viewColumn); - assert.ok(editor.document); + assert.ok(editor.value.document); assert.throws(() => editor._acceptOptions(null!)); assert.throws(() => editor._acceptSelections([])); }); @@ -47,15 +48,15 @@ suite('ExtHostTextEditor', () => { applyCount += 1; return Promise.resolve(true); } - }, new NullLogService(), doc, [], { cursorStyle: 0, insertSpaces: true, lineNumbers: 1, tabSize: 4, indentSize: 4 }, [], 1); + }, new NullLogService(), new Lazy(() => doc.document), [], { cursorStyle: 0, insertSpaces: true, lineNumbers: 1, tabSize: 4 }, [], 1); - await editor.edit(edit => { }); + await editor.value.edit(edit => { }); assert.strictEqual(applyCount, 0); - await editor.edit(edit => { edit.setEndOfLine(1); }); + await editor.value.edit(edit => { edit.setEndOfLine(1); }); assert.strictEqual(applyCount, 1); - await editor.edit(edit => { edit.delete(new Range(0, 0, 1, 1)); }); + await editor.value.edit(edit => { edit.delete(new Range(0, 0, 1, 1)); }); assert.strictEqual(applyCount, 2); }); }); @@ -89,7 +90,6 @@ suite('ExtHostTextEditorOptions', () => { }; opts = new ExtHostTextEditorOptions(mockProxy, '1', { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -103,20 +103,18 @@ suite('ExtHostTextEditorOptions', () => { function assertState(opts: ExtHostTextEditorOptions, expected: IResolvedTextEditorConfiguration): void { let actual = { - tabSize: opts.tabSize, - indentSize: opts.indentSize, - insertSpaces: opts.insertSpaces, - cursorStyle: opts.cursorStyle, - lineNumbers: opts.lineNumbers + tabSize: opts.value.tabSize, + insertSpaces: opts.value.insertSpaces, + cursorStyle: opts.value.cursorStyle, + lineNumbers: opts.value.lineNumbers }; assert.deepStrictEqual(actual, expected); } test('can set tabSize to the same value', () => { - opts.tabSize = 4; + opts.value.tabSize = 4; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -125,10 +123,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can change tabSize to positive integer', () => { - opts.tabSize = 1; + opts.value.tabSize = 1; assertState(opts, { tabSize: 1, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -137,10 +134,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can change tabSize to positive float', () => { - opts.tabSize = 2.3; + opts.value.tabSize = 2.3; assertState(opts, { tabSize: 2, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -149,10 +145,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can change tabSize to a string number', () => { - opts.tabSize = '2'; + opts.value.tabSize = '2'; assertState(opts, { tabSize: 2, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -161,10 +156,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('tabSize can request indentation detection', () => { - opts.tabSize = 'auto'; + opts.value.tabSize = 'auto'; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -173,10 +167,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('ignores invalid tabSize 1', () => { - opts.tabSize = null!; + opts.value.tabSize = null!; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -185,10 +178,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('ignores invalid tabSize 2', () => { - opts.tabSize = -5; + opts.value.tabSize = -5; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -197,10 +189,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('ignores invalid tabSize 3', () => { - opts.tabSize = 'hello'; + opts.value.tabSize = 'hello'; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -209,130 +200,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('ignores invalid tabSize 4', () => { - opts.tabSize = '-17'; + opts.value.tabSize = '-17'; assertState(opts, { tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('can set indentSize to the same value', () => { - opts.indentSize = 4; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('can change indentSize to positive integer', () => { - opts.indentSize = 1; - assertState(opts, { - tabSize: 4, - indentSize: 1, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, [{ indentSize: 1 }]); - }); - - test('can change indentSize to positive float', () => { - opts.indentSize = 2.3; - assertState(opts, { - tabSize: 4, - indentSize: 2, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, [{ indentSize: 2 }]); - }); - - test('can change indentSize to a string number', () => { - opts.indentSize = '2'; - assertState(opts, { - tabSize: 4, - indentSize: 2, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, [{ indentSize: 2 }]); - }); - - test('indentSize can request to use tabSize', () => { - opts.indentSize = 'tabSize'; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, [{ indentSize: 'tabSize' }]); - }); - - test('indentSize cannot request indentation detection', () => { - opts.indentSize = 'auto'; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('ignores invalid indentSize 1', () => { - opts.indentSize = null!; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('ignores invalid indentSize 2', () => { - opts.indentSize = -5; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('ignores invalid indentSize 3', () => { - opts.indentSize = 'hello'; - assertState(opts, { - tabSize: 4, - indentSize: 4, - insertSpaces: false, - cursorStyle: TextEditorCursorStyle.Line, - lineNumbers: RenderLineNumbersType.On - }); - assert.deepStrictEqual(calls, []); - }); - - test('ignores invalid indentSize 4', () => { - opts.indentSize = '-17'; - assertState(opts, { - tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -341,10 +211,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set insertSpaces to the same value', () => { - opts.insertSpaces = false; + opts.value.insertSpaces = false; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -353,10 +222,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set insertSpaces to boolean', () => { - opts.insertSpaces = true; + opts.value.insertSpaces = true; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: true, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -365,10 +233,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set insertSpaces to false string', () => { - opts.insertSpaces = 'false'; + opts.value.insertSpaces = 'false'; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -377,10 +244,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set insertSpaces to truey', () => { - opts.insertSpaces = 'hello'; + opts.value.insertSpaces = 'hello'; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: true, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -389,10 +255,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('insertSpaces can request indentation detection', () => { - opts.insertSpaces = 'auto'; + opts.value.insertSpaces = 'auto'; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -401,10 +266,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set cursorStyle to same value', () => { - opts.cursorStyle = TextEditorCursorStyle.Line; + opts.value.cursorStyle = TextEditorCursorStyle.Line; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -413,10 +277,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can change cursorStyle', () => { - opts.cursorStyle = TextEditorCursorStyle.Block; + opts.value.cursorStyle = TextEditorCursorStyle.Block; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Block, lineNumbers: RenderLineNumbersType.On @@ -425,10 +288,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can set lineNumbers to same value', () => { - opts.lineNumbers = TextEditorLineNumbersStyle.On; + opts.value.lineNumbers = TextEditorLineNumbersStyle.On; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -437,10 +299,9 @@ suite('ExtHostTextEditorOptions', () => { }); test('can change lineNumbers', () => { - opts.lineNumbers = TextEditorLineNumbersStyle.Off; + opts.value.lineNumbers = TextEditorLineNumbersStyle.Off; assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.Off @@ -457,7 +318,6 @@ suite('ExtHostTextEditorOptions', () => { }); assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -472,7 +332,6 @@ suite('ExtHostTextEditorOptions', () => { }); assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: true, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -487,7 +346,6 @@ suite('ExtHostTextEditorOptions', () => { }); assertState(opts, { tabSize: 3, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Line, lineNumbers: RenderLineNumbersType.On @@ -502,7 +360,6 @@ suite('ExtHostTextEditorOptions', () => { }); assertState(opts, { tabSize: 4, - indentSize: 4, insertSpaces: false, cursorStyle: TextEditorCursorStyle.Block, lineNumbers: RenderLineNumbersType.Relative From 3d2ca29012c9dfc4e6671cbda949b458713d7827 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 1 Feb 2021 11:55:06 +0100 Subject: [PATCH 08/11] hide terminal proxy --- .../api/common/extHostTerminalService.ts | 123 +++++++++--------- .../api/node/extHostTerminalService.ts | 4 +- 2 files changed, 62 insertions(+), 65 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index f684dbb85b4b9..babcce145eb0a 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -47,7 +47,7 @@ export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, ID export const IExtHostTerminalService = createDecorator('IExtHostTerminalService'); -export class ExtHostTerminal implements vscode.Terminal { +export class ExtHostTerminal { private _disposed: boolean = false; private _pidPromise: Promise; private _cols: number | undefined; @@ -57,6 +57,8 @@ export class ExtHostTerminal implements vscode.Terminal { public isOpen: boolean = false; + readonly value: vscode.Terminal; + constructor( private _proxy: MainThreadTerminalServiceShape, public _id: TerminalIdentifier, @@ -65,6 +67,49 @@ export class ExtHostTerminal implements vscode.Terminal { ) { this._creationOptions = Object.freeze(this._creationOptions); this._pidPromise = new Promise(c => this._pidPromiseComplete = c); + + const that = this; + this.value = { + get name(): string { + return that._name || ''; + }, + get processId(): Promise { + return that._pidPromise; + }, + get creationOptions(): Readonly { + return that._creationOptions; + }, + get exitStatus(): vscode.TerminalExitStatus | undefined { + return that._exitStatus; + }, + sendText(text: string, addNewLine: boolean = true): void { + that._checkDisposed(); + that._proxy.$sendText(that._id, text, addNewLine); + }, + show(preserveFocus: boolean): void { + that._checkDisposed(); + that._proxy.$show(that._id, preserveFocus); + }, + hide(): void { + that._checkDisposed(); + that._proxy.$hide(that._id); + }, + dispose(): void { + if (!that._disposed) { + that._disposed = true; + that._proxy.$dispose(that._id); + } + }, + get dimensions(): vscode.TerminalDimensions | undefined { + if (that._cols === undefined || that._rows === undefined) { + return undefined; + } + return { + columns: that._cols, + rows: that._rows + }; + } + }; } public async create( @@ -95,41 +140,16 @@ export class ExtHostTerminal implements vscode.Terminal { return this._id; } - public dispose(): void { - if (!this._disposed) { - this._disposed = true; - this._proxy.$dispose(this._id); - } - } - private _checkDisposed() { if (this._disposed) { throw new Error('Terminal has already been disposed'); } } - public get name(): string { - return this._name || ''; - } - public set name(name: string) { this._name = name; } - public get exitStatus(): vscode.TerminalExitStatus | undefined { - return this._exitStatus; - } - - public get dimensions(): vscode.TerminalDimensions | undefined { - if (this._cols === undefined || this._rows === undefined) { - return undefined; - } - return { - columns: this._cols, - rows: this._rows - }; - } - public setExitCode(code: number | undefined) { this._exitStatus = Object.freeze({ code }); } @@ -147,29 +167,6 @@ export class ExtHostTerminal implements vscode.Terminal { return true; } - public get processId(): Promise { - return this._pidPromise; - } - - public get creationOptions(): Readonly { - return this._creationOptions; - } - - public sendText(text: string, addNewLine: boolean = true): void { - this._checkDisposed(); - this._proxy.$sendText(this._id, text, addNewLine); - } - - public show(preserveFocus: boolean): void { - this._checkDisposed(); - this._proxy.$show(this._id, preserveFocus); - } - - public hide(): void { - this._checkDisposed(); - this._proxy.$hide(this._id); - } - public _setProcessId(processId: number | undefined): void { // The event may fire 2 times when the panel is restored if (this._pidPromiseComplete) { @@ -284,8 +281,8 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I private readonly _terminalLinkCache: Map> = new Map(); private readonly _terminalLinkCancellationSource: Map = new Map(); - public get activeTerminal(): ExtHostTerminal | undefined { return this._activeTerminal; } - public get terminals(): ExtHostTerminal[] { return this._terminals; } + public get activeTerminal(): vscode.Terminal | undefined { return this._activeTerminal?.value; } + public get terminals(): vscode.Terminal[] { return this._terminals.map(term => term.value); } protected readonly _onDidCloseTerminal: Emitter = new Emitter(); public get onDidCloseTerminal(): Event { return this._onDidCloseTerminal && this._onDidCloseTerminal.event; } @@ -336,7 +333,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I this._terminalProcessDisposables[id] = disposable; }); this._terminals.push(terminal); - return terminal; + return terminal.value; } public attachPtyToTerminal(id: number, pty: vscode.Pseudoterminal): void { @@ -362,7 +359,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I if (terminal) { this._activeTerminal = terminal; if (original !== this._activeTerminal) { - this._onDidChangeActiveTerminal.fire(this._activeTerminal); + this._onDidChangeActiveTerminal.fire(this._activeTerminal.value); } } } @@ -370,7 +367,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I public async $acceptTerminalProcessData(id: number, data: string): Promise { const terminal = this._getTerminalById(id); if (terminal) { - this._onDidWriteTerminalData.fire({ terminal, data }); + this._onDidWriteTerminalData.fire({ terminal: terminal.value, data }); } } @@ -379,8 +376,8 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I if (terminal) { if (terminal.setDimensions(cols, rows)) { this._onDidChangeTerminalDimensions.fire({ - terminal: terminal, - dimensions: terminal.dimensions as vscode.TerminalDimensions + terminal: terminal.value, + dimensions: terminal.value.dimensions as vscode.TerminalDimensions }); } } @@ -400,11 +397,11 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I } public async $acceptTerminalClosed(id: number, exitCode: number | undefined): Promise { - const index = this._getTerminalObjectIndexById(this.terminals, id); + const index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { const terminal = this._terminals.splice(index, 1)[0]; terminal.setExitCode(exitCode); - this._onDidCloseTerminal.fire(terminal); + this._onDidCloseTerminal.fire(terminal.value); } } @@ -414,9 +411,9 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I const index = this._getTerminalObjectIndexById(this._terminals, extHostTerminalId); if (index !== null) { // The terminal has already been created (via createTerminal*), only fire the event - this.terminals[index]._id = id; + this._terminals[index]._id = id; this._onDidOpenTerminal.fire(this.terminals[index]); - this.terminals[index].isOpen = true; + this._terminals[index].isOpen = true; return; } } @@ -431,7 +428,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I }; const terminal = new ExtHostTerminal(this._proxy, id, creationOptions, name); this._terminals.push(terminal); - this._onDidOpenTerminal.fire(terminal); + this._onDidOpenTerminal.fire(terminal.value); terminal.isOpen = true; } @@ -455,7 +452,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I await new Promise(r => { // Ensure open is called after onDidOpenTerminal const listener = this.onDidOpenTerminal(async e => { - if (e === terminal) { + if (e === terminal.value) { listener.dispose(); r(); } @@ -564,7 +561,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I this._terminalLinkCancellationSource.set(terminalId, cancellationSource); const result: ITerminalLinkDto[] = []; - const context: vscode.TerminalLinkContext = { terminal, line }; + const context: vscode.TerminalLinkContext = { terminal: terminal.value, line }; const promises: vscode.ProviderResult<{ provider: vscode.TerminalLinkProvider, links: vscode.TerminalLink[] }>[] = []; for (const provider of this._linkProviders) { diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index adad271b38fbb..53616f44ba335 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -60,7 +60,7 @@ export class ExtHostTerminalService extends BaseExtHostTerminalService { const terminal = new ExtHostTerminal(this._proxy, generateUuid(), { name, shellPath, shellArgs }, name); this._terminals.push(terminal); terminal.create(shellPath, shellArgs); - return terminal; + return terminal.value; } public createTerminalFromOptions(options: vscode.TerminalOptions, isFeatureTerminal?: boolean): vscode.Terminal { @@ -75,7 +75,7 @@ export class ExtHostTerminalService extends BaseExtHostTerminalService { withNullAsUndefined(options.strictEnv), withNullAsUndefined(options.hideFromUser), withNullAsUndefined(isFeatureTerminal)); - return terminal; + return terminal.value; } public getDefaultShell(useAutomationShell: boolean, configProvider: ExtHostConfigProvider): string { From a0c75f60530da98fbccf796a7c78c23a7ca168d3 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 1 Feb 2021 14:52:37 +0100 Subject: [PATCH 09/11] add tests for createXYZ functions and skip most of them... --- .../src/singlefolder-tests/rpc.test.ts | 49 ++++++++++++++++++- extensions/vscode-api-tests/src/utils.ts | 6 ++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index 596a7f9746f3a..68707e42df213 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -3,11 +3,58 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { assertNoRpc } from '../utils'; +import { assertNoRpcFromEntry, assertNoRpc, disposeAll } from '../utils'; +import * as vscode from 'vscode'; suite('vscode', function () { + const dispo: vscode.Disposable[] = []; + + teardown(() => { + assertNoRpc(); + disposeAll(dispo); + }); + test('no rpc', function () { assertNoRpc(); }); + + test('no rpc, createTextEditorDecorationType(...)', function () { + this.skip(); + const item = vscode.window.createTextEditorDecorationType({}); + dispo.push(item); + assertNoRpcFromEntry([item, 'TextEditorDecorationType']); + }); + + test('no rpc, createOutputChannel(...)', function () { + const item = vscode.window.createOutputChannel('hello'); + dispo.push(item); + assertNoRpcFromEntry([item, 'OutputChannel']); + }); + + test('no rpc, createDiagnosticCollection(...)', function () { + const item = vscode.languages.createDiagnosticCollection(); + dispo.push(item); + assertNoRpcFromEntry([item, 'DiagnosticCollection']); + }); + + test('no rpc, createStatusBarItem(...)', function () { + this.skip(); + const item = vscode.window.createStatusBarItem(); + dispo.push(item); + assertNoRpcFromEntry([item, 'StatusBarItem']); + }); + + test('no rpc, createSourceControl(...)', function () { + this.skip(); + const item = vscode.scm.createSourceControl('foo', 'Hello'); + dispo.push(item); + assertNoRpcFromEntry([item, 'SourceControl']); + }); + test('no rpc, createCommentController(...)', function () { + this.skip(); + const item = vscode.comments.createCommentController('foo', 'Hello'); + dispo.push(item); + assertNoRpcFromEntry([item, 'CommentController']); + }); }); diff --git a/extensions/vscode-api-tests/src/utils.ts b/extensions/vscode-api-tests/src/utils.ts index f35acafde7d9a..f7923bbc9f79e 100644 --- a/extensions/vscode-api-tests/src/utils.ts +++ b/extensions/vscode-api-tests/src/utils.ts @@ -74,6 +74,10 @@ export function withLogDisabled(runnable: () => Promise): () => Promise Date: Mon, 1 Feb 2021 17:06:51 +0100 Subject: [PATCH 10/11] hide proxy from TextEditorDecorationType --- .../src/singlefolder-tests/rpc.test.ts | 1 - .../workbench/api/common/extHostTextEditor.ts | 19 ++++++++++--------- .../api/common/extHostTextEditors.ts | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index 68707e42df213..7d60f3e8771a0 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -20,7 +20,6 @@ suite('vscode', function () { }); test('no rpc, createTextEditorDecorationType(...)', function () { - this.skip(); const item = vscode.window.createTextEditorDecorationType({}); dispo.push(item); assertNoRpcFromEntry([item, 'TextEditorDecorationType']); diff --git a/src/vs/workbench/api/common/extHostTextEditor.ts b/src/vs/workbench/api/common/extHostTextEditor.ts index 6e2a3e4e3249e..f3dff81c54448 100644 --- a/src/vs/workbench/api/common/extHostTextEditor.ts +++ b/src/vs/workbench/api/common/extHostTextEditor.ts @@ -16,22 +16,23 @@ import type * as vscode from 'vscode'; import { ILogService } from 'vs/platform/log/common/log'; import { Lazy } from 'vs/base/common/lazy'; -export class TextEditorDecorationType implements vscode.TextEditorDecorationType { +export class TextEditorDecorationType { private static readonly _Keys = new IdGenerator('TextEditorDecorationType'); - private _proxy: MainThreadTextEditorsShape; - public key: string; + readonly value: vscode.TextEditorDecorationType; constructor(proxy: MainThreadTextEditorsShape, options: vscode.DecorationRenderOptions) { - this.key = TextEditorDecorationType._Keys.nextId(); - this._proxy = proxy; - this._proxy.$registerTextEditorDecorationType(this.key, TypeConverters.DecorationRenderOptions.from(options)); + const key = TextEditorDecorationType._Keys.nextId(); + proxy.$registerTextEditorDecorationType(key, TypeConverters.DecorationRenderOptions.from(options)); + this.value = Object.freeze({ + key, + dispose() { + proxy.$removeTextEditorDecorationType(key); + } + }); } - public dispose(): void { - this._proxy.$removeTextEditorDecorationType(this.key); - } } export interface ITextEditOperation { diff --git a/src/vs/workbench/api/common/extHostTextEditors.ts b/src/vs/workbench/api/common/extHostTextEditors.ts index b01d113fe2b69..dc8bd7bd91082 100644 --- a/src/vs/workbench/api/common/extHostTextEditors.ts +++ b/src/vs/workbench/api/common/extHostTextEditors.ts @@ -92,7 +92,7 @@ export class ExtHostEditors implements ExtHostEditorsShape { } createTextEditorDecorationType(options: vscode.DecorationRenderOptions): vscode.TextEditorDecorationType { - return new TextEditorDecorationType(this._proxy, options); + return new TextEditorDecorationType(this._proxy, options).value; } // --- called from main thread From a99dd66f0f70f547e728ce468a41dee7d90841bf Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 1 Feb 2021 17:41:29 +0100 Subject: [PATCH 11/11] test more results of create-functions --- .../src/singlefolder-tests/rpc.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts index 7d60f3e8771a0..0d203040d3a1d 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/rpc.test.ts @@ -37,6 +37,20 @@ suite('vscode', function () { assertNoRpcFromEntry([item, 'DiagnosticCollection']); }); + test('no rpc, createQuickPick(...)', function () { + this.skip(); + const item = vscode.window.createQuickPick(); + dispo.push(item); + assertNoRpcFromEntry([item, 'QuickPick']); + }); + + test('no rpc, createInputBox(...)', function () { + this.skip(); + const item = vscode.window.createInputBox(); + dispo.push(item); + assertNoRpcFromEntry([item, 'InputBox']); + }); + test('no rpc, createStatusBarItem(...)', function () { this.skip(); const item = vscode.window.createStatusBarItem(); @@ -50,10 +64,31 @@ suite('vscode', function () { dispo.push(item); assertNoRpcFromEntry([item, 'SourceControl']); }); + test('no rpc, createCommentController(...)', function () { this.skip(); const item = vscode.comments.createCommentController('foo', 'Hello'); dispo.push(item); assertNoRpcFromEntry([item, 'CommentController']); }); + + test('no rpc, createWebviewPanel(...)', function () { + const item = vscode.window.createWebviewPanel('webview', 'Hello', vscode.ViewColumn.Active); + dispo.push(item); + assertNoRpcFromEntry([item, 'WebviewPanel']); + }); + + test('no rpc, createTreeView(...)', function () { + const treeDataProvider = new class implements vscode.TreeDataProvider { + getTreeItem(element: string): vscode.TreeItem | Thenable { + return new vscode.TreeItem(element); + } + getChildren(_element?: string): vscode.ProviderResult { + return ['foo', 'bar']; + } + }; + const item = vscode.window.createTreeView('test.treeId', { treeDataProvider }); + dispo.push(item); + assertNoRpcFromEntry([item, 'TreeView']); + }); });