From d545ed4decde0f326913352758b50fbf2addaec8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 1 May 2019 11:21:32 -0700 Subject: [PATCH] Simplify code, make sure error message shows up --- .../terminal/browser/terminalInstance.ts | 57 ++++++------ .../contrib/terminal/node/terminalProcess.ts | 90 +++++++++---------- 2 files changed, 72 insertions(+), 75 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 3b242dde6bf..1fe700d0ad5 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -970,14 +970,32 @@ export class TerminalInstance implements ITerminalInstance { } this._isExiting = true; - let exitCodeMessage: string; + let exitCodeMessage: string | undefined; + // Create exit code message if (exitCode) { - exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); - } - - if (exitCode === SHELL_PATH_INVALID_EXIT_CODE) { - exitCodeMessage = nls.localize('terminal.integrated.exitedWithInvalidPath', 'The terminal process terminated as it could not find the specified path : {0}', this._shellLaunchConfig.executable); + if (exitCode === SHELL_PATH_INVALID_EXIT_CODE) { + exitCodeMessage = nls.localize('terminal.integrated.exitedWithInvalidPath', 'The terminal shell path does not exist: {0}', this._shellLaunchConfig.executable); + } else if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { + let args = ''; + if (typeof this._shellLaunchConfig.args === 'string') { + args = ` ${this._shellLaunchConfig.args}`; + } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { + args = ' ' + this._shellLaunchConfig.args.map(a => { + if (typeof a === 'string' && a.indexOf(' ') !== -1) { + return `'${a}'`; + } + return a; + }).join(' '); + } + if (this._shellLaunchConfig.executable) { + exitCodeMessage = nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode); + } else { + exitCodeMessage = nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode); + } + } else { + exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); + } } this._logService.debug(`Terminal process exit (id: ${this.id})${this._processManager ? ' state ' + this._processManager.processState : ''}`); @@ -985,8 +1003,8 @@ export class TerminalInstance implements ITerminalInstance { // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). if (this._shellLaunchConfig.waitOnExit && (!this._processManager || this._processManager.processState !== ProcessState.KILLED_BY_USER)) { - if (exitCode) { - this._xterm.writeln(exitCodeMessage!); + if (exitCodeMessage) { + this._xterm.writeln(exitCodeMessage); } if (typeof this._shellLaunchConfig.waitOnExit === 'string') { let message = this._shellLaunchConfig.waitOnExit; @@ -1001,29 +1019,14 @@ export class TerminalInstance implements ITerminalInstance { } } else { this.dispose(); - if (exitCode) { + if (exitCodeMessage) { if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { - let args = ''; - if (typeof this._shellLaunchConfig.args === 'string') { - args = ` ${this._shellLaunchConfig.args}`; - } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { - args = ' ' + this._shellLaunchConfig.args.map(a => { - if (typeof a === 'string' && a.indexOf(' ') !== -1) { - return `'${a}'`; - } - return a; - }).join(' '); - } - if (this._shellLaunchConfig.executable) { - this._notificationService.error(nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode)); - } else { - this._notificationService.error(nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode)); - } + this._notificationService.error(exitCodeMessage); } else { if (this._configHelper.config.showExitAlert) { - this._notificationService.error(exitCodeMessage!); + this._notificationService.error(exitCodeMessage); } else { - console.warn(exitCodeMessage!); + console.warn(exitCodeMessage); } } } diff --git a/src/vs/workbench/contrib/terminal/node/terminalProcess.ts b/src/vs/workbench/contrib/terminal/node/terminalProcess.ts index ef7600b5965..cfcd7826dcb 100644 --- a/src/vs/workbench/contrib/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/contrib/terminal/node/terminalProcess.ts @@ -69,8 +69,7 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { experimentalUseConpty: useConpty }; - const filePath = path.basename(shellLaunchConfig.executable!); - fs.stat(filePath, (err, stats) => { + fs.stat(shellLaunchConfig.executable!, (err) => { if (err && err.code === 'ENOENT') { this._exitCode = SHELL_PATH_INVALID_EXIT_CODE; this._queueProcessExit(); @@ -82,29 +81,26 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { } private setupPtyProcess(shellLaunchConfig: IShellLaunchConfig, options: pty.IPtyForkOptions): void { - this._ptyProcess = pty.spawn(shellLaunchConfig.executable!, shellLaunchConfig.args || [], options); + const ptyProcess = pty.spawn(shellLaunchConfig.executable!, shellLaunchConfig.args || [], options); + this._ptyProcess = ptyProcess; this._processStartupComplete = new Promise(c => { - this.onProcessIdReady((pid) => { - c(); - }); + this.onProcessIdReady(() => c()); }); - if (this._ptyProcess) { - this._ptyProcess.on('data', (data) => { - this._onProcessData.fire(data); - if (this._closeTimeout) { - clearTimeout(this._closeTimeout); - this._queueProcessExit(); - } - }); - this._ptyProcess.on('exit', (code) => { - this._exitCode = code; + ptyProcess.on('data', (data) => { + this._onProcessData.fire(data); + if (this._closeTimeout) { + clearTimeout(this._closeTimeout); this._queueProcessExit(); - }); - this._setupTitlePolling(this._ptyProcess); - } + } + }); + ptyProcess.on('exit', (code) => { + this._exitCode = code; + this._queueProcessExit(); + }); + this._setupTitlePolling(ptyProcess); // TODO: We should no longer need to delay this since pty.spawn is sync setTimeout(() => { - this._sendProcessId(); + this._sendProcessId(ptyProcess); }, 500); } @@ -123,12 +119,12 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { private _setupTitlePolling(ptyProcess: pty.IPty) { // Send initial timeout async to give event listeners a chance to init setTimeout(() => { - this._sendProcessTitle(); + this._sendProcessTitle(ptyProcess); }, 0); // Setup polling this._titleInterval = setInterval(() => { if (this._currentTitle !== ptyProcess.process) { - this._sendProcessTitle(); + this._sendProcessTitle(ptyProcess); } }, 200); } @@ -163,20 +159,16 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { }); } - private _sendProcessId() { - if (this._ptyProcess) { - this._onProcessIdReady.fire(this._ptyProcess.pid); - } + private _sendProcessId(ptyProcess: pty.IPty) { + this._onProcessIdReady.fire(ptyProcess.pid); } - private _sendProcessTitle(): void { + private _sendProcessTitle(ptyProcess: pty.IPty): void { if (this._isDisposed) { return; } - if (this._ptyProcess) { - this._currentTitle = this._ptyProcess.process; - this._onProcessTitleChanged.fire(this._currentTitle); - } + this._currentTitle = ptyProcess.process; + this._onProcessTitleChanged.fire(this._currentTitle); } public shutdown(immediate: boolean): void { @@ -188,12 +180,10 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { } public input(data: string): void { - if (this._isDisposed) { + if (this._isDisposed || !this._ptyProcess) { return; } - if (this._ptyProcess) { - this._ptyProcess.write(data); - } + this._ptyProcess.write(data); } public resize(cols: number, rows: number): void { @@ -214,26 +204,30 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable { public getCwd(): Promise { if (platform.isMacintosh) { return new Promise(resolve => { - if (this._ptyProcess) { - exec('lsof -p ' + this._ptyProcess.pid + ' | grep cwd', (error, stdout, stderr) => { - if (stdout !== '') { - resolve(stdout.substring(stdout.indexOf('/'), stdout.length - 1)); - } - }); + if (!this._ptyProcess) { + resolve(this._initialCwd); + return; } + exec('lsof -p ' + this._ptyProcess.pid + ' | grep cwd', (error, stdout, stderr) => { + if (stdout !== '') { + resolve(stdout.substring(stdout.indexOf('/'), stdout.length - 1)); + } + }); }); } if (platform.isLinux) { return new Promise(resolve => { - if (this._ptyProcess) { - fs.readlink('/proc/' + this._ptyProcess.pid + '/cwd', (err, linkedstr) => { - if (err) { - resolve(this._initialCwd); - } - resolve(linkedstr); - }); + if (!this._ptyProcess) { + resolve(this._initialCwd); + return; } + fs.readlink('/proc/' + this._ptyProcess.pid + '/cwd', (err, linkedstr) => { + if (err) { + resolve(this._initialCwd); + } + resolve(linkedstr); + }); }); }