From 35cee02c9fa9a109c4d75c0f686793f903864ae5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 10 Jul 2018 16:56:31 -0700 Subject: [PATCH 01/14] Move terminalProcess to run in renderer process Part of #53974 --- .../terminalProcessManager.ts | 153 ++++---- .../parts/terminal/node/terminalProcess.ts | 332 ++++++++++-------- 2 files changed, 276 insertions(+), 209 deletions(-) diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 64a26d1017cd6..1baae6f64c2f2 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as cp from 'child_process'; +// import * as cp from 'child_process'; import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import Uri from 'vs/base/common/uri'; @@ -15,9 +15,10 @@ import { Emitter, Event } from 'vs/base/common/event'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; -import { ITerminalChildProcess, IMessageFromTerminalProcess } from 'vs/workbench/parts/terminal/node/terminal'; -import { TerminalProcessExtHostProxy } from 'vs/workbench/parts/terminal/node/terminalProcessExtHostProxy'; +// import { ITerminalChildProcess, IMessageFromTerminalProcess } from 'vs/workbench/parts/terminal/node/terminal'; +// import { TerminalProcessExtHostProxy } from 'vs/workbench/parts/terminal/node/terminalProcessExtHostProxy'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; /** The amount of time to consider terminal errors to be related to the launch */ const LAUNCHING_DURATION = 500; @@ -36,7 +37,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { public shellProcessId: number; public initialCwd: string; - private _process: ITerminalChildProcess; + private _process: TerminalProcess; private _preLaunchInputQueue: string[] = []; private _disposables: IDisposable[] = []; @@ -58,6 +59,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private _logService: ILogService ) { + console.log(this._terminalId, this._instantiationService); this.ptyProcessReady = new TPromise(c => { this.onProcessReady(() => { this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); @@ -68,12 +70,13 @@ export class TerminalProcessManager implements ITerminalProcessManager { public dispose(): void { if (this._process) { - if (this._process.connected) { + if (this._process.isConnected) { // If the process was still connected this dispose came from // within VS Code, not the process, so mark the process as // killed by the user. this.processState = ProcessState.KILLED_BY_USER; - this._process.send({ event: 'shutdown' }); + this._process.shutdown(); + // this._process.send({ event: 'shutdown' }); } this._process = null; } @@ -90,42 +93,60 @@ export class TerminalProcessManager implements ITerminalProcessManager { cols: number, rows: number ): void { - const extensionHostOwned = (this._configHelper.config).extHostProcess; - if (extensionHostOwned) { - this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); - } else { - const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; - if (!shellLaunchConfig.executable) { - this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); - } - - const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); - this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); + // const extensionHostOwned = (this._configHelper.config).extHostProcess; + // if (extensionHostOwned) { + // this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); + // } else { + const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; + if (!shellLaunchConfig.executable) { + this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); + } - // Resolve env vars from config and shell - const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); - const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); - const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); - const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - shellLaunchConfig.env = envFromShell; + const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); + this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); + + // Resolve env vars from config and shell + const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); + const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); + const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); + const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); + shellLaunchConfig.env = envFromShell; + + // Merge process env with the env from config + const parentEnv = { ...process.env }; + terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); + + // Continue env initialization, merging in the env from the launch + // config and adding keys that are needed to create the process + const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); + const cwd = Uri.parse(require.toUrl('../node')).fsPath; + const options = { env, cwd }; + this._logService.debug(`Terminal process launching`, options); + + // this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); + this._process = new TerminalProcess(env['PTYSHELL'], [], env['PTYCWD'], cols, rows); + // } + this.processState = ProcessState.LAUNCHING; - // Merge process env with the env from config - const parentEnv = { ...process.env }; - terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); + this._process.onData(data => { + this._onProcessData.fire(data); + }); - // Continue env initialization, merging in the env from the launch - // config and adding keys that are needed to create the process - const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); - const cwd = Uri.parse(require.toUrl('../node')).fsPath; - const options = { env, cwd }; - this._logService.debug(`Terminal process launching`, options); + this._process.onProcessIdReady(pid => { + this.shellProcessId = pid; + this._onProcessReady.fire(); - this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); - } - this.processState = ProcessState.LAUNCHING; + // Send any queued data that's waiting + if (this._preLaunchInputQueue.length > 0) { + this._process.input(this._preLaunchInputQueue.join('')); + this._preLaunchInputQueue.length = 0; + } + }); - this._process.on('message', message => this._onMessage(message)); - this._process.on('exit', exitCode => this._onExit(exitCode)); + this._process.onTitleChanged(title => this._onProcessTitle.fire(title)); + this._process.onExit(exitCode => this._onExit(exitCode)); + // this._process.on('message', message => this._onMessage(message)); + // this._process.on('exit', exitCode => this._onExit(exitCode)); setTimeout(() => { if (this.processState === ProcessState.LAUNCHING) { @@ -135,10 +156,11 @@ export class TerminalProcessManager implements ITerminalProcessManager { } public setDimensions(cols: number, rows: number): void { - if (this._process && this._process.connected) { + if (this._process && this._process.isConnected) { // The child process could aready be terminated try { - this._process.send({ event: 'resize', cols, rows }); + this._process.resize(cols, rows); + // this._process.send({ event: 'resize', cols, rows }); } catch (error) { // We tried to write to a closed pipe / channel. if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { @@ -152,10 +174,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { if (this.shellProcessId) { if (this._process) { // Send data if the pty is ready - this._process.send({ - event: 'input', - data - }); + this._process.input(data); } } else { // If the pty is not ready, queue the data received to send later @@ -163,30 +182,30 @@ export class TerminalProcessManager implements ITerminalProcessManager { } } - private _onMessage(message: IMessageFromTerminalProcess): void { - this._logService.trace(`terminalProcessManager#_onMessage (shellProcessId: ${this.shellProcessId}`, message); - switch (message.type) { - case 'data': - this._onProcessData.fire(message.content); - break; - case 'pid': - this.shellProcessId = message.content; - this._onProcessReady.fire(); - - // Send any queued data that's waiting - if (this._preLaunchInputQueue.length > 0) { - this._process.send({ - event: 'input', - data: this._preLaunchInputQueue.join('') - }); - this._preLaunchInputQueue.length = 0; - } - break; - case 'title': - this._onProcessTitle.fire(message.content); - break; - } - } + // private _onMessage(message: IMessageFromTerminalProcess): void { + // this._logService.trace(`terminalProcessManager#_onMessage (shellProcessId: ${this.shellProcessId}`, message); + // switch (message.type) { + // case 'data': + // this._onProcessData.fire(message.content); + // break; + // case 'pid': + // this.shellProcessId = message.content; + // this._onProcessReady.fire(); + + // // Send any queued data that's waiting + // if (this._preLaunchInputQueue.length > 0) { + // this._process.send({ + // event: 'input', + // data: this._preLaunchInputQueue.join('') + // }); + // this._preLaunchInputQueue.length = 0; + // } + // break; + // case 'title': + // this._onProcessTitle.fire(message.content); + // break; + // } + // } private _onExit(exitCode: number): void { this._process = null; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 8987b35804958..f5d27ed7dcf22 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -6,162 +6,210 @@ import * as os from 'os'; import * as path from 'path'; import * as pty from 'node-pty'; +import { Event, Emitter } from 'vs/base/common/event'; + +export class TerminalProcess { + private _exitCode: number; + private _closeTimeout: number; + private _ptyProcess: pty.IPty; + private _currentTitle: string = ''; + + private readonly _onData: Emitter = new Emitter(); + public get onData(): Event { return this._onData.event; } + private readonly _onExit: Emitter = new Emitter(); + public get onExit(): Event { return this._onExit.event; } + private readonly _onProcessIdReady: Emitter = new Emitter(); + public get onProcessIdReady(): Event { return this._onProcessIdReady.event; } + private readonly _onTitleChanged: Emitter = new Emitter(); + public get onTitleChanged(): Event { return this._onTitleChanged.event; } + + constructor(shell: string, args: string | string[], cwd: string, cols: number, rows: number) { + // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, + // see https://github.com/electron/electron/issues/38 + let shellName: string; + if (os.platform() === 'win32') { + shellName = path.basename(process.env.PTYSHELL); + } else { + // Using 'xterm-256color' here helps ensure that the majority of Linux distributions will use a + // color prompt as defined in the default ~/.bashrc file. + shellName = 'xterm-256color'; + } + // const shell = process.env.PTYSHELL; + // const args = getArgs(); + // const cwd = process.env.PTYCWD; + // const cols = process.env.PTYCOLS; + // const rows = process.env.PTYROWS; + // let currentTitle = ''; + + // setupPlanB(Number(process.env.PTYPID)); + cleanEnv(); + + interface IOptions { + name: string; + cwd: string; + cols?: number; + rows?: number; + } -// The pty process needs to be run in its own child process to get around maxing out CPU on Mac, -// see https://github.com/electron/electron/issues/38 -let shellName: string; -if (os.platform() === 'win32') { - shellName = path.basename(process.env.PTYSHELL); -} else { - // Using 'xterm-256color' here helps ensure that the majority of Linux distributions will use a - // color prompt as defined in the default ~/.bashrc file. - shellName = 'xterm-256color'; -} -const shell = process.env.PTYSHELL; -const args = getArgs(); -const cwd = process.env.PTYCWD; -const cols = process.env.PTYCOLS; -const rows = process.env.PTYROWS; -let currentTitle = ''; - -setupPlanB(Number(process.env.PTYPID)); -cleanEnv(); - -interface IOptions { - name: string; - cwd: string; - cols?: number; - rows?: number; -} - -const options: IOptions = { - name: shellName, - cwd -}; -if (cols && rows) { - options.cols = parseInt(cols, 10); - options.rows = parseInt(rows, 10); -} - -const ptyProcess = pty.spawn(shell, args, options); + const options: IOptions = { + name: shellName, + cwd + }; + if (cols && rows) { + // options.cols = parseInt(cols, 10); + // options.rows = parseInt(rows, 10); + options.cols = cols; + options.rows = rows; + } -let closeTimeout: number; -let exitCode: number; + const ptyProcess = pty.spawn(shell, args, options); + this._ptyProcess = ptyProcess; + + // let closeTimeout: number; + // let exitCode: number; + + (ptyProcess).on('data-buffered', (data) => { + this._onData.fire(data); + // process.send({ + // type: 'data', + // content: data + // }); + if (this._closeTimeout) { + clearTimeout(this._closeTimeout); + this._queueProcessExit(); + } + }); + + ptyProcess.on('exit', (code) => { + this._exitCode = code; + this._queueProcessExit(); + }); + + // process.on('message', (message) => { + // if (message.event === 'input') { + // ptyProcess.write(message.data); + // } else if (message.event === 'resize') { + // // Ensure that cols and rows are always >= 1, this prevents a native + // // exception in winpty. + // ptyProcess.resize(Math.max(message.cols, 1), Math.max(message.rows, 1)); + // } else if (message.event === 'shutdown') { + // this._queueProcessExit(); + // } + // }); + + setTimeout(() => { + this._sendProcessId(); + }, 1000); + this._setupTitlePolling(); + + // function getArgs(): string | string[] { + // if (process.env['PTYSHELLCMDLINE']) { + // return process.env['PTYSHELLCMDLINE']; + // } + // const args = []; + // let i = 0; + // while (process.env['PTYSHELLARG' + i]) { + // args.push(process.env['PTYSHELLARG' + i]); + // i++; + // } + // return args; + // } + + function cleanEnv() { + const keys = [ + 'AMD_ENTRYPOINT', + 'ELECTRON_NO_ASAR', + 'ELECTRON_RUN_AS_NODE', + 'GOOGLE_API_KEY', + 'PTYCWD', + 'PTYPID', + 'PTYSHELL', + 'PTYCOLS', + 'PTYROWS', + 'PTYSHELLCMDLINE', + 'VSCODE_LOGS', + 'VSCODE_PORTABLE', + 'VSCODE_PID', + ]; + // TODO: Don't change process.env, create a new one + keys.forEach(function (key) { + if (process.env[key]) { + delete process.env[key]; + } + }); + let i = 0; + while (process.env['PTYSHELLARG' + i]) { + delete process.env['PTYSHELLARG' + i]; + i++; + } + } -// Allow any trailing data events to be sent before the exit event is sent. -// See https://github.com/Tyriar/node-pty/issues/72 -function queueProcessExit() { - if (closeTimeout) { - clearTimeout(closeTimeout); + // function setupPlanB(parentPid: number) { + // setInterval(function () { + // try { + // process.kill(parentPid, 0); // throws an exception if the main process doesn't exist anymore. + // } catch (e) { + // process.exit(); + // } + // }, 5000); + // } } - closeTimeout = setTimeout(function () { - ptyProcess.kill(); - process.exit(exitCode); - }, 250); -} -ptyProcess.on('data', function (data) { - process.send({ - type: 'data', - content: data - }); - if (closeTimeout) { - clearTimeout(closeTimeout); - queueProcessExit(); + private _setupTitlePolling() { + this._sendProcessTitle(); + setInterval(() => { + if (this._currentTitle !== this._ptyProcess.process) { + this._sendProcessTitle(); + } + }, 200); } -}); -ptyProcess.on('exit', function (code) { - exitCode = code; - queueProcessExit(); -}); - -process.on('message', function (message) { - if (message.event === 'input') { - ptyProcess.write(message.data); - } else if (message.event === 'resize') { - // Ensure that cols and rows are always >= 1, this prevents a native - // exception in winpty. - ptyProcess.resize(Math.max(message.cols, 1), Math.max(message.rows, 1)); - } else if (message.event === 'shutdown') { - queueProcessExit(); + // Allow any trailing data events to be sent before the exit event is sent. + // See https://github.com/Tyriar/node-pty/issues/72 + private _queueProcessExit() { + if (this._closeTimeout) { + clearTimeout(this._closeTimeout); + } + // TODO: Dispose correctly + this._closeTimeout = setTimeout(() => { + this._ptyProcess.kill(); + this._onExit.fire(this._exitCode); + // process.exit(exitCode); + }, 250); } -}); - -sendProcessId(); -setupTitlePolling(); -function getArgs(): string | string[] { - if (process.env['PTYSHELLCMDLINE']) { - return process.env['PTYSHELLCMDLINE']; + private _sendProcessId() { + this._onProcessIdReady.fire(this._ptyProcess.pid); + // process.send({ + // type: 'pid', + // content: ptyProcess.pid + // }); } - const args = []; - let i = 0; - while (process.env['PTYSHELLARG' + i]) { - args.push(process.env['PTYSHELLARG' + i]); - i++; + private _sendProcessTitle(): void { + // process.send({ + // type: 'title', + // content: ptyProcess.process + // }); + this._currentTitle = this._ptyProcess.process; + this._onTitleChanged.fire(this._currentTitle); } - return args; -} -function cleanEnv() { - const keys = [ - 'AMD_ENTRYPOINT', - 'ELECTRON_NO_ASAR', - 'ELECTRON_RUN_AS_NODE', - 'GOOGLE_API_KEY', - 'PTYCWD', - 'PTYPID', - 'PTYSHELL', - 'PTYCOLS', - 'PTYROWS', - 'PTYSHELLCMDLINE', - 'VSCODE_LOGS', - 'VSCODE_PORTABLE', - 'VSCODE_PID', - ]; - keys.forEach(function (key) { - if (process.env[key]) { - delete process.env[key]; - } - }); - let i = 0; - while (process.env['PTYSHELLARG' + i]) { - delete process.env['PTYSHELLARG' + i]; - i++; + public shutdown(): void { + this._queueProcessExit(); } -} -function setupPlanB(parentPid: number) { - setInterval(function () { - try { - process.kill(parentPid, 0); // throws an exception if the main process doesn't exist anymore. - } catch (e) { - process.exit(); - } - }, 5000); -} - -function sendProcessId() { - process.send({ - type: 'pid', - content: ptyProcess.pid - }); -} + public input(data: string): void { + this._ptyProcess.write(data); + } -function setupTitlePolling() { - sendProcessTitle(); - setInterval(function () { - if (currentTitle !== ptyProcess.process) { - sendProcessTitle(); - } - }, 200); -} + public resize(cols: number, rows: number): void { + // Ensure that cols and rows are always >= 1, this prevents a native + // exception in winpty. + this._ptyProcess.resize(Math.max(cols, 1), Math.max(rows, 1)); + } -function sendProcessTitle() { - process.send({ - type: 'title', - content: ptyProcess.process - }); - currentTitle = ptyProcess.process; + public get isConnected(): boolean { + // Don't need connected anymore as it's the same process + return true; + } } From 1cf4ab897855173b047c02cf42ee1be46ec85dac Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 10 Jul 2018 18:31:04 -0700 Subject: [PATCH 02/14] Don't build terminalProcess module --- build/gulpfile.vscode.js | 2 +- src/vs/workbench/buildfile.js | 2 +- src/vs/workbench/parts/terminal/node/terminalProcess.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build/gulpfile.vscode.js b/build/gulpfile.vscode.js index 4831c70e07bfb..f55dfcdfa0cab 100644 --- a/build/gulpfile.vscode.js +++ b/build/gulpfile.vscode.js @@ -78,7 +78,7 @@ const vscodeResources = [ 'out-build/vs/workbench/parts/webview/electron-browser/webview-pre.js', 'out-build/vs/**/markdown.css', 'out-build/vs/workbench/parts/tasks/**/*.json', - 'out-build/vs/workbench/parts/terminal/electron-browser/terminalProcess.js', + // 'out-build/vs/workbench/parts/terminal/electron-browser/terminalProcess.js', 'out-build/vs/workbench/parts/welcome/walkThrough/**/*.md', 'out-build/vs/workbench/services/files/**/*.exe', 'out-build/vs/workbench/services/files/**/*.md', diff --git a/src/vs/workbench/buildfile.js b/src/vs/workbench/buildfile.js index 4b83a39ab1020..281c194b60b4e 100644 --- a/src/vs/workbench/buildfile.js +++ b/src/vs/workbench/buildfile.js @@ -28,7 +28,7 @@ exports.collectModules = function () { createModuleDescription('vs/workbench/node/extensionHostProcess', []), - createModuleDescription('vs/workbench/parts/terminal/node/terminalProcess', []) + // createModuleDescription('vs/workbench/parts/terminal/node/terminalProcess', []) ]; return modules; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index f5d27ed7dcf22..e2bc28313cae1 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -68,7 +68,7 @@ export class TerminalProcess { // let closeTimeout: number; // let exitCode: number; - (ptyProcess).on('data-buffered', (data) => { + (ptyProcess).on('data', (data) => { this._onData.fire(data); // process.send({ // type: 'data', From 6726ef4ea95e4e17b79c5f737aa7453c4933df49 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 10 Jul 2018 21:56:19 -0700 Subject: [PATCH 03/14] Tidy up terminalProcess --- .../parts/terminal/node/terminalProcess.ts | 146 +++++------------- 1 file changed, 38 insertions(+), 108 deletions(-) diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index e2bc28313cae1..5404e59b07da0 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -7,6 +7,7 @@ import * as os from 'os'; import * as path from 'path'; import * as pty from 'node-pty'; import { Event, Emitter } from 'vs/base/common/event'; +import { IProcessEnvironment } from 'vs/base/common/platform'; export class TerminalProcess { private _exitCode: number; @@ -34,125 +35,63 @@ export class TerminalProcess { // color prompt as defined in the default ~/.bashrc file. shellName = 'xterm-256color'; } - // const shell = process.env.PTYSHELL; - // const args = getArgs(); - // const cwd = process.env.PTYCWD; - // const cols = process.env.PTYCOLS; - // const rows = process.env.PTYROWS; - // let currentTitle = ''; - // setupPlanB(Number(process.env.PTYPID)); - cleanEnv(); - - interface IOptions { - name: string; - cwd: string; - cols?: number; - rows?: number; - } - - const options: IOptions = { + const options: pty.IPtyForkOptions = { name: shellName, - cwd + cwd, + env: this._createEnv(), + cols, + rows }; - if (cols && rows) { - // options.cols = parseInt(cols, 10); - // options.rows = parseInt(rows, 10); - options.cols = cols; - options.rows = rows; - } - const ptyProcess = pty.spawn(shell, args, options); - this._ptyProcess = ptyProcess; - - // let closeTimeout: number; - // let exitCode: number; - - (ptyProcess).on('data', (data) => { + this._ptyProcess = pty.spawn(shell, args, options); + this._ptyProcess.on('data', (data) => { this._onData.fire(data); - // process.send({ - // type: 'data', - // content: data - // }); if (this._closeTimeout) { clearTimeout(this._closeTimeout); this._queueProcessExit(); } }); - - ptyProcess.on('exit', (code) => { + this._ptyProcess.on('exit', (code) => { this._exitCode = code; this._queueProcessExit(); }); - // process.on('message', (message) => { - // if (message.event === 'input') { - // ptyProcess.write(message.data); - // } else if (message.event === 'resize') { - // // Ensure that cols and rows are always >= 1, this prevents a native - // // exception in winpty. - // ptyProcess.resize(Math.max(message.cols, 1), Math.max(message.rows, 1)); - // } else if (message.event === 'shutdown') { - // this._queueProcessExit(); - // } - // }); - + // TODO: We should no longer need to delay this since spawn is sync setTimeout(() => { this._sendProcessId(); }, 1000); this._setupTitlePolling(); + } - // function getArgs(): string | string[] { - // if (process.env['PTYSHELLCMDLINE']) { - // return process.env['PTYSHELLCMDLINE']; - // } - // const args = []; - // let i = 0; - // while (process.env['PTYSHELLARG' + i]) { - // args.push(process.env['PTYSHELLARG' + i]); - // i++; - // } - // return args; - // } - - function cleanEnv() { - const keys = [ - 'AMD_ENTRYPOINT', - 'ELECTRON_NO_ASAR', - 'ELECTRON_RUN_AS_NODE', - 'GOOGLE_API_KEY', - 'PTYCWD', - 'PTYPID', - 'PTYSHELL', - 'PTYCOLS', - 'PTYROWS', - 'PTYSHELLCMDLINE', - 'VSCODE_LOGS', - 'VSCODE_PORTABLE', - 'VSCODE_PID', - ]; - // TODO: Don't change process.env, create a new one - keys.forEach(function (key) { - if (process.env[key]) { - delete process.env[key]; - } - }); - let i = 0; - while (process.env['PTYSHELLARG' + i]) { - delete process.env['PTYSHELLARG' + i]; - i++; + private _createEnv(): IProcessEnvironment { + const env: IProcessEnvironment = { ...process.env }; + const keysToRemove = [ + 'AMD_ENTRYPOINT', + 'ELECTRON_ENABLE_STACK_DUMPING', + 'ELECTRON_ENABLE_LOGGING', + 'ELECTRON_NO_ASAR', + 'ELECTRON_RUN_AS_NODE', + 'GOOGLE_API_KEY', + 'VSCODE_CLI', + 'VSCODE_DEV', + 'VSCODE_IPC_HOOK', + 'VSCODE_LOGS', + 'VSCODE_NLS_CONFIG', + 'VSCODE_PORTABLE', + 'VSCODE_PID', + ]; + keysToRemove.forEach((key) => { + if (env[key]) { + delete env[key]; } - } - - // function setupPlanB(parentPid: number) { - // setInterval(function () { - // try { - // process.kill(parentPid, 0); // throws an exception if the main process doesn't exist anymore. - // } catch (e) { - // process.exit(); - // } - // }, 5000); - // } + }); + Object.keys(env).forEach(key => { + if (key.search(/^VSCODE_NODE_CACHED_DATA_DIR_\d+$/) === 0) { + delete env[key]; + } + }); + return env; } private _setupTitlePolling() { @@ -174,22 +113,13 @@ export class TerminalProcess { this._closeTimeout = setTimeout(() => { this._ptyProcess.kill(); this._onExit.fire(this._exitCode); - // process.exit(exitCode); }, 250); } private _sendProcessId() { this._onProcessIdReady.fire(this._ptyProcess.pid); - // process.send({ - // type: 'pid', - // content: ptyProcess.pid - // }); } private _sendProcessTitle(): void { - // process.send({ - // type: 'title', - // content: ptyProcess.process - // }); this._currentTitle = this._ptyProcess.process; this._onTitleChanged.fire(this._currentTitle); } From 2fb8ab512455980a89bd1eb03fd34f365ebe2de7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 10 Jul 2018 23:03:43 -0700 Subject: [PATCH 04/14] Fix most of process proxy --- .../mainThreadTerminalService.ts | 2 +- .../api/node/extHostTerminalService.ts | 44 ++++----- .../parts/terminal/common/terminal.ts | 6 +- .../terminalProcessManager.ts | 77 ++++++++-------- .../workbench/parts/terminal/node/terminal.ts | 26 +++--- .../terminal/node/terminalEnvironment.ts | 28 +++--- .../parts/terminal/node/terminalProcess.ts | 21 ++--- .../node/terminalProcessExtHostProxy.ts | 89 ++++++++++++------- 8 files changed, 158 insertions(+), 135 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index 242091da9b19d..ccf129ce11714 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -178,7 +178,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape }; this._proxy.$createProcess(request.proxy.terminalId, shellLaunchConfigDto, request.cols, request.rows); request.proxy.onInput(data => this._proxy.$acceptProcessInput(request.proxy.terminalId, data)); - request.proxy.onResize((cols, rows) => this._proxy.$acceptProcessResize(request.proxy.terminalId, cols, rows)); + request.proxy.onResize(dimensions => this._proxy.$acceptProcessResize(request.proxy.terminalId, dimensions.cols, dimensions.rows)); request.proxy.onShutdown(() => this._proxy.$acceptProcessShutdown(request.proxy.terminalId)); } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 956f724057e6c..282811368f981 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -5,17 +5,16 @@ 'use strict'; import * as vscode from 'vscode'; -import * as cp from 'child_process'; import * as os from 'os'; import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import Uri from 'vs/base/common/uri'; import { Event, Emitter } from 'vs/base/common/event'; import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto } from 'vs/workbench/api/node/extHost.protocol'; -import { IMessageFromTerminalProcess } from 'vs/workbench/parts/terminal/node/terminal'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; import { ILogService } from 'vs/platform/log/common/log'; import { EXT_HOST_CREATION_DELAY } from 'vs/workbench/parts/terminal/common/terminal'; +import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; const RENDERER_NO_PROCESS_ID = -1; @@ -226,7 +225,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { private _proxy: MainThreadTerminalServiceShape; private _activeTerminal: ExtHostTerminal; private _terminals: ExtHostTerminal[] = []; - private _terminalProcesses: { [id: number]: cp.ChildProcess } = {}; + private _terminalProcesses: { [id: number]: TerminalProcess } = {}; private _terminalRenderers: ExtHostTerminalRenderer[] = []; public get activeTerminal(): ExtHostTerminal { return this._activeTerminal; } @@ -397,27 +396,29 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // Fork the process and listen for messages this._logService.debug(`Terminal process launching on ext host`, options); - this._terminalProcesses[id] = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); - this._terminalProcesses[id].on('message', (message: IMessageFromTerminalProcess) => { - switch (message.type) { - case 'pid': this._proxy.$sendProcessPid(id, message.content); break; - case 'title': this._proxy.$sendProcessTitle(id, message.content); break; - case 'data': this._proxy.$sendProcessData(id, message.content); break; - } - }); - this._terminalProcesses[id].on('exit', (exitCode) => this._onProcessExit(id, exitCode)); + this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, cwd, cols, rows); + // this._terminalProcesses[id].on + // this._terminalProcesses[id].on('message', (message: IMessageFromTerminalProcess) => { + // switch (message.type) { + // }); + this._terminalProcesses[id].onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid)); + this._terminalProcesses[id].onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); + this._terminalProcesses[id].onProcessData(data => this._proxy.$sendProcessData(id, data)); + this._terminalProcesses[id].onProcessExit((exitCode) => this._onProcessExit(id, exitCode)); } public $acceptProcessInput(id: number, data: string): void { - if (this._terminalProcesses[id].connected) { - this._terminalProcesses[id].send({ event: 'input', data }); + if (this._terminalProcesses[id].isConnected) { + this._terminalProcesses[id].input(data); + // this._terminalProcesses[id].send({ event: 'input', data }); } } public $acceptProcessResize(id: number, cols: number, rows: number): void { - if (this._terminalProcesses[id].connected) { + if (this._terminalProcesses[id].isConnected) { try { - this._terminalProcesses[id].send({ event: 'resize', cols, rows }); + this._terminalProcesses[id].resize(cols, rows); + // this._terminalProcesses[id].send({ event: 'resize', cols, rows }); } catch (error) { // We tried to write to a closed pipe / channel. if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { @@ -428,16 +429,17 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } public $acceptProcessShutdown(id: number): void { - if (this._terminalProcesses[id].connected) { - this._terminalProcesses[id].send({ event: 'shutdown' }); + if (this._terminalProcesses[id].isConnected) { + this._terminalProcesses[id].shutdown(); + // this._terminalProcesses[id].send({ event: 'shutdown' }); } } private _onProcessExit(id: number, exitCode: number): void { // Remove listeners - const process = this._terminalProcesses[id]; - process.removeAllListeners('message'); - process.removeAllListeners('exit'); + // const process = this._terminalProcesses[id]; + // process.removeAllListeners('message'); + // process.removeAllListeners('exit'); // Remove process reference delete this._terminalProcesses[id]; diff --git a/src/vs/workbench/parts/terminal/common/terminal.ts b/src/vs/workbench/parts/terminal/common/terminal.ts index 81613ecef7bc3..ffcd27e8f4672 100644 --- a/src/vs/workbench/parts/terminal/common/terminal.ts +++ b/src/vs/workbench/parts/terminal/common/terminal.ts @@ -596,9 +596,9 @@ export interface ITerminalProcessExtHostProxy extends IDisposable { emitPid(pid: number): void; emitExit(exitCode: number): void; - onInput(listener: (data: string) => void): void; - onResize(listener: (cols: number, rows: number) => void): void; - onShutdown(listener: () => void): void; + onInput: Event; + onResize: Event<{ cols: number, rows: number }>; + onShutdown: Event; } export interface ITerminalProcessExtHostRequest { diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 1baae6f64c2f2..6cb1d68cb0c92 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -15,8 +15,8 @@ import { Emitter, Event } from 'vs/base/common/event'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; -// import { ITerminalChildProcess, IMessageFromTerminalProcess } from 'vs/workbench/parts/terminal/node/terminal'; -// import { TerminalProcessExtHostProxy } from 'vs/workbench/parts/terminal/node/terminalProcessExtHostProxy'; +import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; +import { TerminalProcessExtHostProxy } from 'vs/workbench/parts/terminal/node/terminalProcessExtHostProxy'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; @@ -37,7 +37,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { public shellProcessId: number; public initialCwd: string; - private _process: TerminalProcess; + private _process: ITerminalChildProcess; private _preLaunchInputQueue: string[] = []; private _disposables: IDisposable[] = []; @@ -59,7 +59,6 @@ export class TerminalProcessManager implements ITerminalProcessManager { @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private _logService: ILogService ) { - console.log(this._terminalId, this._instantiationService); this.ptyProcessReady = new TPromise(c => { this.onProcessReady(() => { this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); @@ -93,42 +92,42 @@ export class TerminalProcessManager implements ITerminalProcessManager { cols: number, rows: number ): void { - // const extensionHostOwned = (this._configHelper.config).extHostProcess; - // if (extensionHostOwned) { - // this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); - // } else { - const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; - if (!shellLaunchConfig.executable) { - this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); - } + const extensionHostOwned = (this._configHelper.config).extHostProcess; + if (extensionHostOwned) { + this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); + } else { + const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; + if (!shellLaunchConfig.executable) { + this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); + } - const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); - this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); - - // Resolve env vars from config and shell - const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); - const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); - const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); - const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - shellLaunchConfig.env = envFromShell; - - // Merge process env with the env from config - const parentEnv = { ...process.env }; - terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); - - // Continue env initialization, merging in the env from the launch - // config and adding keys that are needed to create the process - const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); - const cwd = Uri.parse(require.toUrl('../node')).fsPath; - const options = { env, cwd }; - this._logService.debug(`Terminal process launching`, options); - - // this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); - this._process = new TerminalProcess(env['PTYSHELL'], [], env['PTYCWD'], cols, rows); - // } + const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); + this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); + + // Resolve env vars from config and shell + const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); + const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); + const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); + const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); + shellLaunchConfig.env = envFromShell; + + // Merge process env with the env from config + const parentEnv = { ...process.env }; + terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); + + // Continue env initialization, merging in the env from the launch + // config and adding keys that are needed to create the process + const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); + const cwd = Uri.parse(require.toUrl('../node')).fsPath; + const options = { env, cwd }; + this._logService.debug(`Terminal process launching`, options); + + // this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); + this._process = new TerminalProcess(env['PTYSHELL'], [], env['PTYCWD'], cols, rows); + } this.processState = ProcessState.LAUNCHING; - this._process.onData(data => { + this._process.onProcessData(data => { this._onProcessData.fire(data); }); @@ -143,8 +142,8 @@ export class TerminalProcessManager implements ITerminalProcessManager { } }); - this._process.onTitleChanged(title => this._onProcessTitle.fire(title)); - this._process.onExit(exitCode => this._onExit(exitCode)); + this._process.onProcessTitleChanged(title => this._onProcessTitle.fire(title)); + this._process.onProcessExit(exitCode => this._onExit(exitCode)); // this._process.on('message', message => this._onMessage(message)); // this._process.on('exit', exitCode => this._onExit(exitCode)); diff --git a/src/vs/workbench/parts/terminal/node/terminal.ts b/src/vs/workbench/parts/terminal/node/terminal.ts index 8cb9bc9dddd25..d85830b8a0412 100644 --- a/src/vs/workbench/parts/terminal/node/terminal.ts +++ b/src/vs/workbench/parts/terminal/node/terminal.ts @@ -7,30 +7,24 @@ import * as os from 'os'; import * as platform from 'vs/base/common/platform'; import * as processes from 'vs/base/node/processes'; import { readFile, fileExists } from 'vs/base/node/pfs'; - -export interface IMessageFromTerminalProcess { - type: 'pid' | 'data' | 'title'; - content: number | string; -} - -export interface IMessageToTerminalProcess { - event: 'resize' | 'input' | 'shutdown'; - data?: string; - cols?: number; - rows?: number; -} +import { Event } from 'vs/base/common/event'; /** * An interface representing a raw terminal child process, this is a subset of the * child_process.ChildProcess node.js interface. */ export interface ITerminalChildProcess { - readonly connected: boolean; + // TODO: Remove connected and references to it + readonly isConnected: boolean; - send(message: IMessageToTerminalProcess): boolean; + onProcessData: Event; + onProcessExit: Event; + onProcessIdReady: Event; + onProcessTitleChanged: Event; - on(event: 'exit', listener: (code: number) => void): this; - on(event: 'message', listener: (message: IMessageFromTerminalProcess) => void): this; + shutdown(): void; + input(data: string): void; + resize(cols: number, rows: number): void; } let _TERMINAL_DEFAULT_SHELL_UNIX_LIKE: string = null; diff --git a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts index 8e2bca6daddce..e54909e939a0f 100644 --- a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts +++ b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts @@ -58,23 +58,23 @@ export function createTerminalEnv(parentEnv: IStringDictionary, shell: I mergeEnvironments(env, shell.env); } - env['PTYPID'] = process.pid.toString(); - env['PTYSHELL'] = shell.executable; + // env['PTYPID'] = process.pid.toString(); + // env['PTYSHELL'] = shell.executable; env['TERM_PROGRAM'] = 'vscode'; env['TERM_PROGRAM_VERSION'] = pkg.version; - if (shell.args) { - if (typeof shell.args === 'string') { - env[`PTYSHELLCMDLINE`] = shell.args; - } else { - shell.args.forEach((arg, i) => env[`PTYSHELLARG${i}`] = arg); - } - } - env['PTYCWD'] = cwd; + // if (shell.args) { + // if (typeof shell.args === 'string') { + // env[`PTYSHELLCMDLINE`] = shell.args; + // } else { + // shell.args.forEach((arg, i) => env[`PTYSHELLARG${i}`] = arg); + // } + // } + // env['PTYCWD'] = cwd; env['LANG'] = _getLangEnvVariable(locale); - if (cols && rows) { - env['PTYCOLS'] = cols.toString(); - env['PTYROWS'] = rows.toString(); - } + // if (cols && rows) { + // env['PTYCOLS'] = cols.toString(); + // env['PTYROWS'] = rows.toString(); + // } env['AMD_ENTRYPOINT'] = 'vs/workbench/parts/terminal/node/terminalProcess'; return env; } diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 5404e59b07da0..050de680e29e5 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -8,21 +8,22 @@ import * as path from 'path'; import * as pty from 'node-pty'; import { Event, Emitter } from 'vs/base/common/event'; import { IProcessEnvironment } from 'vs/base/common/platform'; +import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; -export class TerminalProcess { +export class TerminalProcess implements ITerminalChildProcess { private _exitCode: number; private _closeTimeout: number; private _ptyProcess: pty.IPty; private _currentTitle: string = ''; - private readonly _onData: Emitter = new Emitter(); - public get onData(): Event { return this._onData.event; } - private readonly _onExit: Emitter = new Emitter(); - public get onExit(): Event { return this._onExit.event; } + private readonly _onProcessData: Emitter = new Emitter(); + public get onProcessData(): Event { return this._onProcessData.event; } + private readonly _onProcessExit: Emitter = new Emitter(); + public get onProcessExit(): Event { return this._onProcessExit.event; } private readonly _onProcessIdReady: Emitter = new Emitter(); public get onProcessIdReady(): Event { return this._onProcessIdReady.event; } - private readonly _onTitleChanged: Emitter = new Emitter(); - public get onTitleChanged(): Event { return this._onTitleChanged.event; } + private readonly _onProcessTitleChanged: Emitter = new Emitter(); + public get onProcessTitleChanged(): Event { return this._onProcessTitleChanged.event; } constructor(shell: string, args: string | string[], cwd: string, cols: number, rows: number) { // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, @@ -46,7 +47,7 @@ export class TerminalProcess { this._ptyProcess = pty.spawn(shell, args, options); this._ptyProcess.on('data', (data) => { - this._onData.fire(data); + this._onProcessData.fire(data); if (this._closeTimeout) { clearTimeout(this._closeTimeout); this._queueProcessExit(); @@ -112,7 +113,7 @@ export class TerminalProcess { // TODO: Dispose correctly this._closeTimeout = setTimeout(() => { this._ptyProcess.kill(); - this._onExit.fire(this._exitCode); + this._onProcessExit.fire(this._exitCode); }, 250); } @@ -121,7 +122,7 @@ export class TerminalProcess { } private _sendProcessTitle(): void { this._currentTitle = this._ptyProcess.process; - this._onTitleChanged.fire(this._currentTitle); + this._onProcessTitleChanged.fire(this._currentTitle); } public shutdown(): void { diff --git a/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts b/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts index 9c842fbbfa8cd..422e7ae45c59a 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts @@ -3,17 +3,33 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ITerminalChildProcess, IMessageToTerminalProcess, IMessageFromTerminalProcess } from 'vs/workbench/parts/terminal/node/terminal'; -import { EventEmitter } from 'events'; +import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; +import { Event, Emitter } from 'vs/base/common/event'; import { ITerminalService, ITerminalProcessExtHostProxy, IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal'; -import { IDisposable, toDisposable } from 'vs/base/common/lifecycle'; +import { IDisposable } from 'vs/base/common/lifecycle'; -export class TerminalProcessExtHostProxy extends EventEmitter implements ITerminalChildProcess, ITerminalProcessExtHostProxy { +export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerminalProcessExtHostProxy { // For ext host processes connected checks happen on the ext host - public connected: boolean = true; + public isConnected: boolean = true; private _disposables: IDisposable[] = []; + private readonly _onProcessData: Emitter = new Emitter(); + public get onProcessData(): Event { return this._onProcessData.event; } + private readonly _onProcessExit: Emitter = new Emitter(); + public get onProcessExit(): Event { return this._onProcessExit.event; } + private readonly _onProcessIdReady: Emitter = new Emitter(); + public get onProcessIdReady(): Event { return this._onProcessIdReady.event; } + private readonly _onProcessTitleChanged: Emitter = new Emitter(); + public get onProcessTitleChanged(): Event { return this._onProcessTitleChanged.event; } + + private readonly _onInput: Emitter = new Emitter(); + public get onInput(): Event { return this._onInput.event; } + private readonly _onResize: Emitter<{ cols: number, rows: number }> = new Emitter<{ cols: number, rows: number }>(); + public get onResize(): Event<{ cols: number, rows: number }> { return this._onResize.event; } + private readonly _onShutdown: Emitter = new Emitter(); + public get onShutdown(): Event { return this._onShutdown.event; } + constructor( public terminalId: number, shellLaunchConfig: IShellLaunchConfig, @@ -21,8 +37,6 @@ export class TerminalProcessExtHostProxy extends EventEmitter implements ITermin rows: number, @ITerminalService private _terminalService: ITerminalService ) { - super(); - // TODO: Return TPromise indicating success? Teardown if failure? this._terminalService.requestExtHostProcess(this, shellLaunchConfig, cols, rows); } @@ -33,46 +47,59 @@ export class TerminalProcessExtHostProxy extends EventEmitter implements ITermin } public emitData(data: string): void { - this.emit('message', { type: 'data', content: data } as IMessageFromTerminalProcess); + this._onProcessData.fire(data); } public emitTitle(title: string): void { - this.emit('message', { type: 'title', content: title } as IMessageFromTerminalProcess); + this._onProcessTitleChanged.fire(title); } public emitPid(pid: number): void { - this.emit('message', { type: 'pid', content: pid } as IMessageFromTerminalProcess); + this._onProcessIdReady.fire(pid); } public emitExit(exitCode: number): void { - this.emit('exit', exitCode); + this._onProcessExit.fire(exitCode); this.dispose(); } - public send(message: IMessageToTerminalProcess): boolean { - switch (message.event) { - case 'input': this.emit('input', message.data); break; - case 'resize': this.emit('resize', message.cols, message.rows); break; - case 'shutdown': this.emit('shutdown'); break; - } - return true; - } + // public send(message: IMessageToTerminalProcess): boolean { + // switch (message.event) { + // case 'input': this.emit('input', message.data); break; + // case 'resize': this.emit('resize', message.cols, message.rows); break; + // case 'shutdown': this.emit('shutdown'); break; + // } + // return true; + // } - public onInput(listener: (data: string) => void): void { - const outerListener = (data) => listener(data); - this.on('input', outerListener); - this._disposables.push(toDisposable(() => this.removeListener('input', outerListener))); + public shutdown(): void { + this._onShutdown.fire(); } - public onResize(listener: (cols: number, rows: number) => void): void { - const outerListener = (cols, rows) => listener(cols, rows); - this.on('resize', outerListener); - this._disposables.push(toDisposable(() => this.removeListener('resize', outerListener))); + public input(data: string): void { + this._onInput.fire(data); } - public onShutdown(listener: () => void): void { - const outerListener = () => listener(); - this.on('shutdown', outerListener); - this._disposables.push(toDisposable(() => this.removeListener('shutdown', outerListener))); + public resize(cols: number, rows: number): void { + this._onResize.fire({ cols, rows }); } + + + // public onInput(listener: (data: string) => void): void { + // const outerListener = (data) => listener(data); + // this.on('input', outerListener); + // this._disposables.push(toDisposable(() => this.removeListener('input', outerListener))); + // } + + // public onResize(listener: (cols: number, rows: number) => void): void { + // const outerListener = (cols, rows) => listener(cols, rows); + // this.on('resize', outerListener); + // this._disposables.push(toDisposable(() => this.removeListener('resize', outerListener))); + // } + + // public onShutdown(listener: () => void): void { + // const outerListener = () => listener(); + // this.on('shutdown', outerListener); + // this._disposables.push(toDisposable(() => this.removeListener('shutdown', outerListener))); + // } } \ No newline at end of file From 84a7a188f8996c230016a7879d00782e580df293 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 11:35:50 -0700 Subject: [PATCH 05/14] Remove some comments --- build/gulpfile.vscode.js | 1 - src/vs/workbench/api/node/extHostTerminalService.ts | 11 +---------- .../electron-browser/terminalProcessManager.ts | 2 +- .../workbench/parts/terminal/node/terminalProcess.ts | 10 +++++++++- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/build/gulpfile.vscode.js b/build/gulpfile.vscode.js index f55dfcdfa0cab..71c9530041be5 100644 --- a/build/gulpfile.vscode.js +++ b/build/gulpfile.vscode.js @@ -78,7 +78,6 @@ const vscodeResources = [ 'out-build/vs/workbench/parts/webview/electron-browser/webview-pre.js', 'out-build/vs/**/markdown.css', 'out-build/vs/workbench/parts/tasks/**/*.json', - // 'out-build/vs/workbench/parts/terminal/electron-browser/terminalProcess.js', 'out-build/vs/workbench/parts/welcome/walkThrough/**/*.md', 'out-build/vs/workbench/services/files/**/*.exe', 'out-build/vs/workbench/services/files/**/*.md', diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 282811368f981..0d65e3604286f 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -397,10 +397,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // Fork the process and listen for messages this._logService.debug(`Terminal process launching on ext host`, options); this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, cwd, cols, rows); - // this._terminalProcesses[id].on - // this._terminalProcesses[id].on('message', (message: IMessageFromTerminalProcess) => { - // switch (message.type) { - // }); this._terminalProcesses[id].onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid)); this._terminalProcesses[id].onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); this._terminalProcesses[id].onProcessData(data => this._proxy.$sendProcessData(id, data)); @@ -410,7 +406,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public $acceptProcessInput(id: number, data: string): void { if (this._terminalProcesses[id].isConnected) { this._terminalProcesses[id].input(data); - // this._terminalProcesses[id].send({ event: 'input', data }); } } @@ -418,7 +413,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { if (this._terminalProcesses[id].isConnected) { try { this._terminalProcesses[id].resize(cols, rows); - // this._terminalProcesses[id].send({ event: 'resize', cols, rows }); } catch (error) { // We tried to write to a closed pipe / channel. if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { @@ -431,15 +425,12 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public $acceptProcessShutdown(id: number): void { if (this._terminalProcesses[id].isConnected) { this._terminalProcesses[id].shutdown(); - // this._terminalProcesses[id].send({ event: 'shutdown' }); } } private _onProcessExit(id: number, exitCode: number): void { // Remove listeners - // const process = this._terminalProcesses[id]; - // process.removeAllListeners('message'); - // process.removeAllListeners('exit'); + this._terminalProcesses[id].dispose(); // Remove process reference delete this._terminalProcesses[id]; diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 6cb1d68cb0c92..6519252e7341c 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -122,7 +122,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { const options = { env, cwd }; this._logService.debug(`Terminal process launching`, options); - // this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], options); + // TODO: Send right args (on ext host too) this._process = new TerminalProcess(env['PTYSHELL'], [], env['PTYCWD'], cols, rows); } this.processState = ProcessState.LAUNCHING; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 050de680e29e5..fff587bc9eee7 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -9,8 +9,9 @@ import * as pty from 'node-pty'; import { Event, Emitter } from 'vs/base/common/event'; import { IProcessEnvironment } from 'vs/base/common/platform'; import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; +import { IDisposable } from 'vs/base/common/lifecycle'; -export class TerminalProcess implements ITerminalChildProcess { +export class TerminalProcess implements ITerminalChildProcess, IDisposable { private _exitCode: number; private _closeTimeout: number; private _ptyProcess: pty.IPty; @@ -65,6 +66,13 @@ export class TerminalProcess implements ITerminalChildProcess { this._setupTitlePolling(); } + public dispose(): void { + this._onProcessData.dispose(); + this._onProcessExit.dispose(); + this._onProcessIdReady.dispose(); + this._onProcessTitleChanged.dispose(); + } + private _createEnv(): IProcessEnvironment { const env: IProcessEnvironment = { ...process.env }; const keysToRemove = [ From 8b6f9e7396dd82a8eb3bba36db4b2b76de9df7a0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 11:49:05 -0700 Subject: [PATCH 06/14] Fix shell/args/cwd --- src/vs/workbench/api/node/extHost.api.impl.ts | 2 +- .../api/node/extHostTerminalService.ts | 22 +++++++++---------- .../terminalProcessManager.ts | 5 ++++- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index eea4190baedd5..31e31018b5e88 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -118,7 +118,7 @@ export function createApiFactory( const extHostFileSystem = rpcProtocol.set(ExtHostContext.ExtHostFileSystem, new ExtHostFileSystem(rpcProtocol, extHostLanguageFeatures)); const extHostFileSystemEvent = rpcProtocol.set(ExtHostContext.ExtHostFileSystemEventService, new ExtHostFileSystemEventService(rpcProtocol, extHostDocumentsAndEditors)); const extHostQuickOpen = rpcProtocol.set(ExtHostContext.ExtHostQuickOpen, new ExtHostQuickOpen(rpcProtocol, extHostWorkspace, extHostCommands)); - const extHostTerminalService = rpcProtocol.set(ExtHostContext.ExtHostTerminalService, new ExtHostTerminalService(rpcProtocol, extHostConfiguration, extHostLogService)); + const extHostTerminalService = rpcProtocol.set(ExtHostContext.ExtHostTerminalService, new ExtHostTerminalService(rpcProtocol, extHostConfiguration)); const extHostDebugService = rpcProtocol.set(ExtHostContext.ExtHostDebugService, new ExtHostDebugService(rpcProtocol, extHostWorkspace, extensionService, extHostDocumentsAndEditors, extHostConfiguration, extHostTerminalService)); const extHostSCM = rpcProtocol.set(ExtHostContext.ExtHostSCM, new ExtHostSCM(rpcProtocol, extHostCommands, extHostLogService)); const extHostSearch = rpcProtocol.set(ExtHostContext.ExtHostSearch, new ExtHostSearch(rpcProtocol, schemeTransformer)); diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 0d65e3604286f..dc52bfc002384 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -7,12 +7,12 @@ import * as vscode from 'vscode'; import * as os from 'os'; import * as platform from 'vs/base/common/platform'; -import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; -import Uri from 'vs/base/common/uri'; +// import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; +// import Uri from 'vs/base/common/uri'; import { Event, Emitter } from 'vs/base/common/event'; import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto } from 'vs/workbench/api/node/extHost.protocol'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -import { ILogService } from 'vs/platform/log/common/log'; +// import { ILogService } from 'vs/platform/log/common/log'; import { EXT_HOST_CREATION_DELAY } from 'vs/workbench/parts/terminal/common/terminal'; import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; @@ -241,7 +241,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { constructor( mainContext: IMainContext, private _extHostConfiguration: ExtHostConfiguration, - private _logService: ILogService + // private _logService: ILogService ) { this._proxy = mainContext.getProxy(MainContext.MainThreadTerminalService); } @@ -358,7 +358,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { const terminalConfig = this._extHostConfiguration.getConfiguration('terminal.integrated'); - const locale = terminalConfig.get('setLocaleVariables') ? platform.locale : undefined; + // TODO: Move locale into TerminalProcess + // const locale = terminalConfig.get('setLocaleVariables') ? platform.locale : undefined; if (!shellLaunchConfig.executable) { // TODO: This duplicates some of TerminalConfigHelper.mergeDefaultShellPathAndArgs and should be merged // this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); @@ -385,18 +386,17 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // shellLaunchConfig.env = envFromShell; // Merge process env with the env from config - const parentEnv = { ...process.env }; + // const parentEnv = { ...process.env }; // terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); // Continue env initialization, merging in the env from the launch // config and adding keys that are needed to create the process - const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, initialCwd, locale, cols, rows); - const cwd = Uri.parse(require.toUrl('../../parts/terminal/node')).fsPath; - const options = { env, cwd, execArgv: [] }; + // const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, initialCwd, locale, cols, rows); + // const options = { env, cwd, execArgv: [] }; // Fork the process and listen for messages - this._logService.debug(`Terminal process launching on ext host`, options); - this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, cwd, cols, rows); + // this._logService.debug(`Terminal process launching on ext host`, options); + this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, initialCwd, cols, rows); this._terminalProcesses[id].onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid)); this._terminalProcesses[id].onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); this._terminalProcesses[id].onProcessData(data => this._proxy.$sendProcessData(id, data)); diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 6519252e7341c..b276f5620c786 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -96,6 +96,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { if (extensionHostOwned) { this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); } else { + // TODO: Move locale into TerminalProcess const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; if (!shellLaunchConfig.executable) { this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); @@ -113,6 +114,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { // Merge process env with the env from config const parentEnv = { ...process.env }; + // TODO: Move environment merge stuff into TerminalProcess terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); // Continue env initialization, merging in the env from the launch @@ -123,7 +125,8 @@ export class TerminalProcessManager implements ITerminalProcessManager { this._logService.debug(`Terminal process launching`, options); // TODO: Send right args (on ext host too) - this._process = new TerminalProcess(env['PTYSHELL'], [], env['PTYCWD'], cols, rows); + console.log('create terminal process', env['PTYSHELL'], env['PTYCWD']); + this._process = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, this.initialCwd, cols, rows); } this.processState = ProcessState.LAUNCHING; From 549143dabf5875b6774f0c497e2b6e77135d550d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 13:38:35 -0700 Subject: [PATCH 07/14] Pass shell launch config to TerminalProcess --- .../workbench/api/node/extHostTerminalService.ts | 2 +- .../electron-browser/terminalProcessManager.ts | 2 +- .../parts/terminal/node/terminalProcess.ts | 14 +++++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index dc52bfc002384..4d93e8aed497a 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -396,7 +396,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // Fork the process and listen for messages // this._logService.debug(`Terminal process launching on ext host`, options); - this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, initialCwd, cols, rows); + this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows); this._terminalProcesses[id].onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid)); this._terminalProcesses[id].onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); this._terminalProcesses[id].onProcessData(data => this._proxy.$sendProcessData(id, data)); diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index b276f5620c786..c0c66b94773cf 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -126,7 +126,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { // TODO: Send right args (on ext host too) console.log('create terminal process', env['PTYSHELL'], env['PTYCWD']); - this._process = new TerminalProcess(shellLaunchConfig.executable, shellLaunchConfig.args, this.initialCwd, cols, rows); + this._process = new TerminalProcess(shellLaunchConfig, this.initialCwd, cols, rows); } this.processState = ProcessState.LAUNCHING; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index fff587bc9eee7..794adfa4dd317 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -6,10 +6,12 @@ import * as os from 'os'; import * as path from 'path'; import * as pty from 'node-pty'; +// import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import { Event, Emitter } from 'vs/base/common/event'; import { IProcessEnvironment } from 'vs/base/common/platform'; import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; import { IDisposable } from 'vs/base/common/lifecycle'; +import { IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal'; export class TerminalProcess implements ITerminalChildProcess, IDisposable { private _exitCode: number; @@ -26,12 +28,12 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { private readonly _onProcessTitleChanged: Emitter = new Emitter(); public get onProcessTitleChanged(): Event { return this._onProcessTitleChanged.event; } - constructor(shell: string, args: string | string[], cwd: string, cols: number, rows: number) { + constructor(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number) { // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, // see https://github.com/electron/electron/issues/38 let shellName: string; if (os.platform() === 'win32') { - shellName = path.basename(process.env.PTYSHELL); + shellName = path.basename(shellLaunchConfig.executable); } else { // Using 'xterm-256color' here helps ensure that the majority of Linux distributions will use a // color prompt as defined in the default ~/.bashrc file. @@ -46,7 +48,7 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { rows }; - this._ptyProcess = pty.spawn(shell, args, options); + this._ptyProcess = pty.spawn(shellLaunchConfig.executable, shellLaunchConfig.args, options); this._ptyProcess.on('data', (data) => { this._onProcessData.fire(data); if (this._closeTimeout) { @@ -100,6 +102,12 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { delete env[key]; } }); + // TODO: Determine which parts of env initialization should go where + // const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); + // const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); + // shellLaunchConfig.env = envFromShell; + // terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); + // const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); return env; } From 07da6c891d856733341de2cd50f0ab4adc0de949 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 14:21:25 -0700 Subject: [PATCH 08/14] wip --- src/vs/workbench/buildfile.js | 2 - .../terminalProcessManager.ts | 35 +------------ .../terminal/node/terminalEnvironment.ts | 20 ++------ .../parts/terminal/node/terminalProcess.ts | 51 ++++++++++++++++--- 4 files changed, 48 insertions(+), 60 deletions(-) diff --git a/src/vs/workbench/buildfile.js b/src/vs/workbench/buildfile.js index 281c194b60b4e..1dfb66c06b41e 100644 --- a/src/vs/workbench/buildfile.js +++ b/src/vs/workbench/buildfile.js @@ -27,8 +27,6 @@ exports.collectModules = function () { createModuleDescription('vs/workbench/services/files/node/watcher/nsfw/watcherApp', []), createModuleDescription('vs/workbench/node/extensionHostProcess', []), - - // createModuleDescription('vs/workbench/parts/terminal/node/terminalProcess', []) ]; return modules; diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index c0c66b94773cf..5ef75ca86bc51 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -3,10 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -// import * as cp from 'child_process'; -import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; -import Uri from 'vs/base/common/uri'; import { IDisposable } from 'vs/base/common/lifecycle'; import { ProcessState, ITerminalProcessManager, IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; import { TPromise } from 'vs/base/common/winjs.base'; @@ -53,9 +50,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { constructor( private _terminalId: number, private _configHelper: ITerminalConfigHelper, - @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, @IHistoryService private readonly _historyService: IHistoryService, - @IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private _logService: ILogService ) { @@ -96,37 +91,11 @@ export class TerminalProcessManager implements ITerminalProcessManager { if (extensionHostOwned) { this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); } else { - // TODO: Move locale into TerminalProcess - const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; - if (!shellLaunchConfig.executable) { - this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); - } - const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); - // Resolve env vars from config and shell - const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); - const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); - const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); - const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - shellLaunchConfig.env = envFromShell; - - // Merge process env with the env from config - const parentEnv = { ...process.env }; - // TODO: Move environment merge stuff into TerminalProcess - terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); - - // Continue env initialization, merging in the env from the launch - // config and adding keys that are needed to create the process - const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); - const cwd = Uri.parse(require.toUrl('../node')).fsPath; - const options = { env, cwd }; - this._logService.debug(`Terminal process launching`, options); - - // TODO: Send right args (on ext host too) - console.log('create terminal process', env['PTYSHELL'], env['PTYCWD']); - this._process = new TerminalProcess(shellLaunchConfig, this.initialCwd, cols, rows); + this._logService.debug(`Terminal process launching`, shellLaunchConfig, this.initialCwd, cols, rows); + this._process = this._instantiationService.createInstance(TerminalProcess, shellLaunchConfig, this.initialCwd, cols, rows, lastActiveWorkspaceRootUri, this._configHelper); } this.processState = ProcessState.LAUNCHING; diff --git a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts index e54909e939a0f..d2780d3a1784a 100644 --- a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts +++ b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts @@ -52,30 +52,16 @@ function _mergeEnvironmentValue(env: IStringDictionary, key: string, val } } -export function createTerminalEnv(parentEnv: IStringDictionary, shell: IShellLaunchConfig, cwd: string, locale: string, cols?: number, rows?: number): IStringDictionary { - const env = { ...parentEnv }; +export function createTerminalEnv(shell: IShellLaunchConfig, locale: string): IStringDictionary { + const env = { ...process.env }; if (shell.env) { mergeEnvironments(env, shell.env); } - // env['PTYPID'] = process.pid.toString(); - // env['PTYSHELL'] = shell.executable; env['TERM_PROGRAM'] = 'vscode'; env['TERM_PROGRAM_VERSION'] = pkg.version; - // if (shell.args) { - // if (typeof shell.args === 'string') { - // env[`PTYSHELLCMDLINE`] = shell.args; - // } else { - // shell.args.forEach((arg, i) => env[`PTYSHELLARG${i}`] = arg); - // } - // } - // env['PTYCWD'] = cwd; env['LANG'] = _getLangEnvVariable(locale); - // if (cols && rows) { - // env['PTYCOLS'] = cols.toString(); - // env['PTYROWS'] = rows.toString(); - // } - env['AMD_ENTRYPOINT'] = 'vs/workbench/parts/terminal/node/terminalProcess'; + return env; } diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 794adfa4dd317..6966f93405b02 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -5,13 +5,16 @@ import * as os from 'os'; import * as path from 'path'; +import * as platform from 'vs/base/common/platform'; import * as pty from 'node-pty'; -// import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; +import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import { Event, Emitter } from 'vs/base/common/event'; -import { IProcessEnvironment } from 'vs/base/common/platform'; import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; import { IDisposable } from 'vs/base/common/lifecycle'; -import { IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal'; +import { IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; +import { IWorkspaceContextService, IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; +import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; +import URI from 'vs/base/common/uri'; export class TerminalProcess implements ITerminalChildProcess, IDisposable { private _exitCode: number; @@ -28,7 +31,16 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { private readonly _onProcessTitleChanged: Emitter = new Emitter(); public get onProcessTitleChanged(): Event { return this._onProcessTitleChanged.event; } - constructor(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number) { + constructor( + @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, + @IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService, + shellLaunchConfig: IShellLaunchConfig, + cwd: string, + cols: number, + rows: number, + lastActiveWorkspaceRootUri: URI, + private readonly _configHelper: ITerminalConfigHelper + ) { // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, // see https://github.com/electron/electron/issues/38 let shellName: string; @@ -40,10 +52,17 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { shellName = 'xterm-256color'; } + if (!shellLaunchConfig.executable) { + this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); + } + + const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); + + const env = this._createEnv(shellLaunchConfig, lastActiveWorkspaceRoot); const options: pty.IPtyForkOptions = { name: shellName, cwd, - env: this._createEnv(), + env, cols, rows }; @@ -75,10 +94,26 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { this._onProcessTitleChanged.dispose(); } - private _createEnv(): IProcessEnvironment { - const env: IProcessEnvironment = { ...process.env }; + private _createEnv(shellLaunchConfig: IShellLaunchConfig, lastActiveWorkspaceRoot: IWorkspaceFolder): platform.IProcessEnvironment { + // TODO: Move locale into TerminalProcess + const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; + // Resolve env vars from config and shell + const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); + const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); + const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); + shellLaunchConfig.env = envFromShell; + + const env: platform.IProcessEnvironment = { ...process.env }; + + // Merge process env with the env from config + // TODO: Move environment merge stuff into TerminalProcess + terminalEnvironment.mergeEnvironments(env, envFromConfig); + + // Continue env initialization, merging in the env from the launch + // config and adding keys that are needed to create the process + const env = terminalEnvironment.createTerminalEnv(shellLaunchConfig, locale); + const keysToRemove = [ - 'AMD_ENTRYPOINT', 'ELECTRON_ENABLE_STACK_DUMPING', 'ELECTRON_ENABLE_LOGGING', 'ELECTRON_NO_ASAR', From a50e6d4eeb7c1da3da1d9cd6cd5b81c8a5055241 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 15:20:00 -0700 Subject: [PATCH 09/14] Clean up and fix handling of environment --- src/vs/workbench/api/node/extHost.api.impl.ts | 2 +- .../api/node/extHostTerminalService.ts | 23 +++--- .../terminalProcessManager.ts | 41 ++++++++-- .../terminal/node/terminalEnvironment.ts | 45 ++++++++--- .../parts/terminal/node/terminalProcess.ts | 70 +---------------- .../test/node/terminalEnvironment.test.ts | 76 +++++++++---------- 6 files changed, 119 insertions(+), 138 deletions(-) diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index 31e31018b5e88..eea4190baedd5 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -118,7 +118,7 @@ export function createApiFactory( const extHostFileSystem = rpcProtocol.set(ExtHostContext.ExtHostFileSystem, new ExtHostFileSystem(rpcProtocol, extHostLanguageFeatures)); const extHostFileSystemEvent = rpcProtocol.set(ExtHostContext.ExtHostFileSystemEventService, new ExtHostFileSystemEventService(rpcProtocol, extHostDocumentsAndEditors)); const extHostQuickOpen = rpcProtocol.set(ExtHostContext.ExtHostQuickOpen, new ExtHostQuickOpen(rpcProtocol, extHostWorkspace, extHostCommands)); - const extHostTerminalService = rpcProtocol.set(ExtHostContext.ExtHostTerminalService, new ExtHostTerminalService(rpcProtocol, extHostConfiguration)); + const extHostTerminalService = rpcProtocol.set(ExtHostContext.ExtHostTerminalService, new ExtHostTerminalService(rpcProtocol, extHostConfiguration, extHostLogService)); const extHostDebugService = rpcProtocol.set(ExtHostContext.ExtHostDebugService, new ExtHostDebugService(rpcProtocol, extHostWorkspace, extensionService, extHostDocumentsAndEditors, extHostConfiguration, extHostTerminalService)); const extHostSCM = rpcProtocol.set(ExtHostContext.ExtHostSCM, new ExtHostSCM(rpcProtocol, extHostCommands, extHostLogService)); const extHostSearch = rpcProtocol.set(ExtHostContext.ExtHostSearch, new ExtHostSearch(rpcProtocol, schemeTransformer)); diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 4d93e8aed497a..b6de962d48ce6 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -7,12 +7,11 @@ import * as vscode from 'vscode'; import * as os from 'os'; import * as platform from 'vs/base/common/platform'; -// import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; -// import Uri from 'vs/base/common/uri'; +import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import { Event, Emitter } from 'vs/base/common/event'; import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto } from 'vs/workbench/api/node/extHost.protocol'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -// import { ILogService } from 'vs/platform/log/common/log'; +import { ILogService } from 'vs/platform/log/common/log'; import { EXT_HOST_CREATION_DELAY } from 'vs/workbench/parts/terminal/common/terminal'; import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; @@ -241,7 +240,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { constructor( mainContext: IMainContext, private _extHostConfiguration: ExtHostConfiguration, - // private _logService: ILogService + private _logService: ILogService ) { this._proxy = mainContext.getProxy(MainContext.MainThreadTerminalService); } @@ -358,8 +357,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { const terminalConfig = this._extHostConfiguration.getConfiguration('terminal.integrated'); - // TODO: Move locale into TerminalProcess - // const locale = terminalConfig.get('setLocaleVariables') ? platform.locale : undefined; if (!shellLaunchConfig.executable) { // TODO: This duplicates some of TerminalConfigHelper.mergeDefaultShellPathAndArgs and should be merged // this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); @@ -383,20 +380,20 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); // const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); // const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - // shellLaunchConfig.env = envFromShell; // Merge process env with the env from config - // const parentEnv = { ...process.env }; - // terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); + const env = { ...process.env }; + // terminalEnvironment.mergeEnvironments(env, envFromConfig); + terminalEnvironment.mergeEnvironments(env, shellLaunchConfig.env); // Continue env initialization, merging in the env from the launch // config and adding keys that are needed to create the process - // const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, initialCwd, locale, cols, rows); - // const options = { env, cwd, execArgv: [] }; + const locale = terminalConfig.get('setLocaleVariables') ? platform.locale : undefined; + terminalEnvironment.addTerminalEnvironmentKeys(env, locale); // Fork the process and listen for messages - // this._logService.debug(`Terminal process launching on ext host`, options); - this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows); + this._logService.debug(`Terminal process launching on ext host`, shellLaunchConfig, initialCwd, cols, rows, env); + this._terminalProcesses[id] = new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env); this._terminalProcesses[id].onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid)); this._terminalProcesses[id].onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); this._terminalProcesses[id].onProcessData(data => this._proxy.$sendProcessData(id, data)); diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 5ef75ca86bc51..e2729910d6511 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -3,19 +3,20 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import { IDisposable } from 'vs/base/common/lifecycle'; import { ProcessState, ITerminalProcessManager, IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; import { TPromise } from 'vs/base/common/winjs.base'; import { ILogService } from 'vs/platform/log/common/log'; import { Emitter, Event } from 'vs/base/common/event'; -import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; -import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; import { TerminalProcessExtHostProxy } from 'vs/workbench/parts/terminal/node/terminalProcessExtHostProxy'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { TerminalProcess } from 'vs/workbench/parts/terminal/node/terminalProcess'; +import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; +import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; /** The amount of time to consider terminal errors to be related to the launch */ const LAUNCHING_DURATION = 500; @@ -48,11 +49,13 @@ export class TerminalProcessManager implements ITerminalProcessManager { public get onProcessExit(): Event { return this._onProcessExit.event; } constructor( - private _terminalId: number, - private _configHelper: ITerminalConfigHelper, + private readonly _terminalId: number, + private readonly _configHelper: ITerminalConfigHelper, @IHistoryService private readonly _historyService: IHistoryService, @IInstantiationService private readonly _instantiationService: IInstantiationService, - @ILogService private _logService: ILogService + @ILogService private readonly _logService: ILogService, + @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, + @IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService ) { this.ptyProcessReady = new TPromise(c => { this.onProcessReady(() => { @@ -91,11 +94,35 @@ export class TerminalProcessManager implements ITerminalProcessManager { if (extensionHostOwned) { this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, cols, rows); } else { + if (!shellLaunchConfig.executable) { + this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); + } + const lastActiveWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot('file'); this.initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, lastActiveWorkspaceRootUri, this._configHelper); - this._logService.debug(`Terminal process launching`, shellLaunchConfig, this.initialCwd, cols, rows); - this._process = this._instantiationService.createInstance(TerminalProcess, shellLaunchConfig, this.initialCwd, cols, rows, lastActiveWorkspaceRootUri, this._configHelper); + // Resolve env vars from config and shell + const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); + const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); + const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); + const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); + shellLaunchConfig.env = envFromShell; + + // Merge process env with the env from config and from shellLaunchConfig + const env = { ...process.env }; + terminalEnvironment.mergeEnvironments(env, envFromConfig); + terminalEnvironment.mergeEnvironments(env, shellLaunchConfig.env); + + // Sanitize the environment, removing any undesirable VS Code and Electron environment + // variables + terminalEnvironment.sanitizeEnvironment(env); + + // Adding other env keys necessary to create the process + const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; + terminalEnvironment.addTerminalEnvironmentKeys(env, locale); + + this._logService.debug(`Terminal process launching`, shellLaunchConfig, this.initialCwd, cols, rows, env); + this._process = new TerminalProcess(shellLaunchConfig, this.initialCwd, cols, rows, env); } this.processState = ProcessState.LAUNCHING; diff --git a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts index d2780d3a1784a..84c4aa560c417 100644 --- a/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts +++ b/src/vs/workbench/parts/terminal/node/terminalEnvironment.ts @@ -8,7 +8,6 @@ import * as paths from 'vs/base/common/paths'; import * as platform from 'vs/base/common/platform'; import pkg from 'vs/platform/node/package'; import Uri from 'vs/base/common/uri'; -import { IStringDictionary } from 'vs/base/common/collections'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -17,7 +16,7 @@ import { IConfigurationResolverService } from 'vs/workbench/services/configurati * This module contains utility functions related to the environment, cwd and paths. */ -export function mergeEnvironments(parent: IStringDictionary, other: IStringDictionary) { +export function mergeEnvironments(parent: platform.IProcessEnvironment, other: platform.IProcessEnvironment): void { if (!other) { return; } @@ -44,7 +43,7 @@ export function mergeEnvironments(parent: IStringDictionary, other: IStr } } -function _mergeEnvironmentValue(env: IStringDictionary, key: string, value: string | null) { +function _mergeEnvironmentValue(env: platform.IProcessEnvironment, key: string, value: string | null): void { if (typeof value === 'string') { env[key] = value; } else { @@ -52,20 +51,44 @@ function _mergeEnvironmentValue(env: IStringDictionary, key: string, val } } -export function createTerminalEnv(shell: IShellLaunchConfig, locale: string): IStringDictionary { - const env = { ...process.env }; - if (shell.env) { - mergeEnvironments(env, shell.env); - } +export function sanitizeEnvironment(env: platform.IProcessEnvironment): void { + // Remove keys based on strings + const keysToRemove = [ + 'ELECTRON_ENABLE_STACK_DUMPING', + 'ELECTRON_ENABLE_LOGGING', + 'ELECTRON_NO_ASAR', + 'ELECTRON_NO_ATTACH_CONSOLE', + 'ELECTRON_RUN_AS_NODE', + 'GOOGLE_API_KEY', + 'VSCODE_CLI', + 'VSCODE_DEV', + 'VSCODE_IPC_HOOK', + 'VSCODE_LOGS', + 'VSCODE_NLS_CONFIG', + 'VSCODE_PORTABLE', + 'VSCODE_PID', + ]; + keysToRemove.forEach((key) => { + if (env[key]) { + delete env[key]; + } + }); + + // Remove keys based on regexp + Object.keys(env).forEach(key => { + if (key.search(/^VSCODE_NODE_CACHED_DATA_DIR_\d+$/) === 0) { + delete env[key]; + } + }); +} +export function addTerminalEnvironmentKeys(env: platform.IProcessEnvironment, locale: string | undefined): void { env['TERM_PROGRAM'] = 'vscode'; env['TERM_PROGRAM_VERSION'] = pkg.version; env['LANG'] = _getLangEnvVariable(locale); - - return env; } -export function resolveConfigurationVariables(configurationResolverService: IConfigurationResolverService, env: IStringDictionary, lastActiveWorkspaceRoot: IWorkspaceFolder): IStringDictionary { +export function resolveConfigurationVariables(configurationResolverService: IConfigurationResolverService, env: platform.IProcessEnvironment, lastActiveWorkspaceRoot: IWorkspaceFolder): platform.IProcessEnvironment { Object.keys(env).forEach((key) => { if (typeof env[key] === 'string') { env[key] = configurationResolverService.resolve(lastActiveWorkspaceRoot, env[key]); diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 6966f93405b02..599a1f3156cab 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -7,14 +7,10 @@ import * as os from 'os'; import * as path from 'path'; import * as platform from 'vs/base/common/platform'; import * as pty from 'node-pty'; -import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import { Event, Emitter } from 'vs/base/common/event'; import { ITerminalChildProcess } from 'vs/workbench/parts/terminal/node/terminal'; import { IDisposable } from 'vs/base/common/lifecycle'; -import { IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; -import { IWorkspaceContextService, IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; -import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; -import URI from 'vs/base/common/uri'; +import { IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal'; export class TerminalProcess implements ITerminalChildProcess, IDisposable { private _exitCode: number; @@ -32,14 +28,11 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { public get onProcessTitleChanged(): Event { return this._onProcessTitleChanged.event; } constructor( - @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, - @IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService, shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, - lastActiveWorkspaceRootUri: URI, - private readonly _configHelper: ITerminalConfigHelper + env: platform.IProcessEnvironment ) { // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, // see https://github.com/electron/electron/issues/38 @@ -52,13 +45,6 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { shellName = 'xterm-256color'; } - if (!shellLaunchConfig.executable) { - this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); - } - - const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); - - const env = this._createEnv(shellLaunchConfig, lastActiveWorkspaceRoot); const options: pty.IPtyForkOptions = { name: shellName, cwd, @@ -94,58 +80,6 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { this._onProcessTitleChanged.dispose(); } - private _createEnv(shellLaunchConfig: IShellLaunchConfig, lastActiveWorkspaceRoot: IWorkspaceFolder): platform.IProcessEnvironment { - // TODO: Move locale into TerminalProcess - const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined; - // Resolve env vars from config and shell - const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); - const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); - const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - shellLaunchConfig.env = envFromShell; - - const env: platform.IProcessEnvironment = { ...process.env }; - - // Merge process env with the env from config - // TODO: Move environment merge stuff into TerminalProcess - terminalEnvironment.mergeEnvironments(env, envFromConfig); - - // Continue env initialization, merging in the env from the launch - // config and adding keys that are needed to create the process - const env = terminalEnvironment.createTerminalEnv(shellLaunchConfig, locale); - - const keysToRemove = [ - 'ELECTRON_ENABLE_STACK_DUMPING', - 'ELECTRON_ENABLE_LOGGING', - 'ELECTRON_NO_ASAR', - 'ELECTRON_RUN_AS_NODE', - 'GOOGLE_API_KEY', - 'VSCODE_CLI', - 'VSCODE_DEV', - 'VSCODE_IPC_HOOK', - 'VSCODE_LOGS', - 'VSCODE_NLS_CONFIG', - 'VSCODE_PORTABLE', - 'VSCODE_PID', - ]; - keysToRemove.forEach((key) => { - if (env[key]) { - delete env[key]; - } - }); - Object.keys(env).forEach(key => { - if (key.search(/^VSCODE_NODE_CACHED_DATA_DIR_\d+$/) === 0) { - delete env[key]; - } - }); - // TODO: Determine which parts of env initialization should go where - // const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...this._configHelper.config.env[platformKey] }, lastActiveWorkspaceRoot); - // const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); - // shellLaunchConfig.env = envFromShell; - // terminalEnvironment.mergeEnvironments(parentEnv, envFromConfig); - // const env = terminalEnvironment.createTerminalEnv(parentEnv, shellLaunchConfig, this.initialCwd, locale, cols, rows); - return env; - } - private _setupTitlePolling() { this._sendProcessTitle(); setInterval(() => { diff --git a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts index 5fae93b3d0183..b5223b0949ef7 100644 --- a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts +++ b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts @@ -9,46 +9,46 @@ import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import Uri from 'vs/base/common/uri'; import { IStringDictionary } from 'vs/base/common/collections'; -import { IShellLaunchConfig, ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; +import { /*IShellLaunchConfig,*/ ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; suite('Workbench - TerminalEnvironment', () => { - test('createTerminalEnv', function () { - const shell1 = { - executable: '/bin/foosh', - args: ['-bar', 'baz'] - }; - const parentEnv1: IStringDictionary = { - ok: true - } as any; - const env1 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/foo', 'en-au'); - assert.ok(env1['ok'], 'Parent environment is copied'); - assert.deepStrictEqual(parentEnv1, { ok: true }, 'Parent environment is unchanged'); - assert.equal(env1['PTYPID'], process.pid.toString(), 'PTYPID is equal to the current PID'); - assert.equal(env1['PTYSHELL'], '/bin/foosh', 'PTYSHELL is equal to the provided shell'); - assert.equal(env1['PTYSHELLARG0'], '-bar', 'PTYSHELLARG0 is equal to the first shell argument'); - assert.equal(env1['PTYSHELLARG1'], 'baz', 'PTYSHELLARG1 is equal to the first shell argument'); - assert.ok(!('PTYSHELLARG2' in env1), 'PTYSHELLARG2 is unset'); - assert.equal(env1['PTYCWD'], '/foo', 'PTYCWD is equal to requested cwd'); - assert.equal(env1['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); - - const shell2: IShellLaunchConfig = { - executable: '/bin/foosh', - args: [] - }; - const parentEnv2: IStringDictionary = { - LANG: 'en_US.UTF-8' - }; - const env2 = terminalEnvironment.createTerminalEnv(parentEnv2, shell2, '/foo', 'en-au'); - assert.ok(!('PTYSHELLARG0' in env2), 'PTYSHELLARG0 is unset'); - assert.equal(env2['PTYCWD'], '/foo', 'PTYCWD is equal to /foo'); - assert.equal(env2['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); - - const env3 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/', null); - assert.equal(env3['LANG'], 'en_US.UTF-8', 'LANG is equal to en_US.UTF-8 as fallback.'); // More info on issue #14586 - - const env4 = terminalEnvironment.createTerminalEnv(parentEnv2, shell1, '/', null); - assert.equal(env4['LANG'], 'en_US.UTF-8', 'LANG is equal to the parent environment\'s LANG'); - }); + // test('createTerminalEnv', function () { + // const shell1 = { + // executable: '/bin/foosh', + // args: ['-bar', 'baz'] + // }; + // const parentEnv1: IStringDictionary = { + // ok: true + // } as any; + // const env1 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/foo', 'en-au'); + // assert.ok(env1['ok'], 'Parent environment is copied'); + // assert.deepStrictEqual(parentEnv1, { ok: true }, 'Parent environment is unchanged'); + // assert.equal(env1['PTYPID'], process.pid.toString(), 'PTYPID is equal to the current PID'); + // assert.equal(env1['PTYSHELL'], '/bin/foosh', 'PTYSHELL is equal to the provided shell'); + // assert.equal(env1['PTYSHELLARG0'], '-bar', 'PTYSHELLARG0 is equal to the first shell argument'); + // assert.equal(env1['PTYSHELLARG1'], 'baz', 'PTYSHELLARG1 is equal to the first shell argument'); + // assert.ok(!('PTYSHELLARG2' in env1), 'PTYSHELLARG2 is unset'); + // assert.equal(env1['PTYCWD'], '/foo', 'PTYCWD is equal to requested cwd'); + // assert.equal(env1['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); + + // const shell2: IShellLaunchConfig = { + // executable: '/bin/foosh', + // args: [] + // }; + // const parentEnv2: IStringDictionary = { + // LANG: 'en_US.UTF-8' + // }; + // const env2 = terminalEnvironment.createTerminalEnv(parentEnv2, shell2, '/foo', 'en-au'); + // assert.ok(!('PTYSHELLARG0' in env2), 'PTYSHELLARG0 is unset'); + // assert.equal(env2['PTYCWD'], '/foo', 'PTYCWD is equal to /foo'); + // assert.equal(env2['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); + + // const env3 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/', null); + // assert.equal(env3['LANG'], 'en_US.UTF-8', 'LANG is equal to en_US.UTF-8 as fallback.'); // More info on issue #14586 + + // const env4 = terminalEnvironment.createTerminalEnv(parentEnv2, shell1, '/', null); + // assert.equal(env4['LANG'], 'en_US.UTF-8', 'LANG is equal to the parent environment\'s LANG'); + // }); suite('mergeEnvironments', () => { test('should add keys', () => { From f667da4163276c027a9a91d900d41956585dbe3e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 11 Jul 2018 15:25:21 -0700 Subject: [PATCH 10/14] Remove ITerminalProcess.isConnected The terminal process no longer exists so this isn't needed anymore --- .../api/node/extHostTerminalService.ts | 22 +++++------- .../terminalProcessManager.ts | 35 +++++++++---------- .../workbench/parts/terminal/node/terminal.ts | 3 -- .../parts/terminal/node/terminalProcess.ts | 5 --- .../node/terminalProcessExtHostProxy.ts | 32 ----------------- 5 files changed, 25 insertions(+), 72 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index b6de962d48ce6..55ecfbfe88dcf 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -401,28 +401,22 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } public $acceptProcessInput(id: number, data: string): void { - if (this._terminalProcesses[id].isConnected) { - this._terminalProcesses[id].input(data); - } + this._terminalProcesses[id].input(data); } public $acceptProcessResize(id: number, cols: number, rows: number): void { - if (this._terminalProcesses[id].isConnected) { - try { - this._terminalProcesses[id].resize(cols, rows); - } catch (error) { - // We tried to write to a closed pipe / channel. - if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { - throw (error); - } + try { + this._terminalProcesses[id].resize(cols, rows); + } catch (error) { + // We tried to write to a closed pipe / channel. + if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { + throw (error); } } } public $acceptProcessShutdown(id: number): void { - if (this._terminalProcesses[id].isConnected) { - this._terminalProcesses[id].shutdown(); - } + this._terminalProcesses[id].shutdown(); } private _onProcessExit(id: number, exitCode: number): void { diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index e2729910d6511..249aea18c88e8 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -67,14 +67,11 @@ export class TerminalProcessManager implements ITerminalProcessManager { public dispose(): void { if (this._process) { - if (this._process.isConnected) { - // If the process was still connected this dispose came from - // within VS Code, not the process, so mark the process as - // killed by the user. - this.processState = ProcessState.KILLED_BY_USER; - this._process.shutdown(); - // this._process.send({ event: 'shutdown' }); - } + // If the process was still connected this dispose came from + // within VS Code, not the process, so mark the process as + // killed by the user. + this.processState = ProcessState.KILLED_BY_USER; + this._process.shutdown(); this._process = null; } this._disposables.forEach(d => d.dispose()); @@ -154,16 +151,18 @@ export class TerminalProcessManager implements ITerminalProcessManager { } public setDimensions(cols: number, rows: number): void { - if (this._process && this._process.isConnected) { - // The child process could aready be terminated - try { - this._process.resize(cols, rows); - // this._process.send({ event: 'resize', cols, rows }); - } catch (error) { - // We tried to write to a closed pipe / channel. - if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { - throw (error); - } + if (!this._process) { + return; + } + + // The child process could already be terminated + try { + this._process.resize(cols, rows); + // this._process.send({ event: 'resize', cols, rows }); + } catch (error) { + // We tried to write to a closed pipe / channel. + if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { + throw (error); } } } diff --git a/src/vs/workbench/parts/terminal/node/terminal.ts b/src/vs/workbench/parts/terminal/node/terminal.ts index d85830b8a0412..8d787d74e93f7 100644 --- a/src/vs/workbench/parts/terminal/node/terminal.ts +++ b/src/vs/workbench/parts/terminal/node/terminal.ts @@ -14,9 +14,6 @@ import { Event } from 'vs/base/common/event'; * child_process.ChildProcess node.js interface. */ export interface ITerminalChildProcess { - // TODO: Remove connected and references to it - readonly isConnected: boolean; - onProcessData: Event; onProcessExit: Event; onProcessIdReady: Event; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 599a1f3156cab..0175866b43046 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -123,9 +123,4 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { // exception in winpty. this._ptyProcess.resize(Math.max(cols, 1), Math.max(rows, 1)); } - - public get isConnected(): boolean { - // Don't need connected anymore as it's the same process - return true; - } } diff --git a/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts b/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts index 422e7ae45c59a..52bc33c6fa5b8 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcessExtHostProxy.ts @@ -9,9 +9,6 @@ import { ITerminalService, ITerminalProcessExtHostProxy, IShellLaunchConfig } fr import { IDisposable } from 'vs/base/common/lifecycle'; export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerminalProcessExtHostProxy { - // For ext host processes connected checks happen on the ext host - public isConnected: boolean = true; - private _disposables: IDisposable[] = []; private readonly _onProcessData: Emitter = new Emitter(); @@ -62,16 +59,6 @@ export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerm this._onProcessExit.fire(exitCode); this.dispose(); } - - // public send(message: IMessageToTerminalProcess): boolean { - // switch (message.event) { - // case 'input': this.emit('input', message.data); break; - // case 'resize': this.emit('resize', message.cols, message.rows); break; - // case 'shutdown': this.emit('shutdown'); break; - // } - // return true; - // } - public shutdown(): void { this._onShutdown.fire(); } @@ -83,23 +70,4 @@ export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerm public resize(cols: number, rows: number): void { this._onResize.fire({ cols, rows }); } - - - // public onInput(listener: (data: string) => void): void { - // const outerListener = (data) => listener(data); - // this.on('input', outerListener); - // this._disposables.push(toDisposable(() => this.removeListener('input', outerListener))); - // } - - // public onResize(listener: (cols: number, rows: number) => void): void { - // const outerListener = (cols, rows) => listener(cols, rows); - // this.on('resize', outerListener); - // this._disposables.push(toDisposable(() => this.removeListener('resize', outerListener))); - // } - - // public onShutdown(listener: () => void): void { - // const outerListener = () => listener(); - // this.on('shutdown', outerListener); - // this._disposables.push(toDisposable(() => this.removeListener('shutdown', outerListener))); - // } } \ No newline at end of file From 2b3c5473e5e5c4dbb3b342e38a806055528658e1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 12 Jul 2018 06:20:14 -0700 Subject: [PATCH 11/14] More clean up, add tests back --- .../terminalProcessManager.ts | 28 ------- .../parts/terminal/node/terminalProcess.ts | 8 +- .../test/node/terminalEnvironment.test.ts | 75 ++++++++++--------- 3 files changed, 42 insertions(+), 69 deletions(-) diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts index 249aea18c88e8..c4d62a87eadf0 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalProcessManager.ts @@ -140,8 +140,6 @@ export class TerminalProcessManager implements ITerminalProcessManager { this._process.onProcessTitleChanged(title => this._onProcessTitle.fire(title)); this._process.onProcessExit(exitCode => this._onExit(exitCode)); - // this._process.on('message', message => this._onMessage(message)); - // this._process.on('exit', exitCode => this._onExit(exitCode)); setTimeout(() => { if (this.processState === ProcessState.LAUNCHING) { @@ -158,7 +156,6 @@ export class TerminalProcessManager implements ITerminalProcessManager { // The child process could already be terminated try { this._process.resize(cols, rows); - // this._process.send({ event: 'resize', cols, rows }); } catch (error) { // We tried to write to a closed pipe / channel. if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') { @@ -179,31 +176,6 @@ export class TerminalProcessManager implements ITerminalProcessManager { } } - // private _onMessage(message: IMessageFromTerminalProcess): void { - // this._logService.trace(`terminalProcessManager#_onMessage (shellProcessId: ${this.shellProcessId}`, message); - // switch (message.type) { - // case 'data': - // this._onProcessData.fire(message.content); - // break; - // case 'pid': - // this.shellProcessId = message.content; - // this._onProcessReady.fire(); - - // // Send any queued data that's waiting - // if (this._preLaunchInputQueue.length > 0) { - // this._process.send({ - // event: 'input', - // data: this._preLaunchInputQueue.join('') - // }); - // this._preLaunchInputQueue.length = 0; - // } - // break; - // case 'title': - // this._onProcessTitle.fire(message.content); - // break; - // } - // } - private _onExit(exitCode: number): void { this._process = null; diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 0175866b43046..3a31266d9652b 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -34,8 +34,6 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { rows: number, env: platform.IProcessEnvironment ) { - // The pty process needs to be run in its own child process to get around maxing out CPU on Mac, - // see https://github.com/electron/electron/issues/38 let shellName: string; if (os.platform() === 'win32') { shellName = path.basename(shellLaunchConfig.executable); @@ -66,10 +64,10 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { this._queueProcessExit(); }); - // TODO: We should no longer need to delay this since spawn is sync + // TODO: We should no longer need to delay this since pty.spawn is sync setTimeout(() => { this._sendProcessId(); - }, 1000); + }, 500); this._setupTitlePolling(); } @@ -99,12 +97,14 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { this._closeTimeout = setTimeout(() => { this._ptyProcess.kill(); this._onProcessExit.fire(this._exitCode); + this.dispose(); }, 250); } private _sendProcessId() { this._onProcessIdReady.fire(this._ptyProcess.pid); } + private _sendProcessTitle(): void { this._currentTitle = this._ptyProcess.process; this._onProcessTitleChanged.fire(this._currentTitle); diff --git a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts index b5223b0949ef7..8c524e8324899 100644 --- a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts +++ b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts @@ -12,43 +12,44 @@ import { IStringDictionary } from 'vs/base/common/collections'; import { /*IShellLaunchConfig,*/ ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; suite('Workbench - TerminalEnvironment', () => { - // test('createTerminalEnv', function () { - // const shell1 = { - // executable: '/bin/foosh', - // args: ['-bar', 'baz'] - // }; - // const parentEnv1: IStringDictionary = { - // ok: true - // } as any; - // const env1 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/foo', 'en-au'); - // assert.ok(env1['ok'], 'Parent environment is copied'); - // assert.deepStrictEqual(parentEnv1, { ok: true }, 'Parent environment is unchanged'); - // assert.equal(env1['PTYPID'], process.pid.toString(), 'PTYPID is equal to the current PID'); - // assert.equal(env1['PTYSHELL'], '/bin/foosh', 'PTYSHELL is equal to the provided shell'); - // assert.equal(env1['PTYSHELLARG0'], '-bar', 'PTYSHELLARG0 is equal to the first shell argument'); - // assert.equal(env1['PTYSHELLARG1'], 'baz', 'PTYSHELLARG1 is equal to the first shell argument'); - // assert.ok(!('PTYSHELLARG2' in env1), 'PTYSHELLARG2 is unset'); - // assert.equal(env1['PTYCWD'], '/foo', 'PTYCWD is equal to requested cwd'); - // assert.equal(env1['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); - - // const shell2: IShellLaunchConfig = { - // executable: '/bin/foosh', - // args: [] - // }; - // const parentEnv2: IStringDictionary = { - // LANG: 'en_US.UTF-8' - // }; - // const env2 = terminalEnvironment.createTerminalEnv(parentEnv2, shell2, '/foo', 'en-au'); - // assert.ok(!('PTYSHELLARG0' in env2), 'PTYSHELLARG0 is unset'); - // assert.equal(env2['PTYCWD'], '/foo', 'PTYCWD is equal to /foo'); - // assert.equal(env2['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); - - // const env3 = terminalEnvironment.createTerminalEnv(parentEnv1, shell1, '/', null); - // assert.equal(env3['LANG'], 'en_US.UTF-8', 'LANG is equal to en_US.UTF-8 as fallback.'); // More info on issue #14586 - - // const env4 = terminalEnvironment.createTerminalEnv(parentEnv2, shell1, '/', null); - // assert.equal(env4['LANG'], 'en_US.UTF-8', 'LANG is equal to the parent environment\'s LANG'); - // }); + test('addTerminalEnvironmentKeys', () => { + const env = { FOO: 'bar' }; + const locale = 'en-au'; + terminalEnvironment.addTerminalEnvironmentKeys(env, locale); + assert.equal(env['TERM_PROGRAM'], 'vscode'); + assert.equal(env['TERM_PROGRAM_VERSION'].search(/^\d+\.\d+\.\d+$/), 0); + assert.equal(env['LANG'], 'en_AU.UTF-8', 'LANG is equal to the requested locale with UTF-8'); + + const env2 = { FOO: 'bar' }; + terminalEnvironment.addTerminalEnvironmentKeys(env2, null); + assert.equal(env2['LANG'], 'en_US.UTF-8', 'LANG is equal to en_US.UTF-8 as fallback.'); // More info on issue #14586 + + const env3 = { LANG: 'en_US.UTF-8' }; + terminalEnvironment.addTerminalEnvironmentKeys(env3, null); + assert.equal(env3['LANG'], 'en_US.UTF-8', 'LANG is equal to the parent environment\'s LANG'); + }); + + test('sanitizeEnvironment', () => { + let env = { + FOO: 'bar', + ELECTRON_ENABLE_STACK_DUMPING: 'x', + ELECTRON_ENABLE_LOGGING: 'x', + ELECTRON_NO_ASAR: 'x', + ELECTRON_NO_ATTACH_CONSOLE: 'x', + ELECTRON_RUN_AS_NODE: 'x', + GOOGLE_API_KEY: 'x', + VSCODE_CLI: 'x', + VSCODE_DEV: 'x', + VSCODE_IPC_HOOK: 'x', + VSCODE_LOGS: 'x', + VSCODE_NLS_CONFIG: 'x', + VSCODE_PORTABLE: 'x', + VSCODE_PID: 'x', + VSCODE_NODE_CACHED_DATA_DIR_12345: 'x' + }; + assert.equal(env['FOO'], 'bar'); + assert.equal(Object.keys(env).length, 1); + }); suite('mergeEnvironments', () => { test('should add keys', () => { From c1e6c6486a60a8930f13bcfb46df2e6892c519ea Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 12 Jul 2018 06:21:21 -0700 Subject: [PATCH 12/14] Remove comment from import --- .../parts/terminal/test/node/terminalEnvironment.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts index 8c524e8324899..afe4004f3f1a3 100644 --- a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts +++ b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts @@ -9,7 +9,7 @@ import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/parts/terminal/node/terminalEnvironment'; import Uri from 'vs/base/common/uri'; import { IStringDictionary } from 'vs/base/common/collections'; -import { /*IShellLaunchConfig,*/ ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; +import { ITerminalConfigHelper } from 'vs/workbench/parts/terminal/common/terminal'; suite('Workbench - TerminalEnvironment', () => { test('addTerminalEnvironmentKeys', () => { From ccbbf3f0515f076ef74372f9ad56503c1f40fe4a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 12 Jul 2018 06:21:59 -0700 Subject: [PATCH 13/14] Remove TODO --- src/vs/workbench/parts/terminal/node/terminalProcess.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/parts/terminal/node/terminalProcess.ts b/src/vs/workbench/parts/terminal/node/terminalProcess.ts index 3a31266d9652b..5500c17791a0c 100644 --- a/src/vs/workbench/parts/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/parts/terminal/node/terminalProcess.ts @@ -93,7 +93,6 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { if (this._closeTimeout) { clearTimeout(this._closeTimeout); } - // TODO: Dispose correctly this._closeTimeout = setTimeout(() => { this._ptyProcess.kill(); this._onProcessExit.fire(this._exitCode); From 6dd7bdee7e0d525bbac7762b2d85f7413fa6f76a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 12 Jul 2018 08:53:34 -0700 Subject: [PATCH 14/14] Fix test --- .../parts/terminal/test/node/terminalEnvironment.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts index afe4004f3f1a3..1fad535bd47e0 100644 --- a/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts +++ b/src/vs/workbench/parts/terminal/test/node/terminalEnvironment.test.ts @@ -47,6 +47,7 @@ suite('Workbench - TerminalEnvironment', () => { VSCODE_PID: 'x', VSCODE_NODE_CACHED_DATA_DIR_12345: 'x' }; + terminalEnvironment.sanitizeEnvironment(env); assert.equal(env['FOO'], 'bar'); assert.equal(Object.keys(env).length, 1); });