Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tui): start pty with matching TUI size #9101

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

chris-olszewski
Copy link
Member

Description

Fixes #9060

From the issue

I think I see the issue, it appears that rspack adds enough trailing spaces in the progress bar output the fill the current line:

● ESC[1mESC[0m ESC[32mESC[37mESC[2m━━━━━━━━━━━━━━━━━━━━━━━━━ESC[0mESC[0m (0%) ESC[2msetup compilation                                                                                                                                           ESC[0m^MESC[2K

The ESC[2K operation at the erases the current line, but if the trailing spaces have caused the line to wrap, it won't erase the intended line instead just the line overflow. This probably indicates we have a mismatch in the PTY dimensions and the virtual terminal dimensions so rspack is outputting too many spaces for the terminal.

I'll look at making sure the underlying PTY has the exact same dimensions as the virtual terminal.

There is future work to make sure that we update underlying PTY instances if the pane changes sizes, but that requires a pretty heavy refactor to our process management code to communicate with the PTY "controller" as we only hold onto the "receiver".

Testing Instructions

Before:
Screenshot 2024-09-03 at 9 34 21 AM

After:
Screenshot 2024-09-03 at 9 30 20 AM

Copy link

vercel bot commented Sep 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 3, 2024 1:36pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-gatsby-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-native-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-svelte-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-tailwind-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm
examples-vite-web ⬜️ Ignored (Inspect) Sep 3, 2024 1:36pm

Copy link
Contributor

@dimitropoulos dimitropoulos left a comment

Choose a reason for hiding this comment

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

I tested with

$ npx create-turbo@latest -e with-shell-commands _reproduce -m npm && cd _reproduce && npm create rsbuild@latest -- -d ./apps/app -t react-ts && npm i && npx turbo build --ui-tui

Then I ran again with the local build and I got the intended fix as advertised!

@chris-olszewski chris-olszewski merged commit 29fe5ef into main Sep 5, 2024
40 checks passed
@chris-olszewski chris-olszewski deleted the olszewski/fix_tui_pty_size branch September 5, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Support stdout updates] Every progress bar update occupies a new line
2 participants