From 8e183f0d2ed6fa2295a6b7d35f6d736842cbb9ce Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Mon, 12 Apr 2021 11:03:54 -0700 Subject: [PATCH] terminal: standardize on customPtyImplementation I initially was doing a larger refactor where the `onInstanceRequestStartExtensionTerminal` event was removed from the terminalService and instead events would only be fired in the proxy instance itself, which would be encapsulated better. However, the task system depends on the global even firing so that the MainThreadTerminalService can get the terminal for tasks that create pseudoterminals, so this didn't work out. Kept a more focus refactor for now. --- src/vs/platform/terminal/common/terminal.ts | 8 +------- src/vs/workbench/api/browser/mainThreadTerminalService.ts | 3 ++- .../workbench/contrib/tasks/browser/terminalTaskSystem.ts | 3 ++- .../contrib/terminal/browser/terminalInstance.ts | 2 +- .../terminal/browser/terminalProcessExtHostProxy.ts | 6 +----- .../contrib/terminal/browser/terminalProcessManager.ts | 5 ++--- .../workbench/contrib/terminal/browser/terminalService.ts | 2 +- .../testing/browser/testingOutputTerminalService.ts | 2 +- 8 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/vs/platform/terminal/common/terminal.ts b/src/vs/platform/terminal/common/terminal.ts index 1e2ec937a2e..f6227af2e19 100644 --- a/src/vs/platform/terminal/common/terminal.ts +++ b/src/vs/platform/terminal/common/terminal.ts @@ -238,16 +238,10 @@ export interface IShellLaunchConfig { */ initialText?: string; - /** - * Whether an extension is controlling the terminal via a `vscode.Pseudoterminal`. - */ - isExtensionCustomPtyTerminal?: boolean; - /** * Custom PTY/pseudoterminal process to use. - * @todo should `TerminalProcessExtHostProxy` be passed to here and remove `isExtensionCustomPtyTerminal`? */ - customPtyImplementation?: ITerminalChildProcess; + customPtyImplementation?: (terminalId: number, cols: number, rows: number) => ITerminalChildProcess; /** * A UUID generated by the extension host process for terminals created on the extension host process. diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 856a0f0cc0c..a30e9a22dfc 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -13,6 +13,7 @@ import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBu import { ExtHostContext, ExtHostTerminalServiceShape, IExtHostContext, ITerminalDimensionsDto, MainContext, MainThreadTerminalServiceShape, TerminalIdentifier, TerminalLaunchConfig } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { ITerminalExternalLinkProvider, ITerminalInstance, ITerminalInstanceService, ITerminalLink, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy'; import { IEnvironmentVariableService, ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared'; import { IAvailableProfilesRequest as IAvailableProfilesRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest, ITerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/common/terminal'; @@ -130,7 +131,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape env: launchConfig.env, strictEnv: launchConfig.strictEnv, hideFromUser: launchConfig.hideFromUser, - isExtensionCustomPtyTerminal: launchConfig.isExtensionCustomPtyTerminal, + customPtyImplementation: (id, rows, cols) => new TerminalProcessExtHostProxy(id, cols, rows, this._terminalService), extHostTerminalId: extHostTerminalId, isFeatureTerminal: launchConfig.isFeatureTerminal, isExtensionOwnedTerminal: launchConfig.isExtensionOwnedTerminal diff --git a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts index 05b15d8039e..ad4df1a6304 100644 --- a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts @@ -48,6 +48,7 @@ import { IViewsService, IViewDescriptorService, ViewContainerLocation } from 'vs import { ILogService } from 'vs/platform/log/common/log'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IShellLaunchConfig } from 'vs/platform/terminal/common/terminal'; +import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy'; interface TerminalData { terminal: ITerminalInstance; @@ -1163,7 +1164,7 @@ export class TerminalTaskSystem implements ITaskSystem { if (task.command.runtime === RuntimeType.CustomExecution) { this.currentTask.shellLaunchConfig = launchConfigs = { - isExtensionCustomPtyTerminal: true, + customPtyImplementation: (id, rows, cols) => new TerminalProcessExtHostProxy(id, cols, rows, this.terminalService), waitOnExit, name: this.createTerminalName(task), initialText: task.command.presentation && task.command.presentation.echo ? `\x1b[1m> Executing task: ${task._label} <\x1b[0m\n` : undefined, diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 6450ad88010..ec4ba753435 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -1621,7 +1621,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { this._configHelper.config.environmentChangesRelaunch && !this._processManager.hasWrittenData && !this._shellLaunchConfig.isFeatureTerminal && - !this._shellLaunchConfig.isExtensionCustomPtyTerminal + !this._shellLaunchConfig.customPtyImplementation && !this._shellLaunchConfig.isExtensionOwnedTerminal && !this._shellLaunchConfig.attachPersistentProcess ) { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts index 60afdf9d6f0..b0b17bbfa0c 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts @@ -54,10 +54,9 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal constructor( public instanceId: number, - private _shellLaunchConfig: IShellLaunchConfig, private _cols: number, private _rows: number, - @ITerminalService private readonly _terminalService: ITerminalService + @ITerminalService private readonly _terminalService: ITerminalService, ) { super(); } @@ -106,9 +105,6 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal } public async start(): Promise { - if (!this._shellLaunchConfig.isExtensionCustomPtyTerminal) { - throw new Error('Attempt to start an ext host process that is not an extension terminal'); - } return this._terminalService.requestStartExtensionTerminal(this, this._cols, this._rows); } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 9f37c9694ef..e65d5f08c74 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -10,7 +10,6 @@ import { ProcessState, ITerminalProcessManager, ITerminalConfigHelper, IBeforePr import { ILogService } from 'vs/platform/log/common/log'; import { Emitter, Event } from 'vs/base/common/event'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; -import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy'; import { IInstantiationService, optional } from 'vs/platform/instantiation/common/instantiation'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -191,9 +190,9 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce let newProcess: ITerminalChildProcess; - if (shellLaunchConfig.isExtensionCustomPtyTerminal || shellLaunchConfig.customPtyImplementation) { + if (shellLaunchConfig.customPtyImplementation) { this._processType = ProcessType.PsuedoTerminal; - newProcess = shellLaunchConfig.customPtyImplementation || this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._instanceId, shellLaunchConfig, cols, rows); + newProcess = shellLaunchConfig.customPtyImplementation(this._instanceId, cols, rows); } else { const forceExtHostProcess = (this._configHelper.config as any).extHostProcess; if (shellLaunchConfig.cwd && typeof shellLaunchConfig.cwd === 'object') { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index a9073fda529..c79ceb0e82d 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -996,7 +996,7 @@ export class TerminalService implements ITerminalService { public createTerminal(shellLaunchConfigOrProfile: IShellLaunchConfig | ITerminalProfile): ITerminalInstance { const shellLaunchConfig = this._convertProfileToShellLaunchConfig(shellLaunchConfigOrProfile); - if (!shellLaunchConfig.isExtensionCustomPtyTerminal && !this.isProcessSupportRegistered) { + if (!shellLaunchConfig.customPtyImplementation && !this.isProcessSupportRegistered) { throw new Error('Could not create terminal when process support is not registered'); } if (shellLaunchConfig.hideFromUser) { diff --git a/src/vs/workbench/contrib/testing/browser/testingOutputTerminalService.ts b/src/vs/workbench/contrib/testing/browser/testingOutputTerminalService.ts index 918280afb6f..a5e2682226a 100644 --- a/src/vs/workbench/contrib/testing/browser/testingOutputTerminalService.ts +++ b/src/vs/workbench/contrib/testing/browser/testingOutputTerminalService.ts @@ -66,7 +66,7 @@ export class TestingOutputTerminalService implements ITestingOutputTerminalServi output = new TestOutputProcess(); terminal = this.terminalService.createTerminal({ isFeatureTerminal: true, - customPtyImplementation: output, + customPtyImplementation: () => output, name: title, }) as TestOutputTerminalInstance; }