-
Notifications
You must be signed in to change notification settings - Fork 632
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
feat(cli): make Spinner.message
able to be changed on-the-fly
#4079
Conversation
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.
I like the idea of this. Thank you.
It does make me think that this same change could be applied to some of the other properties.
Like |
Yep 👍🏾 |
Should that be part of this PR or a separate one? |
Let's have it part of this one. |
Actually, |
Ok, I updated the code so it works when changing Example: import { Spinner } from "https://deno.land/std@$STD_VERSION/cli/spinner.ts";
import { delay } from "https://deno.land/std@$STD_VERSION/async/delay.ts";
const spinner = new Spinner({ message: "Loading..." });
spinner.start();
await delay(1000);
spinner.message = "Almost done...";
spinner.color = "blue";
spinner.interval = 25;
await delay(1000);
spinner.stop(); |
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.
Frankly, I don't get some of these changes. I didn't think making color and friends would introduce such logic.
Sorry to change my mind, but can we revert to focusing on making Spinner.message
public? Sorry for my previous suggestion. We should always keep PRs focused.
cli/spinner.ts
Outdated
#color?: Color; | ||
#intervalId: number | undefined; | ||
#active = false; | ||
#frameIndex = 0; |
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.
What's the purpose of this change?
Ok, I reverted the changes and will open other PRs for the |
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.
Nice! LGTM. Thank you.
Spinner.message
propertySpinner.message
able to be changed on-the-fly
Spinner.message
property publicReason: Allows to change message during active spinner.
Example: