search

ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.26k stars 2.42k forks source link

`std.Progress` caps width far below terminal width #13440

Open nektro opened 1 year ago

nektro commented 1 year ago

Zig Version

0.11.0-dev.52+42755a194

Steps to Reproduce and Observed Behavior

./build/zig2 build test-cases -Dskip-non-native -Dskip-release -Dskip-single-threaded

image

Expected Behavior

using the full terminal width

andrewrk commented 1 year ago

This is a known limitation of the way std.Progress works right now. Would be nice if there were a way to solve this.

ghost commented 1 year ago

It looks like the problem comes from output_buffer having fixed length. For it to work with the terminal width, it would have to be dynamically allocated. (Or maybe just pick a large upperbound, like 500?)

mitchellh commented 1 year ago

As a guy who has been learning too much about escape codes recently 😉 I took a look and would suggest perhaps a different approach. I just Iike really quickly looked at how Progress was implemented, I didn’t see how Progress was being used so this whole response might be dumb.

Right now, std.Progress uses ESC[nD to move the cursor back n so it needs to keep track of columns. That’s fine, totally workable. But another approach would be to call ESC[6n at the start of a refresh to query cursor position, which will return the <row, col>. Then just output the whole progress no matter the length and let the terminal wrap it if it has to (auto wrap). Then for the next call use ESC[H to set the cursor position back to an absolute row, col that was queried earlier.

You can have a call to throw away the prev position if you want to preserve it (i.e. you outputted test failure information you want to preserve and not overwrite since the last progress call).

Another approach is to query the terminal size, but this requires making a ioctl call which you may or may not want to deal with in the std.Progress implementation. Another hack that totally works is to use ESC[H with an absurdly large number (say, 10000, 10000) which sets the cursor position. The terminal emulator must bound the cursor position by the terminal size so if you do that then call ESC[6n it’ll report the size in grid cells.

Each of these might require small public API changes to Progress though, but I’m very sure this problem can be solved without heap allocations.

nektro commented 1 year ago

how feasible would a loop like below work for std.Progress?

mitchellh commented 1 year ago

That approach would require you keep track of terminal width. All the line-oriented escape codes treat a line as a visible line (or physical line). It doesn’t unwrap soft-wrapped stuff. So if a terminal soft wraps, that’s 2 lines and you’ll start getting artifacts unless you’re careful to never wrap.

wooster0 commented 1 year ago

I've already gone through this in my PRs:

12079 #13085 #13148 https://github.com/ziglang/zig/pull/13148#issuecomment-1283084057 https://github.com/ziglang/zig/commit/1952dd6437a73e3de211b649924a55fcb6e030be

The status is that there's some weird issue going on that caused my main PR to be reverted

wooster0 commented 1 year ago

That "weird issue" is very likely coming from this function https://github.com/ziglang/zig/pull/12079/files#diff-f05e9ed61d2716c50e09b00c1881f44cdb2a85107d1b5c95231f80e489ea265eR225 which uses ESC[6n that @mitchellh mentioned. It messes with termios which would be great if we could avoid it but there just doesn't seem to be a way to avoid it.

Querying the terminal window size is pretty easy and much less troublesome: https://github.com/ziglang/zig/pull/12079/files#diff-f05e9ed61d2716c50e09b00c1881f44cdb2a85107d1b5c95231f80e489ea265eR205

wooster0 commented 1 year ago

@nektro doesn't work if you have multiple std.Progress instances from different processes (they can't communicate with each other) so if you do ESC[0K you might clear the progress of some other process behind it (see https://github.com/ziglang/zig/pull/13148#issuecomment-1285502143)

wooster0 commented 1 year ago

Here's how my PR determines the optimal width to use for the terminal when a std.Progress is created: https://github.com/ziglang/zig/pull/12079/files#diff-f05e9ed61d2716c50e09b00c1881f44cdb2a85107d1b5c95231f80e489ea265eR179-R181 It gets the terminal width and then the position of the cursor in the terminal. Then we just subtract the position from the terminal width so that we account for text printed before us.

nektro commented 1 year ago

@r00ster91 then you could make it so they have a reference to a parent that they call to print before printing their own status

wooster0 commented 1 year ago

That would involve communication between entirely separate processes right? For example if you have multiple separate zig invocations. I think in that case the solution of getting the terminal cursor's x-axis is pretty simple, but maybe there is a simpler or more reliable solution? If we could communicate with std.Progresss of other system processes, we can solve it that way too. Not sure how we could obtain such a reference to a parent.