Skip to content

Conversation

@meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jan 5, 2026

fixes #271390

  • Shows the output action onCommandExecuted
  • onData we track cursor movement and dirty ranges so we can append new VT sequences instead of rewriting the full buffer.
  • Allows providing vt sequence range even when start or end marker has been disposed to support cases where the output exceeds scrollback

Copilot AI review requested due to automatic review settings January 5, 2026 18:48
@meganrogge meganrogge self-assigned this Jan 5, 2026
@meganrogge meganrogge marked this pull request as draft January 5, 2026 18:49
@meganrogge meganrogge requested a review from Tyriar January 5, 2026 18:49
@meganrogge meganrogge added this to the January 2026 milestone Jan 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements real-time streaming of terminal command output in chat interactions. The implementation monitors terminal events (cursor movement, line feeds, data writes) and incrementally updates the displayed output in the chat interface, rather than waiting for the command to complete before showing results.

Key Changes

  • Made terminal markers optional in getRangeAsVT() to support streaming scenarios where start/end positions may not be immediately available
  • Refactored DetachedTerminalCommandMirror to add streaming capabilities via event listeners and incremental VT sequence appending
  • Enhanced command resolution to support commands that are currently executing (not just finished commands)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts Made startMarker parameter optional and added validation to handle disposed markers (line === -1)
src/vs/workbench/contrib/terminal/browser/terminal.ts Updated getRangeAsVT interface signature and documentation to reflect optional start marker
src/vs/workbench/contrib/terminal/browser/chatTerminalCommandMirror.ts Major refactoring: removed base class, added streaming infrastructure with cursor tracking, dirty range management, and incremental VT updates
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts Added support for resolving executing commands and listening to command execution events for real-time updates

Tyriar
Tyriar previously requested changes Jan 5, 2026
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great!

@Tyriar
Copy link
Member

Tyriar commented Jan 5, 2026

the height sometimes is insufficient

Maybe calls for a setting, perhaps let's wait to see some real world cases to see if we can avoid a setting or do something more clever.

@meganrogge meganrogge force-pushed the merogge/streaming-output-3 branch from 264432c to ed10bcf Compare January 5, 2026 20:43
@meganrogge meganrogge requested a review from Tyriar January 5, 2026 22:00
@meganrogge
Copy link
Contributor Author

meganrogge commented Jan 6, 2026

@Tyriar fixed the height issue

When terminal output was streaming in, the height of the output element was sometimes incorrect (too small) for the initial lines.

  • The line count was being calculated from parsing VT text using text.split('\r\n').length instead of using the actual rendered lines in the terminal buffer
  • The xterm write callback fires when data is parsed, but the buffer cursor position may not have fully updated yet

Now, line count is calculated by:

  • Reading the terminal buffer's cursor position after the write completes [baseY + cursorY + 1]
  • Also counting newlines in the text as a minimum fallback
  • Taking the max of these two values

This ensures the height is never too small. If the terminal buffer hasn't fully updated its cursor position yet, the text-based count provides a floor value.

demo.mov

@meganrogge meganrogge marked this pull request as ready for review January 8, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stream inline terminal tool output into chat view

3 participants