console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.21k stars 238 forks source link

Fix move cursor flag not working #648

Closed SuperTux88 closed 2 weeks ago

SuperTux88 commented 3 weeks ago

I had the problem of my progress bars starting to flicker after adding some more bars, especially over an ssh connection. I then found #42 and from that then found #143, but the fix described there to set m.set_move_cursor(true); didn't make a difference at all.

After some debugging, I found out, that the move_cursor flag was used in the draw_to_term() function, but this is only called from a Drawable::Term, while the set_move_cursor was only set on the MultiState which is only used in the Drawable::Multi. So in the current state, it never has any effect when actually drawing to the terminal.

Then after forwarding the flag to the correct DrawState instance, I noticed, that the first bar always starts drawing one line too high on the last char, so the first line was just off-by-one. This was because the cursor is always on the last char of the last line, and then moving up the number of bars, will always end up one line too high. So just move up one line less, but then the cursor also still needs to be moved to the front of the line.

So this now fixes using of set_move_cursor(true) again, so it can be used to fix the flickering.

SuperTux88 commented 2 weeks ago

Thanks for the review.

Would you be able to add a test that can help us avoid regressing this in the future?

I tried to do that, but failed, because the output looks the same, whether it moves the cursor or uses clear, so the test was always green, even when it was broken (so it wasn't useful). But I just noticed moves_since_last_check() where I can check for moves specifically, and added a test using that. So it now fails when it gets broken. :+1: