-
-
Notifications
You must be signed in to change notification settings - Fork 26
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: add ratatui widget feature #38
base: master
Are you sure you want to change the base?
Conversation
Adds a new ratatui widget behind a ratatui feature flag. Implemented backed by an Arc<Mutex<Vec<u8>>> which is cleared anytime there's new info to be printed.
|
||
/// Handles the Printing logic for the Spinner | ||
#[derive(Default, Copy, Clone)] |
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.
This is technically a breaking change, but I don't think it's possible to implement this without removing Copy.
} | ||
|
||
impl Stream { |
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.
There's a lot of churn in this. In summary:
- rearranged to read well top to bottom
- methods which were passed the writer now just get self and pull the writer if needed
- write calls which would have written ansi / control characters to the stream are now separated from the calls which write info (e.g.
\r
and the erase line sequence.
/// ``` | ||
#[derive(Clone, Default)] | ||
pub struct SpinnerWidget { | ||
value: Arc<Mutex<Vec<u8>>>, |
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.
It's possible that this could have been the shared state instead of the entire widget, but I prefer passing around wrapped shared state values instead of raw values. Do you have preferences one way or another?
Thanks a lot for your submission @joshka, However I don't want to couple spinners with ratatui but however, if you have currently issues integrating spinners inside ratatui spinner widget because spinners needs some rework to make it work to make it further generec, then I'm OK if we go down that path :) |
The problem is the integration with the way this lib does the stream. Any particular reason that you don't want this? |
So let's only change that, and improve spinners genericity but not include anything related to ratatui so it says clean and futur-proof for us 🙏 Could you please update your PR in that direction ? |
Adds a new ratatui widget behind a ratatui feature flag.
Implemented backed by an Arc<Mutex<Vec>> which is cleared anytime
there's new info to be printed.