fix(cli): truncate Spinner message to terminal width on TTY output (#6975)#7157
fix(cli): truncate Spinner message to terminal width on TTY output (#6975)#7157LeSingh1 wants to merge 2 commits into
Conversation
…enoland#6975) When the spinner message is wider than the terminal, `\r[K` only clears the line the cursor was on. The next frame then prints a wrapped block again, so each tick adds another stack of stale lines instead of overwriting the previous one. The repro from the issue (`new Spinner({ message: "x".repeat(1000) }).start()`) scrolls the terminal indefinitely. Truncate the rendered message so the visual width of the prefix (`spinner glyph + space`) plus the message stays within `Deno.consoleSize().columns`. Width is measured with the existing `unicodeWidth` helper so wide CJK / emoji characters that would straddle the boundary are dropped rather than half-rendered. Truncation only happens when the output is a TTY. Piped or redirected output (where wrapping is not visually catastrophic) keeps the full message so logs are still grep-able. This matches the approach BlackAsLight and tomas-zijdemans agreed on in the issue thread — simpler than save/restore cursor and self-contained within `@std/cli/unstable-spinner`. Adds three tests in `cli/unstable_spinner_test.ts`: ASCII truncation to a stubbed 20-column terminal, emoji truncation to 10 columns (4 of the 10 input dinos fit), and non-TTY no-truncation. All 14 spinner tests pass; lint and typecheck clean.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7157 +/- ##
==========================================
- Coverage 94.57% 94.57% -0.01%
==========================================
Files 636 636
Lines 52138 52161 +23
Branches 9399 9407 +8
==========================================
+ Hits 49311 49330 +19
- Misses 2249 2251 +2
- Partials 578 580 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think enabling and disabling auto wrap would be far more efficient and reliable than counting the string and truncating it by hand. Simply put The terminal will truncate the string itself then and this is far more reliable as there isn't a good way to tell which UTF-8 version a terminal is using and may have slightly different behaviour around less common characters. |
Per @BlackAsLight's review on denoland#7157: drop the unicode_width dependency and let the terminal do the truncation by toggling DECAWM (\\u001b[?7l off / \\u001b[?7h on) around each frame. The terminal silently drops anything past the right edge so \\r\\u001b[K keeps clearing the same row. Avoids guessing display widths for emoji, ZWJ sequences, combining marks, and unusual terminal Unicode handling — the terminal already knows. Drops the truncateToWidth + #terminalColumns helpers and the unicodeWidth import. The TTY-vs-not check is preserved: on non-TTY output we don't emit the toggles, so piped/redirected output stays clean of escape bytes. Tests updated: - The TTY case now asserts the full frame is bracketed by DECAWM toggles (no user-space truncation expected). - The non-TTY case unchanged in spirit (full message, no toggles). - The wide-character/emoji truncation test is removed: width is no longer computed in JS.
|
Addressed in 1fe7d55. Dropped the unicode_width import + the JS-side width math and now toggle DECAWM around each TTY frame, exactly as you suggested: Net diff: -64 lines. On non-TTY output the toggles are skipped so piped/redirected output stays clean of escape bytes. Tests updated:
All 13 spinner tests pass. |
Fixes #6975.
Spinnerclears the current line with\r\x1b[Kbefore each frame, but when the rendered message is wider than the terminal, the cursor is now on the wrapped line and the clear only nukes that one. Each tick then prints a fresh wrapped block, so the output scrolls indefinitely. Repro from the issue:Followed the truncation approach BlackAsLight and tomas-zijdemans agreed on in the thread — simpler than save/restore cursor and self-contained in
@std/cli/unstable-spinner. Truncate the message soprefixWidth (glyph + space) + unicodeWidth(message) ≤ columns. Width is measured with the existingunicodeWidthhelper so wide CJK / emoji characters that would straddle the boundary are dropped, not half-rendered.Only truncates when the output is a TTY (
Deno.stdout.isTerminal()etc.). Piped or redirected output gets the full message untouched so logs stay grep-able.Tests
Three new cases in
cli/unstable_spinner_test.ts:truncates message to terminal width when output is a TTY— 50-x message, 20-column terminal, asserts each frame ends at 18 x's.does not truncate when output is not a TTY— stubsisTerminal()false withconsoleSizereporting 10 columns, asserts the full 50-x message is rendered.truncates respecting wide-character (emoji) width— 10 dinos in 10 columns yields 4 (each 🦕 is 2 columns).