Skip to content
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

Do not use boxcar windowing when displaying the final total of the speed #381

Merged
merged 1 commit into from Feb 18, 2022

Conversation

Yatekii
Copy link
Contributor

@Yatekii Yatekii commented Feb 17, 2022

This is especially relevant when speed varies a lot. Then confusing things like this probe-rs/probe-rs#801 can happen. This is unfortunately only barely, if at all, visible in the examples as speed, whilst varying a little, is steady.

src/style.rs Outdated
.unwrap(),
"per_sec" => {
if matches!(&state.status, Status::InProgress) {
buf.write_fmt(format_args!("{:.4}/s", state.per_sec()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to this change, but this way seems unnecessarily repetitive. It looks like we can put the logic here into a separate ProgressState method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! How about now? Not sure you want it always behave like that but it felt like a good thing to me :) In the end you want the overall speed and not the current one in any case I feel like :)

@djc djc merged commit b5f8dfe into console-rs:main Feb 18, 2022
@djc
Copy link
Collaborator

djc commented Feb 18, 2022

That's much better, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants