Make execution.read() return output consistently

Previously TerminalShellExecution.read() would return output inconsistently,
sometimes including the 633;D sequences, sometimes, not, sometimes including the
following prompt. This change ensures that the output is always consistent by
first segmenting any data events containing C and D sequences, and then ensuring
nothing async happens between the D handler firing and the execution end event
firing to the exthost. This means that C will consistently be included (as was
the case before) and D will consistently be excluded.

Fixes #237208
This commit is contained in:
Daniel Imms
2025-02-21 08:41:51 -08:00
parent dc49da54e6
commit e380facb66
3 changed files with 27 additions and 23 deletions
@@ -381,14 +381,11 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
if (newCommand) {
this._commands.push(newCommand);
this._commitCommandFinished = new RunOnceScheduler(() => {
this._onBeforeCommandFinished.fire(newCommand);
if (!this._currentCommand.isInvalid) {
this._logService.debug('CommandDetectionCapability#onCommandFinished', newCommand);
this._onCommandFinished.fire(newCommand);
}
}, 50);
this._commitCommandFinished.schedule();
this._onBeforeCommandFinished.fire(newCommand);
if (!this._currentCommand.isInvalid) {
this._logService.debug('CommandDetectionCapability#onCommandFinished', newCommand);
this._onCommandFinished.fire(newCommand);
}
}
this._currentCommand = new PartialTerminalCommand(this._terminal);
this._handleCommandStartOptions = undefined;
@@ -90,7 +90,9 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
// TerminalShellExecution.createDataStream
// Debounce events to reduce the message count - when this listener is disposed the events will be flushed
instanceDataListeners.get(instanceId)?.dispose();
instanceDataListeners.set(instanceId, Event.accumulate(e.instance.onData, 50, this._store)(events => this._proxy.$shellExecutionData(instanceId, events.join(''))));
instanceDataListeners.set(instanceId, Event.accumulate(e.instance.onData, 50, this._store)(events => {
this._proxy.$shellExecutionData(instanceId, events.join(''));
}));
}));
// onDidEndTerminalShellExecution
@@ -99,10 +101,10 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
currentCommand = undefined;
const instanceId = e.instance.instanceId;
instanceDataListeners.get(instanceId)?.dispose();
// Send end in a microtask to ensure the data events are sent first
setTimeout(() => {
this._proxy.$shellExecutionEnd(instanceId, e.data.command, convertToExtHostCommandLineConfidence(e.data), e.data.isTrusted, e.data.exitCode);
});
// Shell integration C (executed) and D (command finished) sequences should always be in
// their own events, so send this immediately. This means that the D sequence will not
// be included as it's currently being parsed when the command finished event fires.
this._proxy.$shellExecutionEnd(instanceId, e.data.command, convertToExtHostCommandLineConfidence(e.data), e.data.isTrusted, e.data.exitCode);
}));
// Clean up after dispose
@@ -1522,18 +1522,23 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}
private _onProcessData(ev: IProcessDataEvent): void {
// Ensure events are split by SI command execute sequence to ensure the output of the
// command can be read by extensions. This must be done here as xterm.js does not currently
// have a listener for when individual data events are parsed, only `onWriteParsed` which
// fires when the write buffer is flushed.
const execIndex = ev.data.indexOf('\x1b]633;C\x07');
if (execIndex !== -1) {
// Ensure events are split by SI command execute and command finished sequence to ensure the
// output of the command can be read by extensions and the output of the command is of a
// consistent form respectively. This must be done here as xterm.js does not currently have
// a listener for when individual data events are parsed, only `onWriteParsed` which fires
// when the write buffer is flushed.
const match = ev.data.match(/(?<seq>\x1b][16]33;(?:C|D(?:;\d+)?)\x07)/);
const index = match?.index;
if (match?.groups?.seq && index !== undefined) {
const seq = match?.groups?.seq;
if (ev.trackCommit) {
this._writeProcessData(ev.data.substring(0, execIndex + '\x1b]633;C\x07'.length));
ev.writePromise = new Promise<void>(r => this._writeProcessData(ev.data.substring(execIndex + '\x1b]633;C\x07'.length), r));
this._writeProcessData(ev.data.substring(0, index));
this._writeProcessData(seq);
ev.writePromise = new Promise<void>(r => this._writeProcessData(ev.data.substring(index + seq.length), r));
} else {
this._writeProcessData(ev.data.substring(0, execIndex + '\x1b]633;C\x07'.length));
this._writeProcessData(ev.data.substring(execIndex + '\x1b]633;C\x07'.length));
this._writeProcessData(ev.data.substring(0, index));
this._writeProcessData(seq);
this._writeProcessData(ev.data.substring(index + seq.length));
}
} else {
if (ev.trackCommit) {