Refactoring

src/vs/workbench/parts/terminal/common/terminal.ts

@@ -361,5 +361,5 @@ export interface ITerminalInstance {
 	/**
 	 * Sets the title of the terminal instance.
 	 */
-	setTitle(title: string): void;
+	setTitle(title: string, eventFromProcess: boolean): void;
 }

src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts

@@ -622,7 +622,7 @@ export class RenameTerminalAction extends Action {
 			prompt: nls.localize('workbench.action.terminal.rename.prompt', "Enter terminal name"),
 		}).then(name => {
 			if (name) {
-				terminalInstance.setTitle(name);
+				terminalInstance.setTitle(name, false);
 			}
 		});
 	}

src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts

@@ -112,7 +112,7 @@ export class TerminalInstance implements ITerminalInstance {
 		@IWorkbenchEditorService private _editorService: IWorkbenchEditorService,
 		@IInstantiationService private _instantiationService: IInstantiationService,
 		@IClipboardService private _clipboardService: IClipboardService,
-		@IHistoryService private _historyService: IHistoryService,
+		@IHistoryService private _historyService: IHistoryService
 	) {
 		this._instanceDisposables = [];
 		this._processDisposables = [];
@@ -143,11 +143,6 @@ export class TerminalInstance implements ITerminalInstance {
 		if (platform.isWindows) {
 			this._processReady.then(() => {
 				this._windowsShellHelper = new WindowsShellHelper(this._processId, this._shellLaunchConfig.executable);
-			}).then(() => {
-				this._windowsShellHelper.updateShellName().then(result => {
-					this._title = result;
-					this._onTitleChanged.fire(result);
-				});
 			});
 		}
 
@@ -278,11 +273,10 @@ export class TerminalInstance implements ITerminalInstance {
 				return false;
 			}
 
+			// Windows does not get a process title event from terminalProcess so we check the name on enter
+			// messageTitleListener is falsy when the API/user renames the terminal so we don't override it
 			if (platform.isWindows && event.keyCode === 13 /* ENTER */ && this._messageTitleListener) {
-				this._windowsShellHelper.updateShellName().then(result => {
-					this._title = result;
-					this._onTitleChanged.fire(result);
-				});
+				this._windowsShellHelper.getShellName().then(title => this.setTitle(title, true));
 			}
 
 			return undefined;
@@ -540,17 +534,17 @@ export class TerminalInstance implements ITerminalInstance {
 		const platformKey = platform.isWindows ? 'windows' : platform.isMacintosh ? 'osx' : 'linux';
 		const envFromConfig = { ...process.env, ...this._configHelper.config.env[platformKey] };
 		const env = TerminalInstance.createTerminalEnv(envFromConfig, shell, this._initialCwd, locale, this._cols, this._rows);
-		this._title = shell.name || '';
 		this._process = cp.fork(Uri.parse(require.toUrl('bootstrap')).fsPath, ['--type=terminal'], {
 			env,
 			cwd: Uri.parse(path.dirname(require.toUrl('../node/terminalProcess'))).fsPath
 		});
-		if (!shell.name) {
+		if (shell.name) {
+			this.setTitle(shell.name, false);
+		} else {
 			// Only listen for process title changes when a name is not provided
 			this._messageTitleListener = (message) => {
 				if (message.type === 'title') {
-					this._title = message.content ? message.content : '';
-					this._onTitleChanged.fire(this._title);
+					this.setTitle(message.content ? message.content : '', true);
 				}
 			};
 			this._process.on('message', this._messageTitleListener);
@@ -673,7 +667,7 @@ export class TerminalInstance implements ITerminalInstance {
 		const oldTitle = this._title;
 		this._createProcess(shell);
 		if (oldTitle !== this._title) {
-			this._onTitleChanged.fire(this._title);
+			this.setTitle(this._title, true);
 		}
 		this._process.on('message', (message) => this._sendPtyDataToXterm(message));
 
@@ -850,19 +844,25 @@ export class TerminalInstance implements ITerminalInstance {
 		this._terminalProcessFactory = factory;
 	}
 
-	public setTitle(title: string): void {
+	public setTitle(title: string, eventFromProcess: boolean): void {
+		if (eventFromProcess) {
+			if (platform.isWindows) {
+				// Remove the .exe extension
+				title = path.basename(title.split('.exe')[0]);
+			}
+		} else {
+			// If the title has not been set by the API or the rename command, unregister the handler that
+			// automatically updates the terminal name
+			if (this._process && this._messageTitleListener) {
+				this._process.removeListener('message', this._messageTitleListener);
+				this._messageTitleListener = null;
+			}
+		}
 		const didTitleChange = title !== this._title;
 		this._title = title;
 		if (didTitleChange) {
 			this._onTitleChanged.fire(title);
 		}
-
-		// If the title was not set by the API, unregister the handler that
-		// automatically updates the terminal name
-		if (this._process && this._messageTitleListener) {
-			this._process.removeListener('message', this._messageTitleListener);
-			this._messageTitleListener = null;
-		}
 	}
 }
 

src/vs/workbench/parts/terminal/electron-browser/windowsShellHelper.ts

@@ -9,16 +9,18 @@ import * as path from 'path';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { Emitter, debounceEvent } from 'vs/base/common/event';
 
+const SHELL_EXECUTABLES = ['cmd.exe', 'powershell.exe', 'bash.exe'];
+
 export class WindowsShellHelper {
 	private _childProcessIdStack: number[];
 	private _onCheckWindowsShell: Emitter<string>;
 	private _rootShellExecutable: string;
-	private _processId: number;
+	private _rootProcessId: number;
 
-	public constructor(pid: number, rootShellName: string) {
+	public constructor(rootProcessId: number, rootShellExecutable: string) {
 		this._childProcessIdStack = [];
-		this._rootShellExecutable = rootShellName;
-		this._processId = pid;
+		this._rootShellExecutable = rootShellExecutable;
+		this._rootProcessId = rootProcessId;
 
 		if (!platform.isWindows) {
 			throw new Error(`WindowsShellHelper cannot be instantiated on ${platform.platform}`);
@@ -26,11 +28,11 @@ export class WindowsShellHelper {
 
 		this._onCheckWindowsShell = new Emitter<string>();
 		debounceEvent(this._onCheckWindowsShell.event, (l, e) => e, 100, true)(() => {
-			this.updateShellName();
+			this.getShellName();
 		});
 	}
 
-	private getFirstChildProcess(pid: number): TPromise<{ executable: string, pid: number }[]> {
+	private getChildProcessDetails(pid: number): TPromise<{ executable: string, pid: number }[]> {
 		return new TPromise((resolve, reject) => {
 			cp.execFile('wmic.exe', ['process', 'where', `parentProcessId=${pid}`, 'get', 'ExecutablePath,ProcessId'], (err, stdout, stderr) => {
 				if (err) {
@@ -38,59 +40,55 @@ export class WindowsShellHelper {
 				} else if (stderr.length > 0) {
 					resolve([]); // No processes found
 				} else {
-					resolve(stdout.split('\n').slice(1).filter(str => !/^\s*$/.test(str)).map(str => {
+					const childProcessLines = stdout.split('\n').slice(1).filter(str => !/^\s*$/.test(str));
+					const childProcessDetails = childProcessLines.map(str => {
 						const s = str.split('  ');
 						return { executable: s[0], pid: Number(s[1]) };
-					}));
+					});
+					resolve(childProcessDetails);
 				}
 			});
 		});
 	}
 
 	private refreshShellProcessTree(pid: number, parent: string): TPromise<string> {
-		const shellExecutables = ['cmd.exe', 'powershell.exe', 'bash.exe'];
-		return this.getFirstChildProcess(pid).then(result => {
+		return this.getChildProcessDetails(pid).then(result => {
+			// When we didn't find any child processes of the process
 			if (result.length === 0) {
-				if (parent.length > 0) {
+				// Case where we found a child process already and are checking further down the pid tree
+				// We have reached the end here so we know that parent is the deepest first child of the tree
+				if (parent) {
 					return TPromise.as(parent);
 				}
-				if (this._childProcessIdStack.length > 1) {
-					this._childProcessIdStack.pop();
-					return this.refreshShellProcessTree(this._childProcessIdStack[this._childProcessIdStack.length - 1], '');
+				// Case where we haven't found a child and only the root shell is left
+				if (this._childProcessIdStack.length === 1) {
+					return TPromise.as(this._rootShellExecutable);
 				}
-				return TPromise.as([]);
-			} else if (shellExecutables.indexOf(path.basename(result[0].executable)) < 0){
+				// Otherwise, we go up the tree to find the next valid deepest child of the root
+				this._childProcessIdStack.pop();
+				return this.refreshShellProcessTree(this._childProcessIdStack[this._childProcessIdStack.length - 1], null);
+			}
+			// We only go one level deep when checking for children of processes other then shells
+			if (SHELL_EXECUTABLES.indexOf(path.basename(result[0].executable)) === -1) {
 				return TPromise.as(result[0].executable);
 			}
+			// Save the pid in the stack and keep looking for children of that child
 			this._childProcessIdStack.push(result[0].pid);
 			return this.refreshShellProcessTree(result[0].pid, result[0].executable);
 		}, error => { return error; });
 	}
 
 	/**
-	 * Returns the innermost shell running in the terminal.
+	 * Returns the innermost shell executable running in the terminal
 	 */
-	private getShellName(pid: number, shell: string): TPromise<string> {
+	public getShellName(): TPromise<string> {
 		if (this._childProcessIdStack.length === 0) {
-			this._childProcessIdStack.push(pid);
+			this._childProcessIdStack.push(this._rootProcessId);
 		}
 		return new TPromise<string>((resolve) => {
-			this.refreshShellProcessTree(this._childProcessIdStack[this._childProcessIdStack.length - 1], '').then(result => {
-				if (result.length > 0) {
-					resolve(result);
-				}
-				resolve(shell);
+			this.refreshShellProcessTree(this._childProcessIdStack[this._childProcessIdStack.length - 1], null).then(result => {
+				resolve(result);
 			}, error => { return error; });
 		});
 	}
-
-	public updateShellName(): TPromise<string> {
-		return this.getShellName(this._processId, this._rootShellExecutable).then(result => {
-			if (result) {
-				const fullPathName = result.split('.exe')[0];
-				return path.basename(fullPathName);
-			}
-			return this._rootShellExecutable;
-		});
-	}
 }
\ No newline at end of file