-
-
Notifications
You must be signed in to change notification settings - Fork 209
fix: wrap and indent spinner messages
#584
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@clack/prompts": patch | ||
| --- | ||
|
|
||
| Fix `spinner`, `progress` and `task` not wrapping or prefixing newlines in their messages. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { styleText } from 'node:util'; | ||
| import { block, getColumns, settings } from '@clack/core'; | ||
| import { block, getColumns, settings, wrapTextWithPrefix } from '@clack/core'; | ||
| import { wrapAnsi } from 'fast-wrap-ansi'; | ||
| import { cursor, erase } from 'sisteransi'; | ||
| import { | ||
|
|
@@ -127,15 +127,16 @@ export const spinner = ({ | |
| return min > 0 ? `[${min}m ${secs}s]` : `[${secs}s]`; | ||
| }; | ||
|
|
||
| const prefix = `${styleText('gray', S_BAR)} `; | ||
| const hasGuide = opts.withGuide ?? settings.withGuide; | ||
|
|
||
| const start = (msg = ''): void => { | ||
| isSpinnerActive = true; | ||
| unblock = block({ output }); | ||
| _message = removeTrailingDots(msg); | ||
| _message = wrapTextWithPrefix(output, removeTrailingDots(msg), hasGuide ? prefix : '', ''); | ||
| _origin = performance.now(); | ||
| if (hasGuide) { | ||
| output.write(`${styleText('gray', S_BAR)}\n`); | ||
| output.write(`${prefix}\n`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we keep this as just the bar? the 2 trailing spaces from prefix are why basically every snapshot in both files changed. writing trailing whitespace doesn't buy anything and would shrink the diff a ton. |
||
| } | ||
| let frameIndex = 0; | ||
| let indicatorTimer = 0; | ||
|
|
@@ -159,11 +160,7 @@ export const spinner = ({ | |
| outputMessage = `${frame} ${_message}${loadingDots}`; | ||
| } | ||
|
|
||
| const wrapped = wrapAnsi(outputMessage, columns, { | ||
| hard: true, | ||
| trim: false, | ||
| }); | ||
| output.write(wrapped); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pre-existing rather than introduced here, but this diff changes its failure mode: main: "• " <- stale stub
"o done"
this branch: "• xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" <- stale full line
"o done"so same bug, but without the write-time wrap the leftover is now a full line of content instead of a 3-char stub. proper fix is probably counting from the actually-written string (needs its own var, the |
||
| output.write(outputMessage); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like |
||
|
|
||
| frameIndex = frameIndex + 1 < frames.length ? frameIndex + 1 : 0; | ||
| // indicator increase by 1 every 8 frames | ||
|
|
@@ -182,7 +179,7 @@ export const spinner = ({ | |
| : code === 1 | ||
| ? styleText('red', S_STEP_CANCEL) | ||
| : styleText('red', S_STEP_ERROR); | ||
| _message = msg ?? _message; | ||
| _message = wrapTextWithPrefix(output, msg ?? _message, hasGuide ? prefix : '', ''); | ||
| if (!silent) { | ||
| if (indicator === 'timer') { | ||
| output.write(`${step} ${_message} ${formatTimer(_origin)}\n`); | ||
|
|
||
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.
head up that
wrapTextWithPrefixdoescolumn - prefix.lengthand this prefix has ansi codes in it, so it's ~13 chars instead of 3 visible cols. messages wrap ~ 10 columns early. not this PR's fault necessarily but passing a styled prefix here surfaces it, might be worth stripping ansi (or using visible width) in core as follow up.