std.Progress: tree form and line truncation #17165

DISTREAT · 2023-09-15T16:57:26Z

As described in the issue (closes #14946; potentially resolves #13440), this PR converts the progress bar into a tree form and resolve the issue of smaller-sized terminals, where lines wrap automatically.

I've tested the changes both on Linux and Windows (it would make sense to test against additional environments) with mixed benchmark results, the difference when embeded into applications is only marginal. Nonetheless, it would be a good idea to consider other implementations as well for an improved performance, but I assume this first approach isn't bad, since I've also tried using linked lists which performed worse.

Just like before there are zero dynamic allocations, but since I am not the best at multi-threading it might also make sense for someone that understands atomic handling of data to look over the PR.

Merging this PR wouldn't be seamless because the frontend is still on par with the old design (ex. unnamed nodes).

There should be no breaking API changes outside the scope of the module itself, apart from a changed behavior.

I'd be glad to continue on helping and make the changes necessary to satisfy the needs and integrate it into the other existing components.

Vexu

I don't know about the Windows stuff but the Linux side LGTM.

lib/std/Progress.zig

This is useful for a lower-level interaction with the module. An example would be the post-processing of the buffer before writing to `terminal`.

Following the commit that made the output_buffer end index become its own field, it's also reasonable to write to the output_buffer even if the terminal field is null.

Co-authored-by: Veikka Tuominen <[email protected]>

lib/std/Progress.zig

DISTREAT · 2024-04-22T18:26:30Z

I am not entirely satisfied with the implementation. To be more precise, I have performance and design concerns, despite the addressed features inevitably increasing complexity and reducing performance.

Additionally, the whole max-width trimming methods are somewhat hard to read, and maybe storing the buffer differently, say using a nested list, could reduce some of that complexity.

Not sure how I could improve upon these changes: maybe someone else has got some ideas? Though, it could also be that I am being critical of my work and the suggested changes are mostly fine.

This merge resolves conflicts caused by the removal of `termColor` in `tools/docgen.zig`.

Vexu

Even if not perfect it's still an improvement over the status quo.

DISTREAT · 2024-04-28T23:56:35Z

Thank you for approving the changes.

I will try to come up with a more elegant way of storing the buffer - probably as a nested array, since this allows for a concise separation of each row. At the moment, the code keeps track of each and every LF in a "flattened" buffer. Nesting should improve performance and maintainability.

Also, I took note of the failing CI checks and will address the issue. The checks fail due to code being moved away from a docgen function in favor of refactoring into doctest. I did not take note of the changes and assumed the function was removed altogether, my bad.

This refactor should improve readability by utilizing a nested buffer instead of a one-dimensional buffer. The upside is better maintainability. Before this commit, the idea was to keep track of LFs in a single buffer.

The internal structure of `std.Progress` changed and due to the low-level approach of `main.zig` this leads to issues. This commit makes use of methods provided by `std.Progress` and `serveStringMessage` to resolve these issues.

This commit addresses the issue of nodes without name or eta causing a newline. The resulting output is unexpected.

This commit serves as a preparation for restoring the original CI compilation output since the frontend is not making use of the new behavior yet.

Emulate the old progress bar everywhere to provide a decently seamless migration to the new progress bar. In addition, this commit makes it easier to manually initiate a progress bar without using the `start` method.

DISTREAT · 2024-04-30T19:53:49Z

I am not entirely sure what made the CI fail before, but it seems to be building fine on my machine, so I hope the issues are resolved now. In addition, I added support for the old progress bar in the hopes of avoiding a messy output.

andrewrk · 2024-05-01T00:00:20Z

By the way I did check this branch out locally and play with it, and I couldn't get it to work.

I ran zig build test-behavior and observed no difference between master branch and this branch.

DISTREAT · 2024-05-01T00:05:03Z

By the way I did check this branch out locally and play with it, and I couldn't get it to work.

I ran zig build test-behavior and observed no difference between master branch and this branch.

This is because the progress bar implements a mode where it mimics the old design. You can disable it by setting emulate_one_line_bar to false.

It is disabled by default, but I enabled it for all files since I did not want to mess with CI logs during the transitioning phases.

Moreover, I originally implemented it because on "dumb" terminals the single-line progress bar looks arguably better.

This patch removes assert statements in windows that were causing issues due to the assumption that every `terminal` is a TTY.

DISTREAT · 2024-05-01T18:31:10Z

I resolved the Windows-related issues now.

The other additional commit should not cause any issues whatsoever.

Therefore, Linux should continue to pass absolutely fine and it would be cool if we could run the Windows CI again.

Vexu · 2024-05-01T18:48:46Z

What's the point of this change if you disable it everywhere?

DISTREAT · 2024-05-01T19:05:37Z

What's the point of this change if you disable it everywhere?

The logging by the CI does not support ansi escape sequences and is therefore considered a "dumb" terminal. In such case, a LF is written instead of ansi esc seqs that clear the previous output.

The issue is that when you are printing multiple lines, not clearing the previous output this gets rather hard to interpret, as compared to the current single-line progress bar.

A suggested middle ground would be to add checks outside of std.Progress so that on "dumb" terminals the progress bar automatically defaults to the old style.

The current state of the PR solely suggests an extended version that supports multi-line progress bars, allowing for an easy migration when and where a multi-line progress bar is desired.

In addition, this PR adds automatic truncation to the terminal width which should already be noticible.

DISTREAT · 2024-05-01T19:31:55Z

Let me clarify even further. The current state of main.zig (, etc.) is designed for the old behavior of the progress bar. Of course, the API stayed the same and is backwards compatible but in many places the new progress bar seems out of place.

Since the build system is currently being reworked towards something that is a good fit for the new design, I did not bother to bring main.zig (, etc.) on par with the changes.

You can test it for yourself. The compilation output of the CI workflow looks bad with emulate_one_line_bar set to false. In my opinion, this isn't because std.Progress isn't capable of beautiful progress outputs but rather because the frontend doesn't yet make use of the features introduced.

I don't want to suggest changes that make it so contributers, trying to investigate why the CI failed for their PR need to scroll two miles, because the CLI isn't following the new behavior of the progress bar.

If I've got time this weekend, I will take try to integrate the reworked progress bar.

andrewrk · 2024-05-25T22:14:32Z

Closing in favor of #20059.

DISTREAT force-pushed the master branch 4 times, most recently from 068a3cb to 9c21e5c Compare September 18, 2023 19:29

Vexu reviewed Nov 13, 2023

View reviewed changes

lib/std/Progress.zig Outdated Show resolved Hide resolved

lib/std/Progress.zig Outdated Show resolved Hide resolved

lib/std/Progress.zig Outdated Show resolved Hide resolved

lib/std/Progress.zig Outdated Show resolved Hide resolved

DISTREAT force-pushed the master branch from c3ea419 to b3df6fe Compare November 17, 2023 15:06

DISTREAT force-pushed the master branch 2 times, most recently from fab9e4c to 72e0a34 Compare November 26, 2023 14:53

DISTREAT and others added 12 commits November 26, 2023 16:08

std.Progress: display nodes in a tree form

4ed2d63

std.Progress: truncate content to terminal width

1ec1bf3

std.Progress: make the output_buffer end index a field

ae732ba

This is useful for a lower-level interaction with the module. An example would be the post-processing of the buffer before writing to `terminal`.

frontend: migrate to the new API of std.Progress

215c40d

std.Progress: always write the output_buffer

d421c78

Following the commit that made the output_buffer end index become its own field, it's also reasonable to write to the output_buffer even if the terminal field is null.

docgen: add support for more ansi escape sequences

64138fb

std.Progress: use fill option to replace indentation_buffer

4d0574b

Co-authored-by: Veikka Tuominen <[email protected]>

std.Progress: remove confusing comment

9f9c17b

Co-authored-by: Veikka Tuominen <[email protected]>

std.Progress: remove confusing comment

10f04b6

Co-authored-by: Veikka Tuominen <[email protected]>

std.Progress: fix trailing bracket

b8bbf9b

std.Progress: redundant mutability

1ba94ea

std.Progress: add truncation support for macos

72cc178

DISTREAT force-pushed the master branch from 72e0a34 to 72cc178 Compare November 26, 2023 15:09

DISTREAT commented Mar 4, 2024

View reviewed changes

lib/std/Progress.zig Outdated Show resolved Hide resolved

DISTREAT added 4 commits April 21, 2024 20:32

Merge zig master and resolve conflicts

49a521d

std.Progress: update casing of AtomicOrder field identifiers

20cb751

std.Progress: do not copy ellipse if not memory-safe

e5209b9

std.Progress: continously update max_columns

aaa13b0

Merge zig master and resolve docgen conflicts

4a743c6

This merge resolves conflicts caused by the removal of `termColor` in `tools/docgen.zig`.

Vexu approved these changes Apr 28, 2024

View reviewed changes

DISTREAT added 13 commits April 29, 2024 17:43

std.Progress: output_buffer as two-dimensional

aa1c0cc

This refactor should improve readability by utilizing a nested buffer instead of a one-dimensional buffer. The upside is better maintainability. Before this commit, the idea was to keep track of LFs in a single buffer.

std.Progress: add method for deactivating node

623b068

std.Progress: prevent unnecessary iterations

7fcfe79

doctest: add support for more ansi escape sequences

0cbc601

std.Progress: missing check for max_columns is 0

acb5067

std.Progress: double output_buffer row limit

f4258a2

frontend: migrate to new backend of std.Progress

d29ca27

The internal structure of `std.Progress` changed and due to the low-level approach of `main.zig` this leads to issues. This commit makes use of methods provided by `std.Progress` and `serveStringMessage` to resolve these issues.

std.Progress: skip newline on node without info

b5510ce

This commit addresses the issue of nodes without name or eta causing a newline. The resulting output is unexpected.

std.Progress: option to emulate old progress bar

7bc2e15

This commit serves as a preparation for restoring the original CI compilation output since the frontend is not making use of the new behavior yet.

Merge remote-tracking branch 'upstream/master'

433968f

std.Progress: ansi buffer write to end

6b037fc

frontend: enable one-line progress bar everywhere

59e3c2d

Emulate the old progress bar everywhere to provide a decently seamless migration to the new progress bar. In addition, this commit makes it easier to manually initiate a progress bar without using the `start` method.

std.Progress: ignore newline if buffer is full

13c3f69

DISTREAT added 2 commits May 1, 2024 16:41

std.Progress: option to disregard terminal width

41f3adc

std.Progress: do not assume windows is always tty

3cb4e80

This patch removes assert statements in windows that were causing issues due to the assumption that every `terminal` is a TTY.

andrewrk closed this May 25, 2024

Uh oh!

std.Progress: tree form and line truncation #17165

std.Progress: tree form and line truncation #17165

Uh oh!

Conversation

DISTREAT commented Sep 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vexu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DISTREAT commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vexu left a comment

Choose a reason for hiding this comment

Uh oh!

DISTREAT commented Apr 28, 2024

Uh oh!

DISTREAT commented Apr 30, 2024

Uh oh!

andrewrk commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DISTREAT commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DISTREAT commented May 1, 2024

Uh oh!

Vexu commented May 1, 2024

Uh oh!

DISTREAT commented May 1, 2024

Uh oh!

DISTREAT commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewrk commented May 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DISTREAT commented Sep 15, 2023 •

edited

Loading

DISTREAT commented Apr 22, 2024 •

edited

Loading

andrewrk commented May 1, 2024 •

edited

Loading

DISTREAT commented May 1, 2024 •

edited

Loading

DISTREAT commented May 1, 2024 •

edited

Loading