criterion.rs icon indicating copy to clipboard operation
criterion.rs copied to clipboard

Text is cut off if the terminal width is not large enough

Open jyn514 opened this issue 4 years ago • 13 comments

See for example the following output:

Benchmarking set allocating: Collecting 100 samples in estimated 5.0539 s (404k itera                                                                                     set allocating          time:   [13.266 us 13.757 us 14.321 us]
                        change: [+25.539% +31.449% +38.019%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe

Benchmarking set without allocating: Collecting 100 samples in estimated 5.0191 s (75                                                                                     set without allocating  time:   [10.939 us 11.464 us 12.067 us]
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

The text after 80 columns is cut off completely, instead of being printed normally. This is not at all desired behavior - criterion should print it normally and let the terminal handle word wrapping. In particular this breaks copy pasting since the text is just not there.

jyn514 avatar Aug 28 '20 19:08 jyn514

Hi @jyn514. I'm not able to reproduce this behavior. For me, the output is wrapped by the terminal. Looking at the code, it appears to be ordinary println! and eprintln!.

Are you still seeing this behavior with criterion-0.3.5? If so, which terminal are you using?

lemmih avatar Jul 27 '21 13:07 lemmih

Yes, I am still seeing this behavior with 0.3.5. I've used several different terminals and run into the same problem - currently I'm using kitty, but I think I had the same problem with Konsole.

Maybe this is some interaction between the bench framework and criterion? I'm running this with cargo bench.

Benchmarking set allocating: Collecting 100 samples in estimated 5.0050 s (2.3M iterat                                                                                      set allocating          time:   [2.8888 us 5.9331 us 9.6853 us]
                        change: [+53.967% +191.08% +338.05%] (p = 0.00 < 0.05)
                        Performance has regressed.

jyn514 avatar Aug 16 '21 02:08 jyn514

I've just tried this on Mac with the normal terminal and with Kitty and can't replicate this issue. If the terminal window is small enough that that text moves to 2 lines then print_overwritable (used here as far as I can see) will replace the second line only.

The desired effect of print_overwritable is that it's a temporary message when running bench anyway, so it seems to me that this is by design rather than a bug that it can't be copy and pasted?

jmwill86 avatar Feb 24 '22 20:02 jmwill86

Please don't design your output to be impossible to copy paste :( it makes it very difficult to share with other people unless I take screenshots, and screenshots can't go in commit messages.

jyn514 avatar Feb 24 '22 20:02 jyn514

I'm new on the repo, so the 'why' it's done that way is not really one for me to reply to. The iterations message does seem to be purely an output of the progression of the current job though, and estimate of the length of time it will take. All the important and accurate measurement are displayed on completion. There should be no reason to want to copy and paste the estimated time it would take I wouldn't have thought.

jmwill86 avatar Feb 24 '22 22:02 jmwill86

The iterations message does seem to be purely an output of the progression of the current job though, and estimate of the length of time it will take. All the important and accurate measurement are displayed on completion.

Ah, maybe this is the issue: I'm not actually interested in the progress bar, but it seems to be interfering with the final output - if you scroll right on my initial text time doesn't appear until about 200 columns in.

jyn514 avatar Feb 24 '22 23:02 jyn514

Another data point: this works fine for me if the terminal takes up the full width of the screen, it only breaks if it's windowed.

jyn514 avatar Feb 24 '22 23:02 jyn514

One thing that might be messing with this is that print_overwriteable marks the line length as the number of bytes in the string, but that's incorrect when it's colored - it needs it to be the number of columns that's used, which you can find with wcwidth.

jyn514 avatar Feb 24 '22 23:02 jyn514

Ok yup, this reproduces the issue:

$ cat example.py 
#!/usr/bin/env python3
progress = "\x1B[32mthis\x1B[39m \x1B[32mis\x1B[39m \x1B[32ma\x1B[39m very long progress string that takes up most of the screen"
final = "this is the final string"
print(progress, end='')
print("\r", end='')
print(" " * len(progress), end='')
print("\r", end='')
print(final)
$ ./example.py  # full screen terminal
this is the final string                                                                          
$ ./example.py  # half screen terminal
                                                                                             this is the final string

jyn514 avatar Feb 25 '22 00:02 jyn514

My suspicion is that \r is going back to the start of the wrapped line - here's how it looks in my terminal at half width: image so it appears as if there's two lines, not one.

jyn514 avatar Feb 25 '22 00:02 jyn514

and using print("\x1B[0K", end='') instead of print(" " * len(progress), end='') fixes the issue in the example.

jyn514 avatar Feb 25 '22 00:02 jyn514

https://unix.stackexchange.com/questions/502295/clearing-a-line-when-terminal-window-is-narrowed looks relevant.

jyn514 avatar Feb 25 '22 00:02 jyn514

I think the main issue here is that the \r carriage return wont work when the line wraps like you say. \r followed by a println!() still follows the same line when the window size is changed for some reason, so a new line has to be added when going from print_overwritable to println!

I've made some changes which work better for me when resizing the window and improve on how print_overwrite works too. If you could give it a go to see if the problem is fixed I can do a merge request? Just in case your issue is different to what I'm seeing.

Again, new here and to open source mostly so if there's an easier way, or if I should be just submitting a merge request straight off, let me know 😄

https://github.com/jmwill86/criterion.rs

jmwill86 avatar Mar 02 '22 15:03 jmwill86