From c416e9b2c40698c60713aab1cfa3e7b7b304bdad Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 7 May 2021 11:19:01 -0500 Subject: [PATCH] Use ptyHostService Most of this was a straightforward replacement of our code with theirs but I also removed `getDefaultShellAndArgs` since it seems the reference implementation no longer does that either. Fixes #2276. --- .../vs/platform/terminal/node/ptyService.ts | 5 +- .../platform/terminal/node/terminalProcess.ts | 13 +- lib/vscode/src/vs/server/node/channel.ts | 389 +++--------------- lib/vscode/src/vs/server/node/server.ts | 6 +- 4 files changed, 76 insertions(+), 337 deletions(-) diff --git a/lib/vscode/src/vs/platform/terminal/node/ptyService.ts b/lib/vscode/src/vs/platform/terminal/node/ptyService.ts index 6765dfc3..d9ea875c 100644 --- a/lib/vscode/src/vs/platform/terminal/node/ptyService.ts +++ b/lib/vscode/src/vs/platform/terminal/node/ptyService.ts @@ -79,10 +79,7 @@ export class PtyService extends Disposable implements IPtyService { throw new Error('Attempt to create a process when attach object was provided'); } const id = ++this._lastPtyId; - /** - * NOTE@coder: pass ID into TerminalProcess to fix compile. - */ - const process = new TerminalProcess(id, shellLaunchConfig, cwd, cols, rows, env, executableEnv, windowsEnableConpty, this._logService); + const process = new TerminalProcess(shellLaunchConfig, cwd, cols, rows, env, executableEnv, windowsEnableConpty, this._logService); process.onProcessData(event => this._onProcessData.fire({ id, event })); process.onProcessExit(event => this._onProcessExit.fire({ id, event })); if (process.onProcessOverrideDimensions) { diff --git a/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts b/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts index a62b73ac..3626928f 100644 --- a/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts +++ b/lib/vscode/src/vs/platform/terminal/node/terminalProcess.ts @@ -68,6 +68,7 @@ interface IWriteObject { } export class TerminalProcess extends Disposable implements ITerminalChildProcess { + readonly id = 0; readonly shouldPersist = false; private static _lastKillOrStart = 0; @@ -75,14 +76,8 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess private _exitCode: number | undefined; private _exitMessage: string | undefined; private _closeTimeout: any; - /** - * NOTE@coder: set _ptyProcess and _currentTitle to protected - * to allow access from subclasses. - * - * We subclass it in src/vs/server/channel.ts - */ - protected _ptyProcess: pty.IPty | undefined; - protected _currentTitle: string = ''; + private _ptyProcess: pty.IPty | undefined; + private _currentTitle: string = ''; private _processStartupComplete: Promise | undefined; private _isDisposed: boolean = false; private _windowsShellHelper: WindowsShellHelper | undefined; @@ -111,9 +106,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess private readonly _onProcessShellTypeChanged = this._register(new Emitter()); public readonly onProcessShellTypeChanged = this._onProcessShellTypeChanged.event; - // NOTE@coder: add id to constructor constructor( - public readonly id: number = 0, private readonly _shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, diff --git a/lib/vscode/src/vs/server/node/channel.ts b/lib/vscode/src/vs/server/node/channel.ts index 01faa6b9..06e6de6b 100644 --- a/lib/vscode/src/vs/server/node/channel.ts +++ b/lib/vscode/src/vs/server/node/channel.ts @@ -10,7 +10,6 @@ import * as resources from 'vs/base/common/resources'; import { ReadableStreamEventPayload } from 'vs/base/common/stream'; import { URI, UriComponents } from 'vs/base/common/uri'; import { transformOutgoingURIs } from 'vs/base/common/uriIpc'; -import { getSystemShell } from 'vs/base/node/shell'; import { IServerChannel } from 'vs/base/parts/ipc/common/ipc'; import { IDiagnosticInfo } from 'vs/platform/diagnostics/common/diagnostics'; import { INativeEnvironmentService } from 'vs/platform/environment/common/environment'; @@ -21,10 +20,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import product from 'vs/platform/product/common/product'; import { IRemoteAgentEnvironment, RemoteAgentConnectionContext } from 'vs/platform/remote/common/remoteAgentEnvironment'; import { ITelemetryData, ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; -import { IProcessDataEvent, IShellLaunchConfig, ITerminalEnvironment, ITerminalLaunchError, ITerminalsLayoutInfo } from 'vs/platform/terminal/common/terminal'; -import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering'; -import { IGetTerminalLayoutInfoArgs, IProcessDetails, IPtyHostProcessReplayEvent, ISetTerminalLayoutInfoArgs } from 'vs/platform/terminal/common/terminalProcess'; -import { TerminalProcess } from 'vs/platform/terminal/node/terminalProcess'; +import { IPtyService, IShellLaunchConfig, ITerminalEnvironment } from 'vs/platform/terminal/common/terminal'; import { getTranslations } from 'vs/server/node/nls'; import { getUriTransformer } from 'vs/server/node/util'; import { IFileChangeDto } from 'vs/workbench/api/common/extHost.protocol'; @@ -387,122 +383,34 @@ class VariableResolverService extends AbstractVariableResolverService { } } -class Terminal extends TerminalProcess { - private readonly workspaceId: string; - private readonly workspaceName: string; - - // TODO: Implement once we have persistent terminals. - public isOrphan: boolean = false; - - public get title(): string { return this._currentTitle; } - public get pid(): number { return this._ptyProcess?.pid ?? -1; } - - public constructor( - id: number, - config: IShellLaunchConfig & { cwd: string }, - args: terminal.ICreateTerminalProcessArguments, - env: platform.IProcessEnvironment, - logService: ILogService, - ) { - super( - id, - config, - config.cwd, - args.cols, - args.rows, - env, - process.env as platform.IProcessEnvironment, // Environment used for `findExecutable`. - false, // windowsEnableConpty: boolean, - logService, - ); - - this.workspaceId = args.workspaceId; - this.workspaceName = args.workspaceName; - - // Ensure other listeners run before disposing the emitters. - this.onProcessExit(() => setTimeout(() => this.shutdown(true), 1)); - } - - /** - * Run `fn` when the terminal disposes. - */ - public onDispose(dispose: () => void) { - this._register({ dispose }); - } - - /** - * Serializable terminal information that can be sent to the client. - */ - public async description(id: number): Promise { - const cwd = await this.getCwd(); - return { - id, - pid: this.pid, - title: this.title, - cwd, - workspaceId: this.workspaceId, - workspaceName: this.workspaceName, - isOrphan: this.isOrphan, - icon: 'bash' // TODO@oxy: used for icon, but not sure how to resolve it - }; - } -} - -// References: - ../../workbench/api/node/extHostTerminalService.ts -// - ../../workbench/contrib/terminal/browser/terminalProcessManager.ts export class TerminalProviderChannel implements IServerChannel, IDisposable { - private readonly terminals = new Map(); - private id = 0; - - private readonly layouts = new Map(); - - // These re-emit events from terminals with their IDs attached. - private readonly _onProcessData = new Emitter<{ id: number, event: IProcessDataEvent | string }>({ - // Shut down all terminals when all clients disconnect. Unfortunately this - // means that if you open multiple tabs and close one the terminals spawned - // by that tab won't shut down and they can't be reconnected either since we - // don't have persistence yet. - // TODO: Implement persistence. - onLastListenerRemove: () => { - this.terminals.forEach((t) => t.shutdown(true)); - } - }); - private readonly _onProcessExit = new Emitter<{ id: number, event: number | undefined }>(); - private readonly _onProcessReady = new Emitter<{ id: number, event: { pid: number, cwd: string } }>(); - private readonly _onProcessReplay = new Emitter<{ id: number, event: IPtyHostProcessReplayEvent }>(); - private readonly _onProcessTitleChanged = new Emitter<{ id: number, event: string }>(); - - // Buffer to reduce the number of messages going to the renderer. - private readonly bufferer = new TerminalDataBufferer((id, data) => { - this._onProcessData.fire({ id, event: data }); - }); - - public constructor (private readonly logService: ILogService) {} + public constructor ( + private readonly logService: ILogService, + private readonly ptyService: IPtyService, + ) {} public listen(_: RemoteAgentConnectionContext, event: string, args: any): Event { logger.trace('TerminalProviderChannel:listen', field('event', event), field('args', args)); - // TODO@oxy/code-asher: implement these events (currently Event.None) as needed - // Right now, most functionality tested works; - // but VSCode might rely on the other events in the future. switch (event) { - case '$onPtyHostExitEvent': return Event.None; - case '$onPtyHostStartEvent': return Event.None; - case '$onPtyHostUnresponsiveEvent': return Event.None; - case '$onPtyHostResponsiveEvent': return Event.None; + case '$onPtyHostExitEvent': return this.ptyService.onPtyHostExit || Event.None; + case '$onPtyHostStartEvent': return this.ptyService.onPtyHostStart || Event.None; + case '$onPtyHostUnresponsiveEvent': return this.ptyService.onPtyHostUnresponsive || Event.None; + case '$onPtyHostResponsiveEvent': return this.ptyService.onPtyHostResponsive || Event.None; - case '$onProcessDataEvent': return this._onProcessData.event; - case '$onProcessExitEvent': return this._onProcessExit.event; - case '$onProcessReadyEvent': return this._onProcessReady.event; - case '$onProcessReplayEvent': return this._onProcessReplay.event; - case '$onProcessTitleChangedEvent': return this._onProcessTitleChanged.event; - case '$onProcessShellTypeChangedEvent': return Event.None; - case '$onProcessOverrideDimensionsEvent': return Event.None; - case '$onProcessResolvedShellLaunchConfigEvent': return Event.None; - case '$onProcessOrphanQuestion': return Event.None; - // NOTE@code-asher: I think this must have something to do with running commands on - // the terminal that will do things in VS Code but we already have that - // functionality via a socket so I'm not sure what this is for. + case '$onProcessDataEvent': return this.ptyService.onProcessData; + case '$onProcessExitEvent': return this.ptyService.onProcessExit; + case '$onProcessReadyEvent': return this.ptyService.onProcessReady; + case '$onProcessReplayEvent': return this.ptyService.onProcessReplay; + case '$onProcessTitleChangedEvent': return this.ptyService.onProcessTitleChanged; + case '$onProcessShellTypeChangedEvent': return this.ptyService.onProcessShellTypeChanged; + case '$onProcessOverrideDimensionsEvent': return this.ptyService.onProcessOverrideDimensions; + case '$onProcessResolvedShellLaunchConfigEvent': return this.ptyService.onProcessResolvedShellLaunchConfig; + case '$onProcessOrphanQuestion': return this.ptyService.onProcessOrphanQuestion; + // NOTE@asher: I think this must have something to do with running + // commands on the terminal that will do things in VS Code but we + // already have that functionality via a socket so I'm not sure what + // this is for. case '$onExecuteCommand': return Event.None; } @@ -515,40 +423,41 @@ export class TerminalProviderChannel implements IServerChannel t.shutdown(false)); + public async dispose(): Promise { + // Nothing at the moment. } private async restartPtyHost(): Promise { - throw new Error('TODO: restartPtyHost'); + if (this.ptyService.restartPtyHost) { + return this.ptyService.restartPtyHost(); + } } - private async createProcess(remoteAuthority: string, args: terminal.ICreateTerminalProcessArguments): Promise { - const terminalId = this.id++; - logger.debug('Creating terminal', field('id', terminalId), field('terminals', this.terminals.size)); + // References: - ../../workbench/api/node/extHostTerminalService.ts + // - ../../workbench/contrib/terminal/browser/terminalProcessManager.ts + private async createProcess(remoteAuthority: string, args: terminal.ICreateTerminalProcessArguments): Promise { const shellLaunchConfig: IShellLaunchConfig = { name: args.shellLaunchConfig.name, executable: args.shellLaunchConfig.executable, @@ -571,60 +480,14 @@ export class TerminalProviderChannel implements IServerChannel => { - if (shellLaunchConfig.executable) { - const executable = await resolverService.resolveAsync(activeWorkspace, shellLaunchConfig.executable); - let resolvedArgs: string[] | string = []; - if (shellLaunchConfig.args && Array.isArray(shellLaunchConfig.args)) { - for (const arg of shellLaunchConfig.args) { - resolvedArgs.push(await resolverService.resolveAsync(activeWorkspace, arg)); - } - } else if (shellLaunchConfig.args) { - resolvedArgs = await resolverService.resolveAsync(activeWorkspace, shellLaunchConfig.args); - } - return { executable, args: resolvedArgs }; - } - - const executable = terminalEnvironment.getDefaultShell( - (key) => args.configuration[key], - await getSystemShell(platform.OS, process.env as platform.IProcessEnvironment), - process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), - process.env.windir, - resolver, - this.logService, - false, // useAutomationShell - ); - - const resolvedArgs = terminalEnvironment.getDefaultShellArgs( - (key) => args.configuration[key], - false, // useAutomationShell - resolver, - this.logService, - ); - - return { executable, args: resolvedArgs }; - }; - - const getInitialCwd = (): string => { - return terminalEnvironment.getCwd( - shellLaunchConfig, - os.homedir(), - resolver, - activeWorkspaceUri, - args.configuration['terminal.integrated.cwd'], - this.logService, - ); - }; - - // Use a separate var so Typescript recognizes these properties are no - // longer undefined. - const resolvedShellLaunchConfig = { - ...shellLaunchConfig, - ...(await getDefaultShellAndArgs()), - cwd: getInitialCwd(), - }; - - logger.debug('Resolved shell launch configuration', field('id', terminalId)); + shellLaunchConfig.cwd = terminalEnvironment.getCwd( + shellLaunchConfig, + os.homedir(), + resolver, + activeWorkspaceUri, + args.configuration['terminal.integrated.cwd'], + this.logService, + ); // Use instead of `terminal.integrated.env.${platform}` to make types work. const getEnvFromConfig = (): ITerminalEnvironment => { @@ -664,150 +527,32 @@ export class TerminalProviderChannel implements IServerChannel { - this.terminals.delete(terminal.id); - this.bufferer.stopBuffering(terminal.id); - }); - - // Hook up terminal events to the global event emitter. - this.bufferer.startBuffering(terminal.id, terminal.onProcessData); - terminal.onProcessExit((exitCode) => { - logger.debug('Terminal exited', field('id', terminal.id), field('code', exitCode)); - this._onProcessExit.fire({ id: terminal.id, event: exitCode }); - }); - terminal.onProcessReady((event) => { - this._onProcessReady.fire({ id: terminal.id, event }); - }); - terminal.onProcessTitleChanged((title) => { - this._onProcessTitleChanged.fire({ id: terminal.id, event: title }); - }); + const persistentTerminalId = await this.ptyService.createProcess( + shellLaunchConfig, + shellLaunchConfig.cwd, + args.cols, + args.rows, + env, + process.env as platform.IProcessEnvironment, // Environment used for findExecutable + false, // windowsEnableConpty + args.shouldPersistTerminal, + args.workspaceId, + args.workspaceName, + ); return { - persistentTerminalId: terminal.id, - resolvedShellLaunchConfig, + persistentTerminalId, + resolvedShellLaunchConfig: shellLaunchConfig, }; } - private getTerminal(id: number): Terminal { - const terminal = this.terminals.get(id); - if (!terminal) { - throw new Error(`terminal with id ${id} does not exist`); - } - return terminal; - } - - private async attachToProcess(_id: number): Promise { - // TODO: Won't be necessary until we have persistent terminals. - throw new Error('not implemented'); - } - - private async start(id: number): Promise { - return this.getTerminal(id).start(); - } - - private async input(id: number, data: string): Promise { - return this.getTerminal(id).input(data); - } - - private async acknowledgeDataEvent(id: number, charCount: number): Promise { - return this.getTerminal(id).acknowledgeDataEvent(charCount); - } - - private async shutdown(id: number, immediate: boolean): Promise { - return this.getTerminal(id).shutdown(immediate); - } - - private async resize(id: number, cols: number, rows: number): Promise { - return this.getTerminal(id).resize(cols, rows); - } - - private async getInitialCwd(id: number): Promise { - return this.getTerminal(id).getInitialCwd(); - } - - private async getCwd(id: number): Promise { - return this.getTerminal(id).getCwd(); - } - private async sendCommandResult(_id: number, _reqId: number, _isError: boolean, _payload: any): Promise { // NOTE: Not required unless we implement the matching event, see above. throw new Error('not implemented'); } - - private async orphanQuestionReply(_id: number): Promise { - // NOTE: Not required unless we implement the matching event, see above. - throw new Error('not implemented'); - } - - private async reduceConnectionGraceTime(): Promise { - // NOTE: Not required unless we implement orphan terminals, see above. - // Returning instead of throwing error as VSCode expects this function - // to always succeed and throwing an error causes the terminal to crash. - return; - } - - private async listProcesses(): Promise { - const terminals = await Promise.all(Array.from(this.terminals).map(async ([id, terminal]) => { - return terminal.description(id); - })); - - // Only returned orphaned terminals so we don't end up attaching to - // terminals already attached elsewhere. - return terminals.filter((t) => t.isOrphan); - } - - public async setTerminalLayoutInfo(args: ISetTerminalLayoutInfoArgs): Promise { - this.layouts.set(args.workspaceId, args); - } - - public async getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise { - const layout = this.layouts.get(args.workspaceId); - if (!layout) { - return undefined; - } - - const tabs = await Promise.all(layout.tabs.map(async (tab) => { - // The terminals are stored by ID so look them up. - const terminals = await Promise.all(tab.terminals.map(async (t) => { - const terminal = this.terminals.get(t.terminal); - if (!terminal) { - return undefined; - } - return { - ...t, - terminal: await terminal.description(t.terminal), - }; - })); - - return { - ...tab, - // Filter out terminals that have been killed. - terminals: terminals.filter(isDefined), - }; - })); - - return { tabs }; - } - - async getShellEnvironment(): Promise { - return { ...process.env }; - } - - async getDefaultSystemShell(osOverride: platform.OperatingSystem = platform.OS): Promise { - return getSystemShell(osOverride, process.env); - } } function transformIncoming(remoteAuthority: string, uri: UriComponents | undefined): URI | undefined { const transformer = getUriTransformer(remoteAuthority); return uri ? URI.revive(transformer.transformIncoming(uri)) : uri; } - -function isDefined(t: T | undefined): t is T { - return typeof t !== 'undefined'; -} diff --git a/lib/vscode/src/vs/server/node/server.ts b/lib/vscode/src/vs/server/node/server.ts index a3708045..78891437 100644 --- a/lib/vscode/src/vs/server/node/server.ts +++ b/lib/vscode/src/vs/server/node/server.ts @@ -57,6 +57,7 @@ import { getUriTransformer } from 'vs/server/node/util'; import { REMOTE_TERMINAL_CHANNEL_NAME } from 'vs/workbench/contrib/terminal/common/remoteTerminalChannel'; import { REMOTE_FILE_SYSTEM_CHANNEL_NAME } from 'vs/workbench/services/remote/common/remoteAgentFileSystemChannel'; import { RemoteExtensionLogFileName } from 'vs/workbench/services/remote/common/remoteAgentService'; +import { PtyHostService } from 'vs/platform/terminal/node/ptyHostService'; const commit = product.commit || 'development'; @@ -292,7 +293,10 @@ export class Vscode { this.ipc.registerChannel('telemetry', new TelemetryChannel(telemetryService)); this.ipc.registerChannel('localizations', >ProxyChannel.fromService(accessor.get(ILocalizationsService))); this.ipc.registerChannel(REMOTE_FILE_SYSTEM_CHANNEL_NAME, new FileProviderChannel(environmentService, logService)); - this.ipc.registerChannel(REMOTE_TERMINAL_CHANNEL_NAME, new TerminalProviderChannel(logService)); + + const ptyHostService = new PtyHostService(logService, telemetryService); + this.ipc.registerChannel(REMOTE_TERMINAL_CHANNEL_NAME, new TerminalProviderChannel(logService, ptyHostService)); + resolve(new ErrorTelemetry(telemetryService)); }); });