Skip to content

Commit

Permalink
Merge pull request #118574 from microsoft/tyriar/100709
Browse files Browse the repository at this point in the history 
Make terminal relaunch logic more internal to the process manager
  • Loading branch information
@Tyriar
Tyriar committed Mar 11, 2021
2 parents b92b610 + 5afac65 commit 1ad8c46
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 52 deletions.
4 changes: 1 addition & 3 deletions src/vs/workbench/contrib/terminal/browser/terminalActions.ts
View file
Expand Up @@ -357,9 +357,6 @@ export function registerTerminalActions() {
const codeEditorService = accessor.get(ICodeEditorService);
const notificationService = accessor.get(INotificationService);

const instance = terminalService.getActiveOrCreateInstance();
await instance.processReady;

const editor = codeEditorService.getActiveCodeEditor();
if (!editor || !editor.hasModel()) {
return;
Expand All @@ -372,6 +369,7 @@ export function registerTerminalActions() {
}

// TODO: Convert this to ctrl+c, ctrl+v for pwsh?
const instance = terminalService.getActiveOrCreateInstance();
const path = await terminalService.preparePathForTerminalAsync(uri.fsPath, instance.shellLaunchConfig.executable, instance.title, instance.shellType);
instance.sendText(path, true);
return terminalService.showPanel();
Expand Down
56 changes: 22 additions & 34 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View file
Expand Up @@ -859,8 +859,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}

// Send it to the process
await this._processManager.ptyProcessReady;
this._processManager.write(text);
return this._processManager.write(text);
}

public setVisible(visible: boolean): void {
Expand Down Expand Up @@ -938,24 +937,21 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
this._processManager.onProcessOverrideDimensions(e => this.setDimensions(e, true));
this._processManager.onProcessResolvedShellLaunchConfig(e => this._setResolvedShellLaunchConfig(e));
this._processManager.onEnvironmentVariableInfoChanged(e => this._onEnvironmentVariableInfoChanged(e));
this._processManager.onPtyDisconnect(() => {
this._safeSetOption('disableStdin', true);
this._onTitleChanged.fire(this);
});
this._processManager.onPtyReconnect(() => {
this._safeSetOption('disableStdin', false);
this._onTitleChanged.fire(this);
});

this._processManager.onProcessShellTypeChanged(type => this.setShellType(type));
if (this._shellLaunchConfig.name) {
this.setTitle(this._shellLaunchConfig.name, TitleEventSource.Api);
} else {
// Only listen for process title changes when a name is not provided
if (this._configHelper.config.experimentalUseTitleEvent) {
this._processManager.ptyProcessReady.then(() => {
this._terminalInstanceService.getDefaultShellAndArgs(false).then(e => {
this.setTitle(e.shell, TitleEventSource.Sequence);
});
// Set the title to the first event if the sequence hasn't set it yet
Event.once(this._processManager.onProcessTitle)(e => {
if (!this._title) {
this.setTitle(this._title, TitleEventSource.Sequence);
}
});
// Listen to xterm.js' sequence title change event, trigger this async to ensure
// xterm is constructed since this is called from TerminalInstance's ctor
setTimeout(() => {
this._xtermReadyPromise.then(xterm => {
this._messageTitleDisposable = xterm.onTitleChange(e => this._onTitleChange(e));
});
Expand All @@ -965,8 +961,14 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
this._messageTitleDisposable = this._processManager.onProcessTitle(title => this.setTitle(title ? title : '', TitleEventSource.Process));
}
}
this._processManager.onProcessShellTypeChanged(type => {
this.setShellType(type);

this._processManager.onPtyDisconnect(() => {
this._safeSetOption('disableStdin', true);
this._onTitleChanged.fire(this);
});
this._processManager.onPtyReconnect(() => {
this._safeSetOption('disableStdin', false);
this._onTitleChanged.fire(this);
});
}

Expand Down Expand Up @@ -1166,20 +1168,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
// Set the new shell launch config
this._shellLaunchConfig = shell; // Must be done before calling _createProcess()

// Kill and clear up the process, making the process manager ready for a new process
this._processManager.dispose();

// Launch the process unless this is only a renderer.
// In the renderer only cases, we still need to set the title correctly.
const oldTitle = this._title;
this._createProcessManager();

if (oldTitle !== this._title) {
this.setTitle(this._title, TitleEventSource.Process);
}

this._processManager.onProcessData(data => this._onProcessData(data));
this._createProcess();
this._processManager.relaunch(this._shellLaunchConfig, this._cols, this._rows, this._accessibilityService.isScreenReaderOptimized());

this._xtermTypeAhead?.reset(this._processManager);
}
Expand Down Expand Up @@ -1456,10 +1445,9 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {

if (immediate) {
// do not await, call setDimensions synchronously
this._processManager.setDimensions(cols, rows);
this._processManager.setDimensions(cols, rows, true);
} else {
await this._processManager.ptyProcessReady;
this._processManager.setDimensions(cols, rows);
await this._processManager.setDimensions(cols, rows);
}
}

Expand Down
53 changes: 39 additions & 14 deletions src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts
View file
Expand Up @@ -58,6 +58,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
public os: platform.OperatingSystem | undefined;
public userHome: string | undefined;
public isDisconnected: boolean = false;
public environmentVariableInfo: IEnvironmentVariableInfo | undefined;

private _isDisposed: boolean = false;
private _process: ITerminalChildProcess | null = null;
Expand All @@ -67,10 +68,10 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
private _latencyLastMeasured: number = 0;
private _initialCwd: string | undefined;
private _extEnvironmentVariableCollection: IMergedEnvironmentVariableCollection | undefined;
private _environmentVariableInfo: IEnvironmentVariableInfo | undefined;
private _ackDataBufferer: AckDataBufferer;
private _hasWrittenData: boolean = false;
private _ptyResponsiveListener: IDisposable | undefined;
private _ptyListenersAttached: boolean = false;

private readonly _onPtyDisconnect = this._register(new Emitter<void>());
public get onPtyDisconnect(): Event<void> { return this._onPtyDisconnect.event; }
Expand All @@ -96,13 +97,9 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
private readonly _onEnvironmentVariableInfoChange = this._register(new Emitter<IEnvironmentVariableInfo>());
public get onEnvironmentVariableInfoChanged(): Event<IEnvironmentVariableInfo> { return this._onEnvironmentVariableInfoChange.event; }

public get environmentVariableInfo(): IEnvironmentVariableInfo | undefined { return this._environmentVariableInfo; }
public get persistentProcessId(): number | undefined { return this._process?.id; }
public get shouldPersist(): boolean { return this._process ? this._process.shouldPersist : false; }

public get hasWrittenData(): boolean {
return this._hasWrittenData;
}
public get hasWrittenData(): boolean { return this._hasWrittenData; }

private readonly _localTerminalService?: ILocalTerminalService;

Expand Down Expand Up @@ -133,7 +130,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
c(undefined);
});
});
this.ptyProcessReady.then(async () => await this.getLatency());
this.getLatency();
this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e));
}

Expand Down Expand Up @@ -288,6 +285,16 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
return undefined;
}

public async relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise<ITerminalLaunchError | undefined> {
this.ptyProcessReady = new Promise<void>(c => {
this.onProcessReady(() => {
this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`);
c(undefined);
});
});
return this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled);
}

// Fetch any extension environment additions and apply them
private async _setupEnvVariableInfo(activeWorkspaceRootUri: URI | undefined, shellLaunchConfig: IShellLaunchConfig): Promise<platform.IProcessEnvironment> {
const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux');
Expand All @@ -310,8 +317,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
// if it happens - it's not worth adding plumbing to sync back the resolved collection.
this._extEnvironmentVariableCollection.applyToProcessEnvironment(env, variableResolver);
if (this._extEnvironmentVariableCollection.map.size > 0) {
this._environmentVariableInfo = new EnvironmentVariableInfoChangesActive(this._extEnvironmentVariableCollection);
this._onEnvironmentVariableInfoChange.fire(this._environmentVariableInfo);
this.environmentVariableInfo = new EnvironmentVariableInfoChangesActive(this._extEnvironmentVariableCollection);
this._onEnvironmentVariableInfoChange.fire(this.environmentVariableInfo);
}
}
return env;
Expand Down Expand Up @@ -364,6 +371,11 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}

private _setupPtyHostListeners(offProcessTerminalService: IOffProcessTerminalService) {
if (this._ptyListenersAttached) {
return;
}
this._ptyListenersAttached = true;

// Mark the process as disconnected is the pty host is unresponsive, the responsive event
// will fire only when the pty host was already unresponsive
this._register(offProcessTerminalService.onPtyHostUnresponsive(() => {
Expand All @@ -389,14 +401,26 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}));
}

public setDimensions(cols: number, rows: number): void {
public setDimensions(cols: number, rows: number): Promise<void>;
public setDimensions(cols: number, rows: number, sync: false): Promise<void>;
public setDimensions(cols: number, rows: number, sync: true): void;
public setDimensions(cols: number, rows: number, sync?: boolean): Promise<void> | void {
if (!this._process) {
return;
}

if (sync) {
this._resize(cols, rows);
return;
}

return this.ptyProcessReady.then(() => this._resize(cols, rows));
}

private _resize(cols: number, rows: number) {
// The child process could already be terminated
try {
this._process.resize(cols, rows);
this._process!.resize(cols, rows);
} catch (error) {
// We tried to write to a closed pipe / channel.
if (error.code !== 'EPIPE' && error.code !== 'ERR_IPC_CHANNEL_CLOSED') {
Expand All @@ -405,7 +429,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}
}

public write(data: string): void {
public async write(data: string): Promise<void> {
await this.ptyProcessReady;
this._hasWrittenData = true;
if (this.shellProcessId || this._processType === ProcessType.ExtensionTerminal) {
if (this._process) {
Expand Down Expand Up @@ -470,8 +495,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
if (diff === undefined) {
return;
}
this._environmentVariableInfo = this._instantiationService.createInstance(EnvironmentVariableInfoStale, diff, this._instanceId);
this._onEnvironmentVariableInfoChange.fire(this._environmentVariableInfo);
this.environmentVariableInfo = this._instantiationService.createInstance(EnvironmentVariableInfoStale, diff, this._instanceId);
this._onEnvironmentVariableInfoChange.fire(this.environmentVariableInfo);
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/vs/workbench/contrib/terminal/common/terminal.ts
View file
Expand Up @@ -262,8 +262,11 @@ export interface ITerminalProcessManager extends IDisposable {
dispose(immediate?: boolean): void;
detachFromProcess(): void;
createProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise<ITerminalLaunchError | undefined>;
relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise<ITerminalLaunchError | undefined>;
write(data: string): void;
setDimensions(cols: number, rows: number): void;
setDimensions(cols: number, rows: number): Promise<void>;
setDimensions(cols: number, rows: number, sync: false): Promise<void>;
setDimensions(cols: number, rows: number, sync: true): void;
acknowledgeDataEvent(charCount: number): void;

getInitialCwd(): Promise<string>;
Expand Down

0 comments on commit 1ad8c46

Please sign in to comment.