zig icon indicating copy to clipboard operation
zig copied to clipboard

`std.Progress`: fix inaccurate line truncation and use optimal max terminal width

Open wooster0 opened this issue 1 year ago • 1 comments

I really had a lot of attempts at this and I think this might be the best one so far. I think there's still some things to improve but this is definitely a start.

Output now fits nicely on pretty much any terminal width (well, not if it's >256 though, but that limit can easily be bumped): image

Here's another common issue that I think Andrew described in https://github.com/ziglang/zig/issues/12024#issuecomment-1176803566:

image

That issue seems to be fixed now; here's a compilation on a terminal with a very small width:

https://user-images.githubusercontent.com/35064754/178335637-3a187e07-9cc9-4cc5-9ac3-45bda66ebc95.mp4

It succeeds with nothing left on the terminal.

Closes #12024

wooster0 avatar Jul 11 '22 18:07 wooster0

samu: job failed: cd /usr/home/build/zig && /usr/home/build/zig/build/zig0 src/stage1.zig -target x86_64-freebsd-gnu -mcpu=baseline --name zig1 --zig-lib-dir /usr/home/build/zig/lib -femit-bin=/usr/home/build/zig/build/zig1.o -fcompiler-rt -OReleaseFast --strip -lc --pkg-begin build_options /usr/home/build/zig/build/config.zig --pkg-end --pkg-begin compiler_rt /usr/home/build/zig/lib/compiler_rt.zig --pkg-end
./lib/std/os.zig:181:27: error: container 'std.c' has no member called 'termios'
pub const termios = system.termios;
                          ^

So FreeBSD doesn't have termios which I use to get the terminal width...

Or maybe FreeBSD does have it but we just don't have the things exposed in https://github.com/ziglang/zig/blob/master/lib/std/c/freebsd.zig?

I've limited it to Linux (and Windows) for now. Good enough for a start.

wooster0 avatar Jul 11 '22 19:07 wooster0

Thanks for the deep dive into this problem!

andrewrk avatar Oct 13 '22 10:10 andrewrk

This did not solve the problem of resizing the terminal window while progress is being printed:

image

andrewrk avatar Oct 13 '22 10:10 andrewrk

Yes, this behavior is as expected.

The main problem I focused on to solve here was the one you originally described where if the line goes beyond the end of the terminal then the output becomes malformed. Resizing the terminal while progress is being printed and then the output becoming malformed is a different, probably more difficult problem.

It's because getTerminalWidth, a function we would definitely need if we wanted to fix this somehow, is only called once at the start: https://github.com/ziglang/zig/blob/0eb3b8fa44f6dcafad8671695688d13ba8daba9a/lib/std/Progress.zig#L179 So after that we don't recalculate any values based on the terminal width. Well, it's only really max_width that we have to calculate based on terminal width and cursor position.

It seems I talked a bit about it in https://github.com/ziglang/zig/issues/12024#issuecomment-1177831180 as well.

There are two ways I can think of that may be useful here for attempting to fix this:

  1. Detect terminal width changes based on some callback. The only option I can think of at the moment is if we listen to the SIGWINCH ("signal window change") signal on POSIX systems and then call getTerminalWidth if we catch it. Next question would be how to do this on Windows.
  2. Periodically call getTerminalWidth. When would we call it? Before every refresh in refreshWithHeldLock? Well here's a scenario: we printed some progress, then we sleep for say 2 seconds for whatever reason and then during those 2 seconds the user resizes their terminal: the user will immediately notice that characters are overflowing off the right of the terminal down to the next line, and from there on it's kind of hard to recover the previous state. What could we do? Clear the line below to clear the characters that overflowed? I think there are a number of ways we could kind of fix this, but it would need some more thought.

I did try to put a self.calculateMaxWidth(); right here: https://github.com/ziglang/zig/blob/0eb3b8fa44f6dcafad8671695688d13ba8daba9a/lib/std/Progress.zig#L286 but I couldn't really notice a difference. It certainly didn't fix that issue you're describing.

Maybe we can come up with a way to fix this someday. Another thing we could do of course is enabling the terminal's alternate screen buffer which would give us full control and then we could just clear the screen if the terminal is resized. I guess for now what std.Progress expects is that once some process of work is started, std.Progress does its progress output and expects the user to lean back and wait and not adjust the terminal size any further until std.Progress is done.

wooster0 avatar Oct 13 '22 12:10 wooster0