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

Progress bar not clear when changing style #669

Open
DBY047 opened this issue Nov 22, 2024 · 12 comments
Open

Progress bar not clear when changing style #669

DBY047 opened this issue Nov 22, 2024 · 12 comments

Comments

@DBY047
Copy link

DBY047 commented Nov 22, 2024

I built a program to download artifacts from devOps. It uses MultiProgress with tokio to download artifacts in parallels. The program changes the prgress bar style dynamically while the the artifacts are process like progress you would see when you pulled docker images.

I use a default style (ProgressStyle::with_template("{prefix}: {msg}")) when download is not started yet or completed
and another when download is in progress (ProgressStyle::with_template("{prefix}: Downloading [{wide_bar:.white/red}] {percent}% [{bytes}/{total_bytes}]")).

With version 0.17.8, when swaping the progress style, the line in the terminal are all cleaned from previous inforrmations. The bar progress and the percent/bytes downloaded is not visiable anymore. See the result while download is completed and the default progress is used:
image

With version 0.17.9, when swaping the progress from the download style to the default style, the bar, the percent and the bytes information or still shown in the console on all progress bar except the last one 🤔. See the result while download is completed and the default progress is use:
image

I found out that using set_move_cursor(true) with the MultiProgress causes this issue with version 0.17.9. Without this setting, progress bar flicker way more specially on Windows. But I guess it is better than seeing data from the bars previous style.

The new version with set_move_cursor(true) seams to fix the flicker issue. I don't see any flickering at all. That whould be nice to have no flicker and the lines clreared like in the previous version. For the moment I will sitck with the previous version.

Thanks!

@djc
Copy link
Member

djc commented Nov 25, 2024

Hey, sorry for the regression. Unfortunately I probably won't have time to look into this soon -- though it might help if you can do some more debugging on what code changes changed behavior for your use case for the worse. I don't think there were all that many changes between 0.17.8 and 0.17.9...

@spoutn1k
Copy link
Contributor

Hi ! Could you try the branch from #671 to see if it fixes your issue ?

@DBY047
Copy link
Author

DBY047 commented Nov 27, 2024

Of course. I let you know once I have tested it. Thanks for the quick feedback!

@DBY047
Copy link
Author

DBY047 commented Nov 27, 2024

@spoutn1k, I tried branch fix-double-prints from https://github.com/spoutn1k/indicatif.git and it did not fix the issue.

indicatif = { version = "~0.17", features = ["tokio"], git = "https://github.com/spoutn1k/indicatif.git", branch = "fix-double-prints"}

@spoutn1k
Copy link
Contributor

Ok good news is, it keeps the project count broken by me down. Bad news is, I was not reponsible so I have no idea what is up for you. I'm fixing my mess in #671 and then I will take a look.

@djc
Copy link
Member

djc commented Nov 29, 2024

Okay, I suggest you try to use git bisect to identify the offending commit.

@spoutn1k
Copy link
Contributor

@DBY047 Would it be possible to share a minimal version of the code presenting this issue so that I could implement a test for it ?

@DBY047
Copy link
Author

DBY047 commented Nov 29, 2024

@spoutn1k, I reproduced it with the multi.rs example.

Without the set_move_cursor set to true
image

With the the set_move_cursor set to true
image

use std::thread;
use std::time::Duration;

use indicatif::{MultiProgress, ProgressBar, ProgressStyle};

fn main() {
    let m = MultiProgress::new();
    let default_style =  ProgressStyle::with_template("{msg}").unwrap();
    let sty = ProgressStyle::with_template(
        "[{elapsed_precise}] {bar:40.cyan/blue} {pos:>7}/{len:7} {msg}",
    )
    .unwrap()
    .progress_chars("##-");

    let n = 200;
    let pb = m.add(ProgressBar::new(n));
    pb.set_style(sty.clone());
    pb.set_message("todo");
    let pb2 = m.add(ProgressBar::new(n));
    pb2.set_style(sty.clone());
    pb2.set_message("finished");

    let pb3 = m.insert_after(&pb2, ProgressBar::new(1024));
    pb3.set_style(sty);

    m.set_move_cursor(true);

    let mut threads = vec![];
    let default_style_clone = default_style.clone();
    let h3 = thread::spawn(move || {
        for i in 0..1024 {
            thread::sleep(Duration::from_millis(2));
            pb3.set_message(format!("item #{}", i + 1));
            pb3.inc(1);
        }

        pb3.set_style(default_style_clone);
        pb3.finish_with_message("done");
    });

    for i in 0..n {
        thread::sleep(Duration::from_millis(15));
        if i == n / 3 {
            thread::sleep(Duration::from_secs(2));
        }
        pb.inc(1);
        let pb2 = pb2.clone();
        threads.push(thread::spawn(move || {
            pb2.inc(1);
        }));
    }
    pb.set_style(default_style.clone());
    pb.finish_with_message("all jobs started");

    for thread in threads {
        let _ = thread.join();
    }
    let _ = h3.join();

    pb2.set_style(default_style.clone());
    pb2.finish_with_message("all jobs done");

}

@spoutn1k
Copy link
Contributor

Amazing, thank you for the quick check !

@DBY047
Copy link
Author

DBY047 commented Nov 29, 2024

@djc here the git bisect result

648226bee3f76d4db26cf8f1682722a171f31939 is the first bad commit
commit 648226bee3f76d4db26cf8f1682722a171f31939
Author: Benjamin Neff <benjamin@coding4coffee.ch>
Date:   Mon Jun 10 14:05:38 2024 +0200

    Forward move_cursor flag to draw state
    
    The move_cursor flag needs to be set in the wrapped Term draw state, as
    that's the one, that is then actually used to draw to the terminal. The
    move_cursor flag in the multi draw state doesn't have any effect on
    drawing to the terminal.

 src/draw_target.rs |  9 +++++++++
 src/multi.rs       | 14 ++++++--------
 tests/render.rs    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 8 deletions(-)

@djc
Copy link
Member

djc commented Dec 12, 2024

(@chris-laplante if you're really bored, maybe have a look at this one?)

@chris-laplante
Copy link
Collaborator

(@chris-laplante if you're really bored, maybe have a look at this one?)

I should™ be able to this weekend or over holiday break maybe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants