Implement shouldPersist for all terminals processes

shouldPersist was not being set on RemoteTerminalProcess, causing reconnect
to fail for all remote terminals. The fix was to push the persistent logic
up to where the process gets created in TerminalProcessManager just like is
done with local processes. The property was made non-optional to prevent
this sort of thing happening again.

Part of #117896
This commit is contained in:
Daniel Imms 2021-03-01 07:46:34 -08:00
parent 884ed1e3f3
commit edadf1c719
7 changed files with 13 additions and 7 deletions

View File

@ -255,7 +255,7 @@ export interface ITerminalChildProcess {
/**
* Whether the process should be persisted across reloads.
*/
shouldPersist?: boolean;
shouldPersist: boolean;
onProcessData: Event<IProcessDataEvent | string>;
onProcessExit: Event<number | undefined>;

View File

@ -26,6 +26,7 @@ const WRITE_INTERVAL_MS = 5;
export class TerminalProcess extends Disposable implements ITerminalChildProcess {
readonly id = 0;
readonly shouldPersist = false;
private _exitCode: number | undefined;
private _exitMessage: string | undefined;

View File

@ -185,6 +185,7 @@ export class ExtHostTerminal {
export class ExtHostPseudoterminal implements ITerminalChildProcess {
readonly id = 0;
readonly shouldPersist = false;
private readonly _onProcessData = new Emitter<string>();
public readonly onProcessData: Event<string> = this._onProcessData.event;

View File

@ -17,6 +17,7 @@ import { IRemoteTerminalProcessExecCommandEvent, IShellLaunchConfigDto, RemoteTe
import { IRemoteTerminalAttachTarget, ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { IProcessDataEvent, IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensionsOverride, ITerminalLaunchError, ITerminalsLayoutInfo, ITerminalsLayoutInfoById } from 'vs/platform/terminal/common/terminal';
export class RemoteTerminalService extends Disposable implements IRemoteTerminalService {
public _serviceBrand: undefined;
@ -39,7 +40,7 @@ export class RemoteTerminalService extends Disposable implements IRemoteTerminal
}
}
public async createRemoteTerminalProcess(terminalId: number, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI | undefined, cols: number, rows: number, configHelper: ITerminalConfigHelper,): Promise<ITerminalChildProcess> {
public async createRemoteTerminalProcess(terminalId: number, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI | undefined, cols: number, rows: number, shouldPersist: boolean, configHelper: ITerminalConfigHelper): Promise<ITerminalChildProcess> {
if (!this._remoteTerminalChannel) {
throw new Error(`Cannot create remote terminal when there is no remote!`);
}
@ -52,7 +53,7 @@ export class RemoteTerminalService extends Disposable implements IRemoteTerminal
});
}
return new RemoteTerminalProcess(terminalId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, configHelper, isPreconnectionTerminal, this._remoteTerminalChannel, this._remoteAgentService, this._logService, this._commandService);
return new RemoteTerminalProcess(terminalId, shouldPersist, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, configHelper, isPreconnectionTerminal, this._remoteTerminalChannel, this._remoteAgentService, this._logService, this._commandService);
}
public async listTerminals(isInitialization = false): Promise<IRemoteTerminalAttachTarget[]> {
@ -108,6 +109,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
constructor(
readonly id: number,
readonly shouldPersist: boolean,
private readonly _shellLaunchConfig: IShellLaunchConfig,
private readonly _activeWorkspaceRootUri: URI | undefined,
private readonly _cols: number,
@ -154,7 +156,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP
const result = await this._remoteTerminalChannel.createTerminalProcess(
shellLaunchConfigDto,
this._activeWorkspaceRootUri,
!this._shellLaunchConfig.isFeatureTerminal && this._configHelper.config.enablePersistentSessions,
this.shouldPersist,
this._cols,
this._rows,
isWorkspaceShellAllowed,

View File

@ -200,7 +200,7 @@ export interface IRemoteTerminalService {
readonly _serviceBrand: undefined;
dispose(): void;
listTerminals(isInitialization?: boolean): Promise<IRemoteTerminalAttachTarget[]>;
createRemoteTerminalProcess(terminalId: number, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI | undefined, cols: number, rows: number, configHelper: ITerminalConfigHelper,): Promise<ITerminalChildProcess>;
createRemoteTerminalProcess(terminalId: number, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI | undefined, cols: number, rows: number, shouldPersist: boolean, configHelper: ITerminalConfigHelper,): Promise<ITerminalChildProcess>;
setTerminalLayoutInfo(layout: ITerminalsLayoutInfoById): Promise<void>;
getTerminalLayoutInfo(): Promise<ITerminalsLayoutInfo | undefined>;

View File

@ -11,6 +11,7 @@ import { ITerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/comm
export class TerminalProcessExtHostProxy extends Disposable implements ITerminalChildProcess, ITerminalProcessExtHostProxy {
readonly id = 0;
readonly shouldPersist = false;
private readonly _onProcessData = this._register(new Emitter<string>());
public readonly onProcessData: Event<string> = this._onProcessData.event;

View File

@ -90,7 +90,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
public get environmentVariableInfo(): IEnvironmentVariableInfo | undefined { return this._environmentVariableInfo; }
public get persistentTerminalId(): number | undefined { return this._process?.id; }
public get shouldPersist(): boolean { return this._process?.shouldPersist || false; }
public get shouldPersist(): boolean { return this._process ? this._process.shouldPersist : false; }
public get hasWrittenData(): boolean {
return this._hasWrittenData;
@ -185,7 +185,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
// this is a copy of what the merged environment collection is on the remote side
await this._setupEnvVariableInfo(activeWorkspaceRootUri, shellLaunchConfig);
this._process = await this._remoteTerminalService.createRemoteTerminalProcess(this._terminalId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, this._configHelper);
const shouldPersist = !shellLaunchConfig.isFeatureTerminal && this._configHelper.config.enablePersistentSessions;
this._process = await this._remoteTerminalService.createRemoteTerminalProcess(this._terminalId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, shouldPersist, this._configHelper);
} else {
// Flow control is not needed for ptys hosted in the same process (ie. the electron
// renderer).