Skip to content

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented Apr 29, 2025

Refactored CommandExecution.tsx to:

  • Implement CodeBlock component for terminal display
  • Use Shiki for syntax highlighting of shell commands and terminal output

Important

Refactor CommandExecution.tsx to use CodeBlock for Shiki syntax highlighting and improve output display.

  • Component Refactor:
    • Refactor CommandExecution.tsx to use CodeBlock for syntax highlighting of command and output.
    • Remove Virtuoso and ChevronDown imports, replacing with CodeBlock and useTranslation.
  • Behavior:
    • Use Shiki for syntax highlighting shell commands and terminal output.
    • Add expand/collapse functionality for output display with a chevron icon.
  • Styling:
    • Apply consistent styling using CODE_BLOCK_BG_COLOR and inline styles for borders and padding.

This description was created by Ellipsis for b4fd9ee. You can customize this summary. It will automatically update as commits are pushed.

Refactored CommandExecution.tsx to:
- Implement CodeBlock component for terminal display
- Use Shiki for syntax highlighting of shell commands and terminal output

Signed-off-by: Eric Wheeler <[email protected]>
@KJ7LNW KJ7LNW requested review from cte and mrubens as code owners April 29, 2025 00:33
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 29, 2025
<CodeBlock source={command} language="shell" />
{output.length > 0 && (
<div style={{ width: "100%" }}>
<div
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using a semantic <button> element instead of a <div> with onClick for better accessibility and keyboard interactions.

{output.length > 0 && (
<div style={{ width: "100%" }}>
<div
onClick={onToggleExpand}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use ARIA attributes on interactive elements (e.g., aria-expanded) to improve accessibility.

@samhvw8
Copy link
Contributor

samhvw8 commented Apr 29, 2025

@KJ7LNW i think we should keep virturso, that address for this problems

#2326

you can read it again in this pr

can you test it again like my screencast

@dosubot dosubot bot added the enhancement New feature or request label Apr 29, 2025
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Apr 29, 2025

@KJ7LNW i think we should keep virturso, that address for this problems

#2326

you can read it again in this pr

can you test it again like my screencast

We can try but, virtuoso rendering is buggy, especially for container height.

I am trying a few things ...

Avoids rendering problems and performance regressions:
- Compresses output using BaseTerminal.compressTerminalOutput
- Eliminates need for Virtuoso by limiting output size

Ensures command output shown in UI matches the compressed format provided to the model:
- Applies same BaseTerminal.compressTerminalOutput processing
- Uses configured terminalOutputLineLimit for consistency

Signed-off-by: Eric Wheeler <[email protected]>
@changeset-bot
Copy link

changeset-bot bot commented Apr 29, 2025

⚠️ No Changeset found

Latest commit: 7ec9381

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

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Apr 29, 2025

@samhvw8 ,

Ultimately the goal is to prevent large outputs from slowing down the system, correct?

Would b342ee4 be an acceptable compromise, because I am having a lot of trouble getting virtuoso to do what needs to happen to render properly in the user interface with Shiki and proper height scaling.

It is also an improvement because it presents to the user exactly what the model will receive after compression/truncation, which certainly seems appropriate.

- Replace virtuoso mock with CodeBlock mock to fix shiki import error
- Update CodeBlock mock to properly handle source prop
- Add proper empty output test case verification
- Keep original test values and structure

Signed-off-by: Eric Wheeler <[email protected]>
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Apr 29, 2025
@samhvw8
Copy link
Contributor

samhvw8 commented Apr 29, 2025

@samhvw8 ,

Ultimately the goal is to prevent large outputs from slowing down the system, correct?

Would b342ee4 be an acceptable compromise, because I am having a lot of trouble getting virtuoso to do what needs to happen to render properly in the user interface with Shiki and proper height scaling.

It is also an improvement because it presents to the user exactly what the model will receive after compression/truncation, which certainly seems appropriate.

Can you make a screencasts to test this ?
If we can solve this problems we can get rid of virtuoso 🙆

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Apr 29, 2025

Can you make a screencasts to test this ?
If we can solve this problems we can get rid of virtuoso 🙆

My command output limit is set to 100 lines:

image

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 29, 2025
@cte cte merged commit 8b072ad into RooCodeInc:main Apr 29, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Apr 29, 2025
@cte
Copy link
Collaborator

cte commented Apr 29, 2025

Thank you!

@samhvw8
Copy link
Contributor

samhvw8 commented Apr 29, 2025

Can you make a screencasts to test this ?

If we can solve this problems we can get rid of virtuoso 🙆

My command output limit is set to 100 lines:

image

Look good to me🙆

cte added a commit that referenced this pull request Apr 29, 2025
cte added a commit that referenced this pull request Apr 29, 2025
Revert "feat: syntax highlighting terminal output with Shiki (#3021)"

This reverts commit 8b072ad.
shariqriazz pushed a commit to shariqriazz/Roo-Code that referenced this pull request Apr 29, 2025
* feat: syntax highlighting terminal output with Shiki

Refactored CommandExecution.tsx to:
- Implement CodeBlock component for terminal display
- Use Shiki for syntax highlighting of shell commands and terminal output

Signed-off-by: Eric Wheeler <[email protected]>

* ui: present same compressed command output that the model will receive

Avoids rendering problems and performance regressions:
- Compresses output using BaseTerminal.compressTerminalOutput
- Eliminates need for Virtuoso by limiting output size

Ensures command output shown in UI matches the compressed format provided to the model:
- Applies same BaseTerminal.compressTerminalOutput processing
- Uses configured terminalOutputLineLimit for consistency

Signed-off-by: Eric Wheeler <[email protected]>

* test: update CommandExecution tests to use CodeBlock mock

- Replace virtuoso mock with CodeBlock mock to fix shiki import error
- Update CodeBlock mock to properly handle source prop
- Add proper empty output test case verification
- Keep original test values and structure

Signed-off-by: Eric Wheeler <[email protected]>

---------

Signed-off-by: Eric Wheeler <[email protected]>
Co-authored-by: Eric Wheeler <[email protected]>
shariqriazz pushed a commit to shariqriazz/Roo-Code that referenced this pull request Apr 29, 2025
…eInc#3023)

Revert "feat: syntax highlighting terminal output with Shiki (RooCodeInc#3021)"

This reverts commit 8b072ad.
SmartManoj pushed a commit to SmartManoj/Raa-Code that referenced this pull request May 6, 2025
…oCodeInc#3021)

* add github action for creating linear tickets for unconnected PRs

* changset

* only load fetch if not present

* omit fetch

* add error handling

* fix gql query

* only run for opened PRs

* break out into actions

* fix folders

* checkout first

* remove the actions

* add sync

* remove sync
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:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants