fix(prompts): correct spinner clearing by tracking previous rendered output#478
fix(prompts): correct spinner clearing by tracking previous rendered output#478escral wants to merge 2 commits intobombshell-dev:mainfrom
Conversation
🦋 Changeset detectedLatest commit: ccff1f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
we're actually in the middle of reworking the spinner to be a so im not sure if we should merge this. maybe instead, we should try get the spinner rework over the line will wait and see what @dreyfus92 thinks too |
There was a problem hiding this comment.
Thank you @escral for taking the time to work on this, we're planning to rework the spinner so it uses the same Prompt base class and diff rendering as our other prompts (text, select, etc.) in that design, like @43081j stated, we always diff the full rendered frame, so this clearing bug doesn't arise and the current clearing path goes away. Because the rework would replace this code path, we're hesitant to merge this fix as-is. We'd rather not add code we know we'll remove soon, we'd like to prioritise getting the spinner rework over the line.
We really appreciate the fix and the repro, we'll keep it in mind when we do the rework 💜
|
Sure, thanks for the quick response! Looking forward to the reworked spinner. |
|
ill close this and open a draft with the spinner rework 👍 will try find time in the next week to finish it off |
PR Description
First of all, sorry for not opening an issue before submitting this PR.
The issue
When the spinner message is shorter than the terminal width, but becomes longer after the
"..."suffix added by the"dots"indicator, the previous frame is not cleared correctly.Currently the clearing logic calculates the number of lines based on
_prevMessage. However,_prevMessageis not the actual rendered output. The rendered output also includes:./../...)indicator: "timer")Because
_prevMessagedoes not include these elements, the number of wrapped lines can be calculated incorrectly. As a result, the spinner sometimes fails to fully clear the previous frame.How to reproduce
A reproduction script was added:
examples/basic/spinner-wrap.tsManual reproduction steps:
"dots"indicator appends.→..→..., the message becomes longer than the terminal width.Fix
Instead of tracking
_prevMessage, the fix tracks the previous rendered output.This ensures the clearing logic uses the exact string that was written to the terminal, producing correct line calculations when wrapping occurs.
Videos
Current behavior
simplescreenrecorder-2026-03-04_14.19.03.webm
With fix
simplescreenrecorder-2026-03-04_14.17.05.webm