From b9f99bc75c7c44ec11e90296dc42bdf8c6706716 Mon Sep 17 00:00:00 2001 From: Amy Qiu Date: Wed, 19 Jul 2017 17:38:52 -0700 Subject: [PATCH] Refactoring --- .../parts/terminal/common/terminal.ts | 2 +- .../electron-browser/terminalActions.ts | 2 +- .../electron-browser/terminalInstance.ts | 46 ++++++------- .../electron-browser/windowsShellHelper.ts | 66 +++++++++---------- 4 files changed, 57 insertions(+), 59 deletions(-) diff --git a/src/vs/workbench/parts/terminal/common/terminal.ts b/src/vs/workbench/parts/terminal/common/terminal.ts index 3d03cfbba78..d3ef8db64d2 100644 --- a/src/vs/workbench/parts/terminal/common/terminal.ts +++ b/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; } diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts index 3b5b38af6be..4d3632ba2d0 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts +++ b/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); } }); } diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index 583aafa7bc8..53755c84f48 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/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; - } } } diff --git a/src/vs/workbench/parts/terminal/electron-browser/windowsShellHelper.ts b/src/vs/workbench/parts/terminal/electron-browser/windowsShellHelper.ts index 3973ecb197b..ab5f254324d 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/windowsShellHelper.ts +++ b/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; 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(); 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 { - 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 { + public getShellName(): TPromise { if (this._childProcessIdStack.length === 0) { - this._childProcessIdStack.push(pid); + this._childProcessIdStack.push(this._rootProcessId); } return new TPromise((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 { - 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 -- GitLab