search

console-rs / indicatif

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

Empty lines in `MultiProgress::println` leads to lost output #606

Closed tgolsson closed 6 months ago

tgolsson commented 7 months ago

Associated repro repo: https://github.com/tgolsson/indicatif-repro. It expects the indicatif repo to be checked out adjacently.

I'm debugging this issue in Pantsbuild: https://github.com/pantsbuild/pants/issues/20171, and have isolated a reproduction for a bad print related to multiple repeated \n. On line 225 in that repository, there's a \n\n before the final line. That leads to losing the final line. If it's a single \n, the last line is printed. As far as I can tell while playing around with that message; each time \n\n is seen we lose one line of output from the end. So \n\n seen three times means we lose three lines from the end, and so on.

I'm 95% confident that the error exists here: https://github.com/console-rs/indicatif/blob/f33789e8466dd0ca64a05859244eddf098922bf1/src/multi.rs#L278-L282

As that will count "zero-width" lines as being no line at all. Indeed, changing that to floor + 1 fixes the issue. It also fixes the issue for single progress bars (that are part of multiprogress), which I guess is because they share state somehow? You can play around with it on my repo by printing to pb instead of multiprogress, again on line 225.

tgolsson commented 7 months ago

I think my fix is maybe a bit too simplistic on the other edge now that I think about it, but if the location feels right I can do a robust solution with some proper tests.

djc commented 7 months ago

I don't remember all the context here, but it looks to me like real_len() is about the line width, not about the number of lines? @chris-laplante do you have an intuition for this?

tgolsson commented 7 months ago

As far as I can tell, it is the "sum of wrapped line count", i.e., number of lines once wrapping is applied.

chris-laplante commented 7 months ago

I don't remember all the context here, but it looks to me like real_len() is about the line width, not about the number of lines? @chris-laplante do you have an intuition for this?

I don't have this stored in my head anymore unfortunately :/. I will try to take some time over the coming holiday weeks to look at it.