More reactive updates with enable_steady_tick()#760
More reactive updates with enable_steady_tick()#760MoreDelay wants to merge 5 commits intoconsole-rs:mainfrom
enable_steady_tick()#760Conversation
|
I don't really think it's a good idea to change the documented behavior for steady ticks in a semver-compatible release, I'm not confident in the proposed changes (and I don't have time to dig in in the near future), and I don't think it's worth a semver-incompatible release. Note that I'm not saying the idea is wholly without merit -- I can see where you're coming from and it might make sense. But as a project that mostly gets passive, volunteer maintenance, I don't think this is a change I'm able to digest soon. |
|
I expected as much, these commits are meant more to act as a basis for discussion. Would you say it is fine to add a new method that uses the new behavior, perhaps something like |
|
Probably, though I'd like a more pithy/descriptive name. |
src/progress_bar.rs
Outdated
|
|
||
| /// Behavior of a steady ticker when its progress bar is ticked | ||
| #[derive(Debug, Clone, Copy)] | ||
| pub(crate) enum TickerReaction { |
src/progress_bar.rs
Outdated
| let now = Instant::now(); | ||
|
|
||
| let do_tick = match reaction { | ||
| TickerReaction::Immediately => true, | ||
| TickerReaction::OnTimeout => match last_tick { | ||
| None => true, | ||
| Some(last_tick) => { | ||
| let passed = now - last_tick; | ||
| passed >= interval | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| if do_tick { | ||
| state.tick(now); | ||
| last_tick = Some(now); | ||
| } |
There was a problem hiding this comment.
I think this is confused about what it wants to be. If you want .tick() to be responsive, IMO it should not affect a running ticker at all.
There was a problem hiding this comment.
I know what you mean, but this would actually result in a different behavior for spinners. When the spinner thread keeps track of the last tick, then there will always be an interval pause until the next automatic tick happens. When we just do the tick outside with no communication with the thread, then these reactive ticks and the automatic ticks appear irregular.
The attached video demonstrates this, where all tickers tick once a second, while the bars increments every 1.2 seconds. The "Inside Ticker" bar uses a responsive ticker as implemented here, the "Outside Ticker" uses the original ticker implementation but also ticks on increments.
To me both approaches are fine, I just want you to be aware.
ticker-behavior.webm
There was a problem hiding this comment.
After thinking more about it, I found a way to get the desired spinner behavior without that complicated timeout logic. Now the thread is only notified when we want it to tick, so the check is moved outside.
2 participants
I have a use case where I create two progress bars where the first bar shows the overall progress whereas the second bar shows the progress of the current task, each also showing the elapsed time. A task can take a few seconds or up to a few minutes. This is exactly the kind of situation where one would use
enable_steady_tick()to let the overall progress bar update its timer.However I noticed that on task switches, the overall progress bar lags behind ever so slightly as the tick gets ignored on its increment, and thus waits until the specified interval has passed. I would like to have the overall progress bar update at the same time as the task progress bar. In this situation, I would either have to make due with a stale elapsed timer, or implement my own ticker thread logic around the bar.
So instead, I propose to add a configuration option to specify how the steady ticker should react to calls to
tick().The first commit changes the behavior such that the bar gets updated immediately on a call to
tick(), even with a running ticker. The second commit adds a configuration option to get back the old behavior if preferred. This is currently a breaking API change onenable_steady_tick(), so I'm not sure if you may want this to be a new method instead.Feel free to leave suggestions, or whether or not these changes are in scope for this project.