console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.36k stars 241 forks source link

Spinners no longer advance #433

Closed rteabeault closed 2 years ago

rteabeault commented 2 years ago

It appears that commit 6697ac3472010e54589c14a485512da543b0c923 has broken spinners. After this commit spinners no longer spin.

Run cargo run --example yarnish before and at this commit to see the difference.

chris-laplante commented 2 years ago

Good catch, thanks! I'm working on a fix now and will add a test so it doesn't happen again.

chris-laplante commented 2 years ago

@djc any idea why we check for tick != 0?

This broke because I removed the check for a ticker being installed (since BarState no longer knows about the ticker). But I wonder whether we can just change this to always increment tick?

https://github.com/console-rs/indicatif/blob/2ca9d019fbb4b0aed11bfad984daef064998f5ef/src/state.rs#L105-L109

There's a similar check in ticker which I also question:

https://github.com/console-rs/indicatif/blob/2ca9d019fbb4b0aed11bfad984daef064998f5ef/src/progress_bar.rs#L594-L598

djc commented 2 years ago

No, I don't quite understand why we check for tick != 0. I think this was introduced all the way back in a889b3ae0ea4c77ebe5b6df9e5af527cb703c9a9? My intuition was that it's maybe a sentinel for if there's even a spinner kind of thing that we need to tick, but I don't think that's it?

@mitsuhiko do you remember what the intention there was?