From 8f8321d638dab9ec3edf5391f91f39c33ad1d5fa Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 11 Mar 2021 08:48:07 -0800 Subject: [PATCH 1/6] Smooth relaunch poc --- .../browser/terminalProcessManager.ts | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 6921fc2911c44..ee772dcecc106 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -28,6 +28,8 @@ import { IPathService } from 'vs/workbench/services/path/common/pathService'; import { URI } from 'vs/base/common/uri'; import { IEnvironmentVariableInfo, IEnvironmentVariableService, IMergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { IProcessDataEvent, IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensionsOverride, ITerminalEnvironment, ITerminalLaunchError, FlowControlConstants, TerminalShellType, ILocalTerminalService, IOffProcessTerminalService } from 'vs/platform/terminal/common/terminal'; +import { TerminalRecorder } from 'vs/platform/terminal/common/terminalRecorder'; +import { timeout } from 'vs/base/common/async'; /** The amount of time to consider terminal errors to be related to the launch */ const LAUNCHING_DURATION = 500; @@ -72,6 +74,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce private _hasWrittenData: boolean = false; private _ptyResponsiveListener: IDisposable | undefined; private _ptyListenersAttached: boolean = false; + private _relaunchRecorder: TerminalRecorder | undefined; private readonly _onPtyDisconnect = this._register(new Emitter()); public get onPtyDisconnect(): Event { return this._onPtyDisconnect.event; } @@ -132,6 +135,15 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce }); this.getLatency(); this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e)); + // TODO: Fix resize - we don't want to consider these events + this._relaunchRecorder = new TerminalRecorder(80, 30); + const relaunchDisposable = this.onProcessData(e => this._relaunchRecorder?.recordData('data' in e ? e.data : e)); + // setTimeout(() => { + // const replay = this._relaunchRecorder?.generateReplayEvent(); + // const dataEvents = replay?.events.filter(e => !!e.data); + // console.log('data events', dataEvents); + // relaunchDisposable.dispose(); + // }, 5000); } public dispose(immediate: boolean = false): void { @@ -248,6 +260,10 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce } }); + // TODO: Fix resize - we don't want to consider these events + this._relaunchRecorder = new TerminalRecorder(80, 30); + const relaunchDisposable = this.onProcessData(e => this._relaunchRecorder?.recordData('data' in e ? e.data : e)); + this._process.onProcessReady((e: { pid: number, cwd: string }) => { this.shellProcessId = e.pid; this._initialCwd = e.cwd; @@ -292,7 +308,34 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce c(undefined); }); }); - return this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled); + console.log('relaunch!'); + const lastRecorder = this._relaunchRecorder; + const process = this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled); + if (!lastRecorder) { + return process; + } + + await timeout(3000); + + if (!this._relaunchRecorder) { + return process; + } + const firstReplay = lastRecorder.generateReplayEvent(); + const firstData = firstReplay.events.filter(e => !!e.data).map(e => e.data).join(''); + const secondReplay = this._relaunchRecorder.generateReplayEvent(); + const secondData = secondReplay.events.filter(e => !!e.data).map(e => e.data).join(''); + + console.log('first data', firstData); + console.log('secondReplay events', secondReplay.events); + console.log('second data', secondData); + console.log('equal?', firstData === secondData); + if (firstData !== secondData) { + console.log('not equal, replaying instantly after 2 seconds'); + await timeout(2000); + // Fire full reset (RIS) followed by the new data so the update happens in the same frame + this._onProcessData.fire({ data: `\x1bc${secondData}2`, sync: false }); + } + return process; } // Fetch any extension environment additions and apply them From 569aedce01485862be1991d99c0dad343dcf3ad5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 12 Mar 2021 08:21:57 -0800 Subject: [PATCH 2/6] Mostly working seamless relaunch --- .../terminal/browser/terminalInstance.ts | 8 +- .../browser/terminalProcessManager.ts | 266 ++++++++++++++---- .../contrib/terminal/common/terminal.ts | 2 +- 3 files changed, 217 insertions(+), 59 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index ec9401662faba..22fe634fc772d 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -1127,16 +1127,14 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } } - @debounce(500) + @debounce(1000) public reuseTerminal(shell: IShellLaunchConfig, reset: boolean = false): void { // Unsubscribe any key listener we may have. this._pressAnyKeyToCloseListener?.dispose(); this._pressAnyKeyToCloseListener = undefined; if (this._xterm) { - if (reset) { - this._xterm.reset(); - } else { + if (!reset) { // Ensure new processes' output starts at start of new line this._xterm.write('\n\x1b[G'); } @@ -1168,7 +1166,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { // Set the new shell launch config this._shellLaunchConfig = shell; // Must be done before calling _createProcess() - this._processManager.relaunch(this._shellLaunchConfig, this._cols, this._rows, this._accessibilityService.isScreenReaderOptimized()); + this._processManager.relaunch(this._shellLaunchConfig, this._cols, this._rows, this._accessibilityService.isScreenReaderOptimized(), reset); this._xtermTypeAhead?.reset(this._processManager); } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index ee772dcecc106..4508757d33717 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -29,7 +29,6 @@ import { URI } from 'vs/base/common/uri'; import { IEnvironmentVariableInfo, IEnvironmentVariableService, IMergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { IProcessDataEvent, IShellLaunchConfig, ITerminalChildProcess, ITerminalDimensionsOverride, ITerminalEnvironment, ITerminalLaunchError, FlowControlConstants, TerminalShellType, ILocalTerminalService, IOffProcessTerminalService } from 'vs/platform/terminal/common/terminal'; import { TerminalRecorder } from 'vs/platform/terminal/common/terminalRecorder'; -import { timeout } from 'vs/base/common/async'; /** The amount of time to consider terminal errors to be related to the launch */ const LAUNCHING_DURATION = 500; @@ -74,7 +73,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce private _hasWrittenData: boolean = false; private _ptyResponsiveListener: IDisposable | undefined; private _ptyListenersAttached: boolean = false; - private _relaunchRecorder: TerminalRecorder | undefined; + private _dataFilter: SeamlessRelaunchDataFilter; private readonly _onPtyDisconnect = this._register(new Emitter()); public get onPtyDisconnect(): Event { return this._onPtyDisconnect.event; } @@ -135,15 +134,16 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce }); this.getLatency(); this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e)); - // TODO: Fix resize - we don't want to consider these events - this._relaunchRecorder = new TerminalRecorder(80, 30); - const relaunchDisposable = this.onProcessData(e => this._relaunchRecorder?.recordData('data' in e ? e.data : e)); - // setTimeout(() => { - // const replay = this._relaunchRecorder?.generateReplayEvent(); - // const dataEvents = replay?.events.filter(e => !!e.data); - // console.log('data events', dataEvents); - // relaunchDisposable.dispose(); - // }, 5000); + this._dataFilter = this._instantiationService.createInstance(SeamlessRelaunchDataFilter); + this._dataFilter.onProcessData(ev => { + const data = (typeof ev === 'string' ? ev : ev.data); + const sync = (typeof ev === 'string' ? false : ev.sync); + const beforeProcessDataEvent: IBeforeProcessDataEvent = { data }; + this._onBeforeProcessData.fire(beforeProcessDataEvent); + if (beforeProcessDataEvent.data && beforeProcessDataEvent.data.length > 0) { + this._onProcessData.fire({ data: beforeProcessDataEvent.data, sync }); + } + }); } public dispose(immediate: boolean = false): void { @@ -169,7 +169,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, - isScreenReaderModeEnabled: boolean + isScreenReaderModeEnabled: boolean, + reset: boolean = true ): Promise { if (shellLaunchConfig.isExtensionCustomPtyTerminal) { this._processType = ProcessType.ExtensionTerminal; @@ -250,19 +251,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce this.processState = ProcessState.LAUNCHING; - this._process.onProcessData(ev => { - const data = (typeof ev === 'string' ? ev : ev.data); - const sync = (typeof ev === 'string' ? false : ev.sync); - const beforeProcessDataEvent: IBeforeProcessDataEvent = { data }; - this._onBeforeProcessData.fire(beforeProcessDataEvent); - if (beforeProcessDataEvent.data && beforeProcessDataEvent.data.length > 0) { - this._onProcessData.fire({ data: beforeProcessDataEvent.data, sync }); - } - }); - - // TODO: Fix resize - we don't want to consider these events - this._relaunchRecorder = new TerminalRecorder(80, 30); - const relaunchDisposable = this.onProcessData(e => this._relaunchRecorder?.recordData('data' in e ? e.data : e)); + this._dataFilter.newProcess(this._process, reset); this._process.onProcessReady((e: { pid: number, cwd: string }) => { this.shellProcessId = e.pid; @@ -301,7 +290,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce return undefined; } - public async relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise { + public async relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean, reset: boolean): Promise { + console.log('time', performance.now()); this.ptyProcessReady = new Promise(c => { this.onProcessReady(() => { this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); @@ -309,35 +299,48 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce }); }); console.log('relaunch!'); - const lastRecorder = this._relaunchRecorder; - const process = this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled); - if (!lastRecorder) { - return process; - } - - await timeout(3000); - - if (!this._relaunchRecorder) { - return process; - } - const firstReplay = lastRecorder.generateReplayEvent(); - const firstData = firstReplay.events.filter(e => !!e.data).map(e => e.data).join(''); - const secondReplay = this._relaunchRecorder.generateReplayEvent(); - const secondData = secondReplay.events.filter(e => !!e.data).map(e => e.data).join(''); - - console.log('first data', firstData); - console.log('secondReplay events', secondReplay.events); - console.log('second data', secondData); - console.log('equal?', firstData === secondData); - if (firstData !== secondData) { - console.log('not equal, replaying instantly after 2 seconds'); - await timeout(2000); - // Fire full reset (RIS) followed by the new data so the update happens in the same frame - this._onProcessData.fire({ data: `\x1bc${secondData}2`, sync: false }); - } - return process; + // this._relaunchDisposable?.dispose(); + // this._relaunchDisposable = undefined; + return this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled, reset); + + // if (!reset || !this._relaunchRecorder) { + // return process; + // } + // const initialData = this._getDataFromRecorder(this._relaunchRecorder); + + // await timeout(3000); + // this._swapProcesses(initialData); + // return process; } + // private _getDataFromRecorder(recorder: TerminalRecorder): string { + // return recorder.generateReplayEvent().events.filter(e => !!e.data).map(e => e.data).join(''); + // } + + // private _swapProcesses(initialData: string) { + // // TODO: Trigger immediately on input + // // TODO: Is this right? What can unset this? + // if (!this._relaunchRecorder) { + // return process; + // } + // const secondReplay = this._relaunchRecorder.generateReplayEvent(); + // const secondData = secondReplay.events.filter(e => !!e.data).map(e => e.data).join(''); + + // console.log('first data', initialData); + // console.log('secondReplay events', secondReplay.events); + // console.log('second data', secondData); + // console.log('equal?', initialData === secondData); + // // Re-write the terminal if the data differs + // if (initialData === secondData) { + // this._logService.trace(`Relaunching terminal "${this._instanceId}" to update environment - identical content`); + // } else { + // this._logService.trace(`Relaunching terminal "${this._instanceId}" to update environment - resetting content`); + // // Fire full reset (RIS) followed by the new data so the update happens in the same frame + // this._onProcessData.fire({ data: `\x1bc${secondData}`, sync: false }); + // } + // return process; + // } + // Fetch any extension environment additions and apply them private async _setupEnvVariableInfo(activeWorkspaceRootUri: URI | undefined, shellLaunchConfig: IShellLaunchConfig): Promise { const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); @@ -474,6 +477,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce public async write(data: string): Promise { await this.ptyProcessReady; + this._dataFilter.triggerSwap(); this._hasWrittenData = true; if (this.shellProcessId || this._processType === ProcessType.ExtensionTerminal) { if (this._process) { @@ -559,3 +563,159 @@ class AckDataBufferer { } } } + +const enum SeamlessRelaunchConstants { + /** + * How long to record data events for new terminals. + */ + RecordTerminalDuration = 10000, + /** + * The maximum duration after a relaunch occurs to trigger a swap. + */ + SwapWaitMaximumDuration = 3000 + + // TODO: Need 2 times: How long to hold onto the first recorder for and how long to wait until diffing the 2 procs +} + +/** + * Filters data events from the process and supports seamlessly restarting swapping out the process + * with another, delaying the swap in output in order to minimize flickering/clearing of the + * terminal. + */ +class SeamlessRelaunchDataFilter extends Disposable { + private _firstRecorder?: TerminalRecorder; + private _secondRecorder?: TerminalRecorder; + private _firstDisposable?: IDisposable; + private _secondDisposable?: IDisposable; + private _dataListener?: IDisposable; + private _activeProcess?: ITerminalChildProcess; + + private _recordingTimeout?: number; + private _swapTimeout?: number; + + private readonly _onProcessData = this._register(new Emitter()); + public get onProcessData(): Event { return this._onProcessData.event; } + + constructor( + @ILogService private readonly _logService: ILogService + ) { + super(); + } + + newProcess(process: ITerminalChildProcess, reset: boolean) { + // TODO: Disable recorder for reconnected terminals + this._activeProcess = process; + + // Stop listening to the old process + this._dataListener?.dispose(); + + // If the process is new, relaunch has timed out or the terminal should not reset, start + // listening and firing data events immediately + console.log('relaunch newProcess', this._firstRecorder, reset); + if (!this._firstRecorder || !reset) { + this._firstDisposable?.dispose(); + [this._firstRecorder, this._firstDisposable] = this._createRecorder(process); + this._dataListener = process.onProcessData(e => this._onProcessData.fire(e)); + if (this._recordingTimeout) { + window.clearTimeout(this._recordingTimeout); + } + this._recordingTimeout = window.setTimeout(() => this._stopRecording(), SeamlessRelaunchConstants.RecordTerminalDuration); + return; + } + + // Trigger a swap if there was a recent relaunch + if (this._secondRecorder) { + this.triggerSwap(); + } + + this._swapTimeout = window.setTimeout(() => this.triggerSwap(), SeamlessRelaunchConstants.SwapWaitMaximumDuration); + + // Pause all outgoing data events + this._dataListener?.dispose(); + + this._firstDisposable?.dispose(); + const recorder = this._createRecorder(process); + [this._secondRecorder, this._secondDisposable] = recorder; + } + + /** + * Trigger the swap of the processes if needed (eg. timeout, input) + */ + triggerSwap() { + console.trace('trigger swap'); + // Clear the swap timeout if it exists + if (this._swapTimeout) { + window.clearTimeout(this._swapTimeout); + this._swapTimeout = undefined; + } + + // Do nothing if there's nothing being recorder + if (!this._firstRecorder) { + console.log('no first'); + return; + } + + // Clear the first recorder if no second process was attached before the swap trigger + if (!this._secondRecorder) { + console.log('no second'); + this._firstRecorder = undefined; + this._firstDisposable?.dispose(); + return; + } + + // Generate data for each recorder + const firstData = this._getDataFromRecorder(this._firstRecorder); + const secondData = this._getDataFromRecorder(this._secondRecorder); + + // Re-write the terminal if the data differs + if (firstData === secondData) { + this._logService.trace(`Relaunching terminal to update environment - identical content`); + } else { + this._logService.trace(`Relaunching terminal to update environment - resetting content`); + // Fire full reset (RIS) followed by the new data so the update happens in the same frame + this._onProcessData.fire({ data: `\x1bc${secondData}`, sync: false }); + } + + // Set up the new data listener + this._dataListener?.dispose(); + this._dataListener = this._activeProcess!.onProcessData(e => this._onProcessData.fire(e)); + + // Replace first recorder with second + this._firstRecorder = this._secondRecorder; + this._firstDisposable?.dispose(); + this._firstDisposable = this._secondDisposable; + if (this._recordingTimeout) { + window.clearTimeout(this._recordingTimeout); + } + this._recordingTimeout = window.setTimeout(() => this._stopRecording(), SeamlessRelaunchConstants.RecordTerminalDuration); + } + + private _stopRecording() { + // Continue recording if a swap is coming + if (this._swapTimeout) { + return; + } + + // Clear the timeout + if (this._recordingTimeout) { + window.clearTimeout(this._recordingTimeout); + this._recordingTimeout = undefined; + } + + // Stop recording + this._firstRecorder = undefined; + this._firstDisposable?.dispose(); + this._secondRecorder = undefined; + this._secondDisposable?.dispose(); + } + + private _createRecorder(process: ITerminalChildProcess): [TerminalRecorder, IDisposable] { + const recorder = new TerminalRecorder(0, 0); + const disposable = process.onProcessData(e => recorder.recordData(typeof e === 'string' ? e : e.data)); + return [recorder, disposable]; + } + + private _getDataFromRecorder(recorder: TerminalRecorder): string { + return recorder.generateReplayEvent().events.filter(e => !!e.data).map(e => e.data).join(''); + } +} diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 62094770b0e56..1320ed34e418b 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -262,7 +262,7 @@ export interface ITerminalProcessManager extends IDisposable { dispose(immediate?: boolean): void; detachFromProcess(): void; createProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise; - relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise; + relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean, reset: boolean): Promise; write(data: string): void; setDimensions(cols: number, rows: number): Promise; setDimensions(cols: number, rows: number, sync: false): Promise; From 162d08892db9b430588040df848c4170361d3074 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 12 Mar 2021 08:22:08 -0800 Subject: [PATCH 3/6] Clean up --- .../browser/terminalProcessManager.ts | 54 ++----------------- 1 file changed, 4 insertions(+), 50 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 4508757d33717..a810dc6a2452f 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -291,56 +291,18 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce } public async relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean, reset: boolean): Promise { - console.log('time', performance.now()); this.ptyProcessReady = new Promise(c => { this.onProcessReady(() => { this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); c(undefined); }); }); - console.log('relaunch!'); + this._logService.trace(`Relaunching terminal instance ${this._instanceId}`); // this._relaunchDisposable?.dispose(); // this._relaunchDisposable = undefined; return this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled, reset); - - // if (!reset || !this._relaunchRecorder) { - // return process; - // } - // const initialData = this._getDataFromRecorder(this._relaunchRecorder); - - // await timeout(3000); - // this._swapProcesses(initialData); - // return process; } - // private _getDataFromRecorder(recorder: TerminalRecorder): string { - // return recorder.generateReplayEvent().events.filter(e => !!e.data).map(e => e.data).join(''); - // } - - // private _swapProcesses(initialData: string) { - // // TODO: Trigger immediately on input - // // TODO: Is this right? What can unset this? - // if (!this._relaunchRecorder) { - // return process; - // } - // const secondReplay = this._relaunchRecorder.generateReplayEvent(); - // const secondData = secondReplay.events.filter(e => !!e.data).map(e => e.data).join(''); - - // console.log('first data', initialData); - // console.log('secondReplay events', secondReplay.events); - // console.log('second data', secondData); - // console.log('equal?', initialData === secondData); - // // Re-write the terminal if the data differs - // if (initialData === secondData) { - // this._logService.trace(`Relaunching terminal "${this._instanceId}" to update environment - identical content`); - // } else { - // this._logService.trace(`Relaunching terminal "${this._instanceId}" to update environment - resetting content`); - // // Fire full reset (RIS) followed by the new data so the update happens in the same frame - // this._onProcessData.fire({ data: `\x1bc${secondData}`, sync: false }); - // } - // return process; - // } - // Fetch any extension environment additions and apply them private async _setupEnvVariableInfo(activeWorkspaceRootUri: URI | undefined, shellLaunchConfig: IShellLaunchConfig): Promise { const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); @@ -573,8 +535,6 @@ const enum SeamlessRelaunchConstants { * The maximum duration after a relaunch occurs to trigger a swap. */ SwapWaitMaximumDuration = 3000 - - // TODO: Need 2 times: How long to hold onto the first recorder for and how long to wait until diffing the 2 procs } /** @@ -603,15 +563,12 @@ class SeamlessRelaunchDataFilter extends Disposable { } newProcess(process: ITerminalChildProcess, reset: boolean) { - // TODO: Disable recorder for reconnected terminals - this._activeProcess = process; - // Stop listening to the old process this._dataListener?.dispose(); + this._activeProcess = process; // If the process is new, relaunch has timed out or the terminal should not reset, start // listening and firing data events immediately - console.log('relaunch newProcess', this._firstRecorder, reset); if (!this._firstRecorder || !reset) { this._firstDisposable?.dispose(); [this._firstRecorder, this._firstDisposable] = this._createRecorder(process); @@ -642,7 +599,6 @@ class SeamlessRelaunchDataFilter extends Disposable { * Trigger the swap of the processes if needed (eg. timeout, input) */ triggerSwap() { - console.trace('trigger swap'); // Clear the swap timeout if it exists if (this._swapTimeout) { window.clearTimeout(this._swapTimeout); @@ -651,13 +607,11 @@ class SeamlessRelaunchDataFilter extends Disposable { // Do nothing if there's nothing being recorder if (!this._firstRecorder) { - console.log('no first'); return; } // Clear the first recorder if no second process was attached before the swap trigger if (!this._secondRecorder) { - console.log('no second'); this._firstRecorder = undefined; this._firstDisposable?.dispose(); return; @@ -669,9 +623,9 @@ class SeamlessRelaunchDataFilter extends Disposable { // Re-write the terminal if the data differs if (firstData === secondData) { - this._logService.trace(`Relaunching terminal to update environment - identical content`); + this._logService.trace(`Seamless terminal relaunch - identical content`); } else { - this._logService.trace(`Relaunching terminal to update environment - resetting content`); + this._logService.trace(`Seamless terminal relaunch - resetting content`); // Fire full reset (RIS) followed by the new data so the update happens in the same frame this._onProcessData.fire({ data: `\x1bc${secondData}`, sync: false }); } From 9a7366de9bd302b21dd0ce3e064471c05d743240 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 16 Mar 2021 08:24:21 -0700 Subject: [PATCH 4/6] Prevent process ready log firing multiple times --- .../browser/terminalProcessManager.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index a810dc6a2452f..4a92c9d82ae00 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -126,12 +126,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce super(); this._localTerminalService = localTerminalService; - this.ptyProcessReady = new Promise(c => { - this.onProcessReady(() => { - this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); - c(undefined); - }); - }); + this.ptyProcessReady = this._createPtyProcessReadyPromise(); this.getLatency(); this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e)); this._dataFilter = this._instantiationService.createInstance(SeamlessRelaunchDataFilter); @@ -159,6 +154,16 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce super.dispose(); } + private _createPtyProcessReadyPromise(): Promise { + return new Promise(c => { + const listener = this.onProcessReady(() => { + this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); + listener.dispose(); + c(undefined); + }); + }); + } + public detachFromProcess(): void { if (this._process?.detach) { this._process.detach(); @@ -291,15 +296,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce } public async relaunch(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean, reset: boolean): Promise { - this.ptyProcessReady = new Promise(c => { - this.onProcessReady(() => { - this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`); - c(undefined); - }); - }); + this.ptyProcessReady = this._createPtyProcessReadyPromise(); this._logService.trace(`Relaunching terminal instance ${this._instanceId}`); - // this._relaunchDisposable?.dispose(); - // this._relaunchDisposable = undefined; return this.createProcess(shellLaunchConfig, cols, rows, isScreenReaderModeEnabled, reset); } From 7c0e20ca4c0adc80bb23b195683c420e8edf6622 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 16 Mar 2021 08:29:29 -0700 Subject: [PATCH 5/6] Allow terminal recorder to avoid initial resize event --- .../platform/terminal/common/terminalRecorder.ts | 14 +++++++++++--- src/vs/platform/terminal/node/ptyService.ts | 2 +- .../terminal/browser/terminalProcessManager.ts | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/vs/platform/terminal/common/terminalRecorder.ts b/src/vs/platform/terminal/common/terminalRecorder.ts index 6341bcc17a5d1..4d5957cf9fd4e 100644 --- a/src/vs/platform/terminal/common/terminalRecorder.ts +++ b/src/vs/platform/terminal/common/terminalRecorder.ts @@ -22,10 +22,18 @@ export interface IRemoteTerminalProcessReplayEvent { export class TerminalRecorder { private _entries: RecorderEntry[]; - private _totalDataLength: number; + private _totalDataLength: number = 0; - constructor(cols: number, rows: number) { - this._entries = [{ cols, rows, data: [] }]; + constructor(initialDimensions?: { cols: number, rows: number }) { + if (initialDimensions) { + this._entries = [{ + cols: initialDimensions.cols, + rows: initialDimensions.rows, + data: [] + }]; + } else { + this._entries = []; + } this._totalDataLength = 0; } diff --git a/src/vs/platform/terminal/node/ptyService.ts b/src/vs/platform/terminal/node/ptyService.ts index 3dfcaf8a5a5b6..ecd9bd908b75b 100644 --- a/src/vs/platform/terminal/node/ptyService.ts +++ b/src/vs/platform/terminal/node/ptyService.ts @@ -270,7 +270,7 @@ export class PersistentTerminalProcess extends Disposable { private readonly _logService: ILogService ) { super(); - this._recorder = new TerminalRecorder(cols, rows); + this._recorder = new TerminalRecorder({ cols, rows }); this._orphanQuestionBarrier = null; this._orphanQuestionReplyTime = 0; this._disconnectRunner1 = this._register(new RunOnceScheduler(() => { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 4a92c9d82ae00..a82bb43f80890 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -662,7 +662,7 @@ class SeamlessRelaunchDataFilter extends Disposable { } private _createRecorder(process: ITerminalChildProcess): [TerminalRecorder, IDisposable] { - const recorder = new TerminalRecorder(0, 0); + const recorder = new TerminalRecorder(); const disposable = process.onProcessData(e => recorder.recordData(typeof e === 'string' ? e : e.data)); return [recorder, disposable]; } From 7a6944d2d0d41da53fc66bc09bc3539923a131a1 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 16 Mar 2021 09:34:19 -0700 Subject: [PATCH 6/6] Add unit tests for TerminalRecorder --- .../terminal/common/terminalRecorder.ts | 17 ++---- src/vs/platform/terminal/node/ptyService.ts | 2 +- .../test/common/terminalRecorder.test.ts | 52 +++++++++++++++++++ .../browser/terminalProcessManager.ts | 2 +- 4 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 src/vs/platform/terminal/test/common/terminalRecorder.test.ts diff --git a/src/vs/platform/terminal/common/terminalRecorder.ts b/src/vs/platform/terminal/common/terminalRecorder.ts index 4d5957cf9fd4e..e8a6e17c3667a 100644 --- a/src/vs/platform/terminal/common/terminalRecorder.ts +++ b/src/vs/platform/terminal/common/terminalRecorder.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IPtyHostProcessReplayEvent } from 'vs/platform/terminal/common/terminalProcess'; +import { IPtyHostProcessReplayEvent, ReplayEntry } from 'vs/platform/terminal/common/terminalProcess'; const MAX_RECORDER_DATA_SIZE = 1024 * 1024; // 1MB @@ -13,8 +13,6 @@ interface RecorderEntry { data: string[]; } -export interface ReplayEntry { cols: number; rows: number; data: string; } - export interface IRemoteTerminalProcessReplayEvent { events: ReplayEntry[]; } @@ -24,17 +22,8 @@ export class TerminalRecorder { private _entries: RecorderEntry[]; private _totalDataLength: number = 0; - constructor(initialDimensions?: { cols: number, rows: number }) { - if (initialDimensions) { - this._entries = [{ - cols: initialDimensions.cols, - rows: initialDimensions.rows, - data: [] - }]; - } else { - this._entries = []; - } - this._totalDataLength = 0; + constructor(cols: number, rows: number) { + this._entries = [{ cols, rows, data: [] }]; } public recordResize(cols: number, rows: number): void { diff --git a/src/vs/platform/terminal/node/ptyService.ts b/src/vs/platform/terminal/node/ptyService.ts index ecd9bd908b75b..3dfcaf8a5a5b6 100644 --- a/src/vs/platform/terminal/node/ptyService.ts +++ b/src/vs/platform/terminal/node/ptyService.ts @@ -270,7 +270,7 @@ export class PersistentTerminalProcess extends Disposable { private readonly _logService: ILogService ) { super(); - this._recorder = new TerminalRecorder({ cols, rows }); + this._recorder = new TerminalRecorder(cols, rows); this._orphanQuestionBarrier = null; this._orphanQuestionReplyTime = 0; this._disconnectRunner1 = this._register(new RunOnceScheduler(() => { diff --git a/src/vs/platform/terminal/test/common/terminalRecorder.test.ts b/src/vs/platform/terminal/test/common/terminalRecorder.test.ts new file mode 100644 index 0000000000000..007ce51448d38 --- /dev/null +++ b/src/vs/platform/terminal/test/common/terminalRecorder.test.ts @@ -0,0 +1,52 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { TerminalRecorder } from 'vs/platform/terminal/common/terminalRecorder'; +import { ReplayEntry } from 'vs/platform/terminal/common/terminalProcess'; + +function eventsEqual(recorder: TerminalRecorder, expected: ReplayEntry[]) { + const actual = recorder.generateReplayEvent().events; + for (let i = 0; i < expected.length; i++) { + assert.deepStrictEqual(actual[i], expected[i]); + } +} + +suite('TerminalRecorder', () => { + test('should record dimensions', () => { + const recorder = new TerminalRecorder(1, 2); + eventsEqual(recorder, [ + { cols: 1, rows: 2, data: '' } + ]); + recorder.recordData('a'); + recorder.recordResize(3, 4); + eventsEqual(recorder, [ + { cols: 1, rows: 2, data: 'a' }, + { cols: 3, rows: 4, data: '' } + ]); + }); + test('should ignore resize events without data', () => { + const recorder = new TerminalRecorder(1, 2); + eventsEqual(recorder, [ + { cols: 1, rows: 2, data: '' } + ]); + recorder.recordResize(3, 4); + eventsEqual(recorder, [ + { cols: 3, rows: 4, data: '' } + ]); + }); + test('should record data and combine it into the previous resize event', () => { + const recorder = new TerminalRecorder(1, 2); + recorder.recordData('a'); + recorder.recordData('b'); + recorder.recordResize(3, 4); + recorder.recordData('c'); + recorder.recordData('d'); + eventsEqual(recorder, [ + { cols: 1, rows: 2, data: 'ab' }, + { cols: 3, rows: 4, data: 'cd' } + ]); + }); +}); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index a82bb43f80890..4a92c9d82ae00 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -662,7 +662,7 @@ class SeamlessRelaunchDataFilter extends Disposable { } private _createRecorder(process: ITerminalChildProcess): [TerminalRecorder, IDisposable] { - const recorder = new TerminalRecorder(); + const recorder = new TerminalRecorder(0, 0); const disposable = process.onProcessData(e => recorder.recordData(typeof e === 'string' ? e : e.data)); return [recorder, disposable]; }