-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix: Cursor Positioning After Line Wrap #1943
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1943 +/- ##
=======================================
Coverage ? 94.75%
=======================================
Files ? 37
Lines ? 1562
Branches ? 456
=======================================
Hits ? 1480
Misses ? 80
Partials ? 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @param {number} width | ||
| * @return {string} | ||
| */ | ||
| export function breakLines(content: string, width: number): string { |
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.
One thing here that isn't clear anymore about this approach;
We were talking of how the bug reported seemed to be an issue with how different terminal auto-wrap content and how that conflicted with the cursor position and the way wrap-ansi would do the wrapping.
Given that, it's unclear why a different wrapping algorithm fix this issue. This would fix it for terminals doing simple wraps, but won't it now break for those who're maybe implementing word wrapping instead?
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.
By implementing character-based wrapping directly in the code rather than relying on wrap-ansi we Make wrapping deterministic, so the exact same wrapping happens on all terminals. So since we control the wrapping, we can accurately calculate cursor positions.
Now a terminal's auto-wrap behavior becomes irrelevant because we've already wrapped the content at exactly width characters, and the terminal receives pre-wrapped lines that fit within its display width. So even if a terminal has word-wrapping enabled, it only applies to content that exceeds the width, which ours won't
Example
Terminal A (word-wrap): "This is a long message"
Terminal B (char-wrap): "This is a long message"
using wrap-ansi:
- Terminal A might wrap: "This is a long" / "message"
- Terminal B might wrap: "This is a lo" / "ng message"
- Cursor calculation broken because we don't know which one we're on!
character-based:
- All terminals receive: "This is a lo" / "ng message"
- Both terminals see it's already wrapped
- Cursor calculation is accurate because we control the wrapping!
Summary
Implemented a character-based ANSI-safe wrapper directly in the
breakLinesfunction to force deterministic wrapping across all terminals:breakLinesfunction now uses character-based wrapping instead of word-based wrappingFixes: #1818
Testing
All existing tests pass. Run:
demo
https://vimeo.com/1147364471?fl=pl&fe=sh