Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Mar 5, 2025

Context

If the terminal output from a command exceeds 100KB then remove the middle of the output and just return the first 50KB and last 50KB. The 100KB limit is configurable:

Screenshot 2025-03-05 at 7 59 14 PM

Before:

3-million-hello-worlds-before.mp4

After:

3-million-hello-worlds-after.mp4

Implementation

We now have an OutputBuilder object that you can append text to and it will maintain two buffers if the maxSize is exceeded. It also supports cursor-based reads from the buffer so we can properly implement getUnretrievedOutput.

How to Test

Get in Touch


Important

Adds OutputBuilder for smart truncation of terminal output, updates settings for output limit, and includes comprehensive tests.

  • Behavior:
    • Introduces OutputBuilder class in OutputBuilder.ts for smart truncation of terminal output, preserving start and end.
    • Replaces truncateOutput with OutputBuilder in Cline.ts and TerminalManager.ts.
    • Adds TERMINAL_OUTPUT_LIMIT constant in terminal.ts.
  • Settings:
    • Adds terminalOutputLimit setting in ExtensionStateContext.tsx, AdvancedSettings.tsx, and SettingsView.tsx.
    • Updates WebviewMessage.ts and ExtensionMessage.ts to handle terminalOutputLimit.
  • Testing:
    • Adds tests for OutputBuilder in OutputBuilder.test.ts.
    • Updates tests in TerminalProcess.test.ts and mergePromise.test.ts to reflect new behavior.
  • Misc:
    • Removes truncateOutput from extract-text.ts and related tests.

This description was created by Ellipsis for 7eee3e0. It will automatically update as commits are pushed.

@cte cte requested a review from mrubens as a code owner March 5, 2025 06:15
@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2025

⚠️ No Changeset found

Latest commit: 7eee3e0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -0,0 +1,154 @@
interface OutputBuilderOptions {
maxSize?: number // Max size of the buffer (in bytes).
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider updating the documentation for maxSize. Although the comment states it's in bytes, this implementation uses string length (code units), which may not match actual byte counts for Unicode. Clarify this to avoid confusion.

Suggested change
maxSize?: number // Max size of the buffer (in bytes).
maxSize?: number // Max size of the buffer (in code units, not bytes).

@hannesrudolph hannesrudolph moved this from PR to Needs Review in Roo Code Roadmap Mar 5, 2025
@hannesrudolph hannesrudolph moved this from PR - Needs Review to PR - In Progress in Roo Code Roadmap Mar 5, 2025
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 6, 2025
@cte cte force-pushed the cte/terminal-middle-out branch from ae3738b to 8bfcd83 Compare March 6, 2025 03:35
@cte cte force-pushed the cte/terminal-middle-out branch from 0deaf60 to 7eee3e0 Compare March 6, 2025 03:37
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 6, 2025
@cte cte merged commit aa57ae9 into main Mar 6, 2025
11 checks passed
@cte cte deleted the cte/terminal-middle-out branch March 6, 2025 04:14
@github-project-automation github-project-automation bot moved this from PR [Greenlit] to Done in Roo Code Roadmap Mar 6, 2025
cte pushed a commit that referenced this pull request Mar 11, 2025
This reverts commit 7eee3e0.

Middle-out truncation is a really great feature and it should still be
implemented, however it unnecessarily interferes with #1365 because it
hooked into the low-level chunk management that comes directly from VSCE
shell integration.

The best place to hook OutputBuilder is as follows depending on the
state of terminal interaction:

1. Foreground terminals:

Cline.ts:
	 executeCommandTool(...) {
	     process.on("line", (line) => {
		    lines.push(line)
		    ...
	    }
	 }

2. For background terminals: hook in at the point that getUnretrievedOutput is consumed for active or
   inactive terminals in Cline.ts:getEnvironmentDetails()

Please note:

The Terminal classes are very sensitive to change, partially because of
the complicated way that shell integration works with VSCE, and
partially because of the way that Cline interacts with the Terminal*
class abstractions that make VSCE shell integration easier to work with.

At the point that PR#1365 is merged, it is unlikely that any Terminal*
classes will need to be modified substantially.  Generally speaking, we
should think of this is a stable interface and minimize changes.

Reverts: #1390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

3 participants