-
Notifications
You must be signed in to change notification settings - Fork 735
feat(terminal): upgrade xterm.js to 6.1.0 with synchronized output support #2792
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
base: main
Are you sure you want to change the base?
Conversation
|
|
WalkthroughThis PR upgrades xterm.js and related addons to 6.1.0-beta and updates package.json accordingly. FitAddon was migrated to the public xterm 6.1 API, adding a public Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Replace private import from @xterm/xterm/src/browser/renderer/shared/Types with public type import from @xterm/xterm - Replace core._renderService.dimensions with terminal.dimensions public API - Add null check for terminal.dimensions (may be undefined before open()) - Replace scrollbar DOM measurement (core.viewport._viewportElement/scrollArea) with constant DEFAULT_SCROLLBAR_WIDTH = 15 (matching xterm.js) - Remove core._renderService.clear() call (resize handles re-render internally) - Preserve noScrollbar flag for macOS scrollbar behavior - Update file comments to reflect xterm.js 6.1.0 update This refactoring is required for the xterm.js 6.1.0 upgrade as the private APIs used previously were removed in xterm.js 6.x viewport/scrollbar overhaul. Related: spec-001-fitaddon-refactor.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update all @xterm/* packages to latest beta versions to enable: - Synchronized Output (DEC mode 2026) for TUI animation fixes - Public terminal.dimensions API (added in 6.1.0) Package versions: - @xterm/xterm: 5.5.0 → 6.1.0-beta.106 - @xterm/addon-fit: 0.10.0 → 0.12.0-beta.106 - @xterm/addon-search: 0.15.0 → 0.17.0-beta.106 - @xterm/addon-serialize: 0.13.0 → 0.15.0-beta.106 - @xterm/addon-web-links: 0.11.0 → 0.13.0-beta.106 - @xterm/addon-webgl: 0.18.0 → 0.20.0-beta.105 Relates to: wavetermdev#2787 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update xterm.css to match xterm.js 6.1.0 CSS structure - Add new DomScrollableElement scrollbar classes - Add scrollbar slider styles for new xterm-scrollable-element - Keep legacy webkit-scrollbar styles for compatibility - Add VS Code scrollbar shadow and visibility class support Relates to: wavetermdev#2787 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add scrollbar width validation to prevent invalid dimensions - Add comments explaining terminal.dimensions initialization state - Document legacy webkit scrollbar CSS and xterm.js 6.x behavior - Update FitAddon header comment to reflect all changes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace all powershell commands with pwsh -NoProfile in Taskfile.yml to prevent profile scripts from corrupting build output (fixes ldflags contamination causing Go linker failures) - Add -NoProfile flag to PowerShell shell launches in shellexec.go for local, remote, and WSL shells to prevent profile loading at runtime Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cf2ad85 to
a86f347
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Taskfile.yml`:
- Around line 11-12: The Taskfile.yml change replaced PowerShell usage with pwsh
in the RMRF and DATE variables (symbols RMRF and DATE), which will break Windows
hosts without PowerShell Core; update Taskfile.yml to either (a) use a fallback
that tries pwsh and falls back to powershell (e.g. conditional/ternary around
pwsh vs powershell for the RMRF and DATE commands) or (b) revert to using
powershell -NoProfile if no pwsh-specific features are required, and if you
intentionally require pwsh add a note to BUILD.md declaring PowerShell Core as a
prerequisite; apply this change to the RMRF and DATE definitions and any other
places where pwsh was introduced.
🧹 Nitpick comments (4)
frontend/app/view/term/xterm.css (3)
195-202: Inconsistent indentation: tabs used instead of spaces.Lines 196-197 and 201 use tab characters for indentation while the rest of the file uses spaces. Consider normalizing to spaces for consistency.
♻️ Suggested fix
.xterm-screen .xterm-decoration-container .xterm-decoration { - z-index: 6; - position: absolute; + z-index: 6; + position: absolute; } .xterm-screen .xterm-decoration-container .xterm-decoration.xterm-decoration-top-layer { - z-index: 7; + z-index: 7; }
227-230: Consider documenting or avoiding!important.The
!importantonfont-size(line 229) may be necessary to override VS Code's scrollbar defaults, but it's generally a code smell. If it's required for the override to work, adding a brief comment explaining why would help future maintainers.
232-241: Minor formatting inconsistency.Line 236 is missing a space after the colon in
background:rgba(0,0,0,0). For consistency with the rest of the file, add a space.♻️ Suggested fix
.xterm .xterm-scrollable-element > .visible { opacity: 1; /* Background rule added for IE9 - to allow clicks on dom node */ - background:rgba(0,0,0,0); + background: rgba(0,0,0,0); transition: opacity 100ms linear; /* In front of peek view */ z-index: 11; }Taskfile.yml (1)
588-599: Consider consolidating into a single PowerShell script block.These 12 separate
pwshinvocations will spawn 12 processes, adding overhead compared to the Unix equivalent. For a build script this is likely acceptable, but if build performance on Windows becomes a concern, these could be consolidated into a single script block.♻️ Optional: Consolidated script block
- - pwsh -NoProfile -Command New-Item -ItemType Directory -Force -Path scaffold - - pwsh -NoProfile -Command Copy-Item -Path ../templates/package.json.tmpl -Destination scaffold/package.json - - pwsh -NoProfile -Command "Set-Location scaffold; npm install" - - pwsh -NoProfile -Command Move-Item -Path scaffold/node_modules -Destination scaffold/nm - - pwsh -NoProfile -Command Copy-Item -Recurse -Force -Path dist -Destination scaffold/ - - pwsh -NoProfile -Command New-Item -ItemType Directory -Force -Path scaffold/dist/tw - - pwsh -NoProfile -Command Copy-Item -Path '../templates/*.go.tmpl' -Destination scaffold/ - - pwsh -NoProfile -Command Copy-Item -Path ../templates/tailwind.css -Destination scaffold/ - - pwsh -NoProfile -Command Copy-Item -Path ../templates/gitignore.tmpl -Destination scaffold/.gitignore - - pwsh -NoProfile -Command Copy-Item -Path 'src/element/*.tsx' -Destination scaffold/dist/tw/ - - pwsh -NoProfile -Command Copy-Item -Path '../ui/*.go' -Destination scaffold/dist/tw/ - - pwsh -NoProfile -Command Copy-Item -Path ../engine/errcomponent.go -Destination scaffold/dist/tw/ + - | + pwsh -NoProfile -Command { + New-Item -ItemType Directory -Force -Path scaffold + Copy-Item -Path ../templates/package.json.tmpl -Destination scaffold/package.json + Set-Location scaffold; npm install + Move-Item -Path node_modules -Destination nm + Set-Location .. + Copy-Item -Recurse -Force -Path dist -Destination scaffold/ + New-Item -ItemType Directory -Force -Path scaffold/dist/tw + Copy-Item -Path '../templates/*.go.tmpl' -Destination scaffold/ + Copy-Item -Path ../templates/tailwind.css -Destination scaffold/ + Copy-Item -Path ../templates/gitignore.tmpl -Destination scaffold/.gitignore + Copy-Item -Path 'src/element/*.tsx' -Destination scaffold/dist/tw/ + Copy-Item -Path '../ui/*.go' -Destination scaffold/dist/tw/ + Copy-Item -Path ../engine/errcomponent.go -Destination scaffold/dist/tw/ + }
Document that PowerShell 7 (pwsh) is required for Windows builds. Windows ships with PowerShell 5.1, but the build system uses pwsh with -NoProfile to prevent profile scripts from interfering with build output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Upgrades xterm.js from 5.5.0 to 6.1.0-beta.106, enabling Synchronized Output (DEC mode 2026) support. This eliminates visual tearing and flickering in TUI applications.
Key Changes
terminal.dimensionsAPI instead of private internalspwsh -NoProfileto prevent profile interferenceCommits
refactor(fitaddon): Update to xterm.js 6.1.0 public APIchore(deps): Upgrade xterm.js to 6.1.0-beta.106style(terminal): Update CSS for xterm.js 6.1.0 scrollbarfix(terminal): Address code review findingsfix(windows): Use pwsh with -NoProfile for build and shellNew Features from xterm.js 6.x
Breaking Changes Handled
.xterm-scrollable-elementTest plan
npm installprogress bars animate in-placehtoporbtoprenders without tearing🤖 Generated with Claude Code