Don't reply terminal shell on error, dismiss terminal when exited

Fixes #6683 Fixes #6762
microsoft · May 25, 2016 · 4dbb657 · 4dbb657
1 parent 025009b
commit 4dbb657
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 16 deletions.
diff --git a/src/vs/workbench/parts/terminal/common/terminal.ts b/src/vs/workbench/parts/terminal/common/terminal.ts
@@ -51,5 +51,5 @@ export interface ITerminalConfiguration {
 export interface ITerminalService {
 	serviceId: ServiceIdentifier<any>;
 
-	show(): TPromise<any>;
+	toggle(): TPromise<any>;
 }
diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts
@@ -10,7 +10,7 @@ import {ITerminalService} from 'vs/workbench/parts/terminal/common/terminal';
 
 export class ToggleTerminalAction extends Action {
 
-	public static ID = 'workbench.action.terminal.toggleTerminal';
+	public static ID = 'workbench.action.terminal.toggle';
 	public static LABEL = nls.localize('toggleTerminal', "Toggle Integrated Terminal");
 
 	constructor(
@@ -21,6 +21,6 @@ export class ToggleTerminalAction extends Action {
 	}
 
 	public run(event?: any): TPromise<any> {
-		return this.terminalService.show();
+		return this.terminalService.toggle();
 	}
 }
diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalPanel.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalPanel.ts
@@ -17,7 +17,7 @@ import {IConfigurationService} from 'vs/platform/configuration/common/configurat
 import {IStringDictionary} from 'vs/base/common/collections';
 import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
 import {IWorkspaceContextService} from 'vs/platform/workspace/common/workspace';
-import {ITerminalConfiguration, TERMINAL_PANEL_ID} from 'vs/workbench/parts/terminal/common/terminal';
+import {ITerminalConfiguration, ITerminalService, TERMINAL_PANEL_ID} from 'vs/workbench/parts/terminal/common/terminal';
 import {Panel} from 'vs/workbench/browser/panel';
 import {DomScrollableElement} from 'vs/base/browser/ui/scrollbar/scrollableElement';
 import {ScrollbarVisibility} from 'vs/base/browser/ui/scrollbar/scrollableElementOptions';
@@ -36,7 +36,8 @@ export class TerminalPanel extends Panel {
 	constructor(
 		@IConfigurationService private configurationService: IConfigurationService,
 		@ITelemetryService telemetryService: ITelemetryService,
-		@IWorkspaceContextService private contextService: IWorkspaceContextService
+		@IWorkspaceContextService private contextService: IWorkspaceContextService,
+		@ITerminalService private terminalService: ITerminalService
 	) {
 		super(TERMINAL_PANEL_ID, telemetryService);
 		this.toDispose = [];
@@ -46,11 +47,13 @@ export class TerminalPanel extends Panel {
 		let cols = Math.floor(this.parentDomElement.offsetWidth / TERMINAL_CHAR_WIDTH);
 		let rows = Math.floor(this.parentDomElement.offsetHeight / TERMINAL_CHAR_HEIGHT);
 		this.terminal.resize(cols, rows);
-		this.ptyProcess.send({
-			event: 'resize',
-			cols: cols,
-			rows: rows
-		});
+		if (this.ptyProcess.connected) {
+			this.ptyProcess.send({
+				event: 'resize',
+				cols: cols,
+				rows: rows
+			});
+		}
 	}
 
 	public create(parent: Builder): TPromise<void> {
@@ -103,12 +106,19 @@ export class TerminalPanel extends Panel {
 				});
 				return false;
 			});
-			this.ptyProcess.on('exit', (data) => {
+			this.ptyProcess.on('exit', (exitCode) => {
+				this.toDispose = lifecycle.dispose(this.toDispose);
 				this.terminal.destroy();
 				// TODO: When multiple terminals are supported this should do something smarter. There is
 				// also a weird bug here at least on Ubuntu 15.10 where the new terminal text does not
 				// repaint correctly.
-				this.createTerminal();
+				if (exitCode === 0) {
+					this.createTerminal();
+				} else {
+					// TODO: Allow the terminal to be relaunched after an error
+					console.error('Integrated terminal exited with code ' + exitCode);
+				}
+				this.terminalService.toggle();
 			});
 			this.toDispose.push(DOM.addDisposableListener(this.parentDomElement, 'mousedown', (event) => {
 				// Drop selection and focus terminal on Linux to enable middle button paste when click

diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalProcess.js b/src/vs/workbench/parts/terminal/electron-browser/terminalProcess.js
@@ -17,9 +17,9 @@ ptyProcess.on('data', function (data) {
 	process.send(data);
 });
 
-ptyProcess.on('exit', function () {
-	process.exit(0);
-})
+ptyProcess.on('exit', function (exitCode) {
+	process.exit(exitCode);
+});
 
 process.on('message', function (message) {
 	if (message.event === 'input') {

diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts
@@ -17,7 +17,7 @@ export class TerminalService implements ITerminalService {
 	) {
 	}
 
-	public show(): TPromise<any> {
+	public toggle(): TPromise<any> {
 		const panel = this.panelService.getActivePanel();
 		if (panel && panel.getId() === TERMINAL_PANEL_ID) {
 			this.partService.setPanelHidden(true);