Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.Progress: instead of one line of terminal progress, make it check the terminal width and use multiple lines #14946

Closed
Tracked by #14647
andrewrk opened this issue Mar 16, 2023 · 15 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@andrewrk
Copy link
Member

Extracted from #14647.

When multiple things are being worked on, and the executable has decided to use terminal emulator codes to display a progress bar, display the multiple things in a tree form rather than as a single line. This will give a better idea of what is happening when multiple threads are doing multiple things at once.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels Mar 16, 2023
@andrewrk andrewrk added this to the 0.12.0 milestone Mar 16, 2023
@silversquirl
Copy link
Contributor

I'd also like to add that this should fix an annoying issue where std.Progress will wrap on small terminals and not properly replace its previous progress message, leading to a lot of output spam.

@DISTREAT
Copy link

I've been able to make the changes needed to print multiple lines and I am confident that I'll also manage to implement the other features.

I must add, though, that I have basically removed columns_written, making it redundant through the use of more specific ANSI escape sequences. I don't think that is an issue, because ANSI lets you skip to the beginning of the line with ease. Nonetheless, I'll need to find a solution for Windows support and I am not proficient, working with Windows, but I am sure I'll manage.

@andrewrk
Copy link
Member Author

Sounds promising!

Tip: You can use https://asciinema.org/ to share demos of your code in action.

@DISTREAT
Copy link

I couldn't manage to properly record using asciinema, so here a video instead:

demo.webm

Although I think the code isn't the most elegant (might just be my craving for perfection tho), I think I've managed to find an acceptable solution that isn't performance optimized. If I take some more time playing around then I might as well find something more optimized. Windows support is still to be adapted and I need to test for potential issues, as well as find a way to word-wrap on smaller terminal windows. Furthermore, I also need to make sure that multiple nodes can be displayed at a time while also having children nodes. At the moment, I have only tested for a simple cascade of nodes.

To help me get a better idea of the 'vision', feedback is always appreciated.

@DISTREAT
Copy link

I've also changed the spacing to make it look a bit more compact:

Building... [901/1000]
- Task 1 [2/1]
  - Task 2 [1/1]

@DISTREAT
Copy link

DISTREAT commented Sep 13, 2023

Here another demo:

demo2.webm

I've finally managed to implement a tree representation and all that is left to do is add Windows support (fingers crossed), do some testing (crossed even tighter), major cleanup (code got messy), and create another commit for the line-wrapping.

It was harder than I initially thought, since the original approach was just having the active Node point to parent which does not exactly present any non-active Nodes. In addition, and the way it's often done, working with trees, I had to move some code into a recursive function. Still, zero dynamic allocation and hopefully Thread-safety, since I lack experience with that, apart from implementing a queue and thread blocking in a python project.

I should also mention that the screen is not being cleared so no previous lines are overwritten.

@andrewrk
Copy link
Member Author

Looks pretty good! Can you show a demo of running 2 threads that simultaneously increment different tasks from 0 to 1000?

@DISTREAT
Copy link

Looks pretty good! Can you show a demo of running 2 threads that simultaneously increment different tasks from 0 to 1000?

For sure :)

demo3.webm
const std = @import("std");
const Thread = std.Thread;

fn run(node: *std.Progress.Node, arg: []const u8) void {
    var child = node.start(arg, 1000);
    defer child.end();

    for (0..1000) |i| {
        child.activate();
        child.setCompletedItems(i);
        std.time.sleep(5000000);
    }
}

pub fn main() !void {
    var progress = std.Progress{};

    var node = progress.start("Building", 1);
    defer node.end();

    const handler_1 = try Thread.spawn(.{}, run, .{ node, "Task 1" });
    const handler_2 = try Thread.spawn(.{}, run, .{ node, "Task 2" });

    handler_1.join();
    // node.completeOne();
    handler_2.join();
    // node.completeOne();
}

@andrewrk
Copy link
Member Author

Looks great!

@DISTREAT
Copy link

DISTREAT commented Sep 14, 2023

I am currently in the testing stages, is there anything in specific I should test against?.

The line-wrapping will come as a separate commit, after which I'll do the PR for revision.

@andrewrk
Copy link
Member Author

It needs to work on Windows too (although it is not required to have identical behavior)

@DISTREAT
Copy link

DISTREAT commented Sep 14, 2023

It needs to work on Windows too (although it is not required to have identical behavior)

I can confirm it works on Windows as well. Anything else?

@andrewrk
Copy link
Member Author

I think the next step would be to review a pull request

@DISTREAT
Copy link

Alright then, I'll try to implement the line-wrapping and open a PR for revision afterwards.

@andrewrk
Copy link
Member Author

andrewrk commented Jun 1, 2024

Done in #20059

@andrewrk andrewrk closed this as completed Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants