From 0a19f7702a9c9205cdab214860fa97e97dc7c4ea Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 12 Jan 2021 11:36:21 -0800 Subject: [PATCH] Rename ackId to charCount --- src/vs/workbench/api/browser/mainThreadTerminalService.ts | 2 +- src/vs/workbench/api/common/extHost.protocol.ts | 2 +- src/vs/workbench/api/common/extHostTerminalService.ts | 6 +++--- .../contrib/terminal/browser/remoteTerminalService.ts | 2 +- .../terminal/browser/terminalProcessExtHostProxy.ts | 4 ++-- .../contrib/terminal/browser/terminalProcessManager.ts | 5 +++-- src/vs/workbench/contrib/terminal/common/terminal.ts | 6 +++--- src/vs/workbench/contrib/terminal/node/terminalProcess.ts | 8 +++----- 8 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 4c9ab2d4e06..1888982a193 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -272,7 +272,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape request.isWorkspaceShellAllowed ).then(request.callback, request.callback); - proxy.onAcknowledgeDataEvent(ackId => this._proxy.$acceptProcessAckDataEvent(proxy.terminalId, ackId)); + proxy.onAcknowledgeDataEvent(charCount => this._proxy.$acceptProcessAckDataEvent(proxy.terminalId, charCount)); proxy.onInput(data => this._proxy.$acceptProcessInput(proxy.terminalId, data)); proxy.onResize(dimensions => this._proxy.$acceptProcessResize(proxy.terminalId, dimensions.cols, dimensions.rows)); proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(proxy.terminalId, immediate)); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index e151a129653..d3a5b973021 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1545,7 +1545,7 @@ export interface ExtHostTerminalServiceShape { $acceptTerminalMaximumDimensions(id: number, cols: number, rows: number): void; $spawnExtHostProcess(id: number, shellLaunchConfig: IShellLaunchConfigDto, activeWorkspaceRootUri: UriComponents | undefined, cols: number, rows: number, isWorkspaceShellAllowed: boolean): Promise; $startExtensionTerminal(id: number, initialDimensions: ITerminalDimensionsDto | undefined): Promise; - $acceptProcessAckDataEvent(id: number, ackId: number): void; + $acceptProcessAckDataEvent(id: number, charCount: number): void; $acceptProcessInput(id: number, data: string): void; $acceptProcessResize(id: number, cols: number, rows: number): void; $acceptProcessShutdown(id: number, immediate: boolean): void; diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 8d96e657b59..597b304f4fc 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -220,7 +220,7 @@ export class ExtHostPseudoterminal implements ITerminalChildProcess { } } - acknowledgeDataEvent(ackId: number): void { + acknowledgeDataEvent(charCount: number): void { // TODO: Determine whether ExtHostPseudoterminal terminals should support flow control, this // would need resume/pause APIs @@ -495,8 +495,8 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I return disposables; } - public $acceptProcessAckDataEvent(id: number, ackId: number): void { - this._terminalProcesses.get(id)?.acknowledgeDataEvent(ackId); + public $acceptProcessAckDataEvent(id: number, charCount: number): void { + this._terminalProcesses.get(id)?.acknowledgeDataEvent(charCount); } public $acceptProcessInput(id: number, data: string): void { diff --git a/src/vs/workbench/contrib/terminal/browser/remoteTerminalService.ts b/src/vs/workbench/contrib/terminal/browser/remoteTerminalService.ts index c7be47e5214..872e9a36fbf 100644 --- a/src/vs/workbench/contrib/terminal/browser/remoteTerminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/remoteTerminalService.ts @@ -252,7 +252,7 @@ export class RemoteTerminalProcess extends Disposable implements ITerminalChildP }); } - public acknowledgeDataEvent(ackId: number): void { + public acknowledgeDataEvent(charCount: number): void { // TODO: Support flow control for server spawned processes } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts index 40895f7e432..0be5cf88be5 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy.ts @@ -141,8 +141,8 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal this._onResize.fire({ cols, rows }); } - public acknowledgeDataEvent(ackId: number): void { - this._onAcknowledgeDataEvent.fire(ackId); + public acknowledgeDataEvent(charCount: number): void { + this._onAcknowledgeDataEvent.fire(charCount); } public getInitialCwd(): Promise { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 0368eef6b69..bec1293ce6e 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -331,8 +331,9 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce return Promise.resolve(this._latency); } - public acknowledgeDataEvent(ackId: number): void { - this._process?.acknowledgeDataEvent(ackId); + public acknowledgeDataEvent(charCount: number): void { + // TODO: Batch these acknowledge calls (in proxy/remote connection?) + this._process?.acknowledgeDataEvent(charCount); } private _onExit(exitCode: number | undefined): void { diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index b46c5367028..076030c6b4d 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -379,8 +379,7 @@ export interface ITerminalProcessManager extends IDisposable { createProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number, isScreenReaderModeEnabled: boolean): Promise; write(data: string): void; setDimensions(cols: number, rows: number): void; - // TODO: Rename to charCount - acknowledgeDataEvent(ackId: number): void; + acknowledgeDataEvent(charCount: number): void; getInitialCwd(): Promise; getCwd(): Promise; @@ -514,8 +513,9 @@ export interface ITerminalChildProcess { * Acknowledge a data event has been parsed by the terminal, this is used to implement flow * control to ensure remote processes to not get too far ahead of the client and flood the * connection. + * @param charCount The number of characters being acknowledged. */ - acknowledgeDataEvent(ackId: number): void; + acknowledgeDataEvent(charCount: number): void; getInitialCwd(): Promise; getCwd(): Promise; diff --git a/src/vs/workbench/contrib/terminal/node/terminalProcess.ts b/src/vs/workbench/contrib/terminal/node/terminalProcess.ts index 4eec770e6c8..fa2ba90fb6d 100644 --- a/src/vs/workbench/contrib/terminal/node/terminalProcess.ts +++ b/src/vs/workbench/contrib/terminal/node/terminalProcess.ts @@ -186,14 +186,11 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess ptyProcess.onData(data => { // TODO: Periodically request ACK between low and high watermark const fakeLatency = 1000; - // TODO: if we're just sending the data length over, ackId doesn't need to exist at all // TODO: We don't need to ack everything, just count on the other side and ack every 1000/10000 bytes this._totalDataCharCount += data.length; setTimeout(() => { this._onProcessData.fire(data); }, fakeLatency); - // TODO: Use bytes, not messages - // console.log('check', this._currentAckRequestId - this._ackedRequestId, FlowControl.HighWatermarkBytes); if (this._totalDataCharCount - this._acknowledgedDataCharCount > FlowControl.HighWatermarkChars) { // TODO: Expose as public API in node-pty console.log('pause', this._totalDataCharCount - this._acknowledgedDataCharCount, '>', FlowControl.HighWatermarkChars); @@ -360,9 +357,10 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess } } - public acknowledgeDataEvent(ackId: number): void { - this._acknowledgedDataCharCount += ackId; + public acknowledgeDataEvent(charCount: number): void { + this._acknowledgedDataCharCount += charCount; if (this._totalDataCharCount - this._acknowledgedDataCharCount < FlowControl.LowWatermarkChars) { + // TODO: Check whether it is paused before resuming // TODO: Expose as public API in node-pty (this._ptyProcess as any).resume(); }