console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.41k stars 241 forks source link

Adding/removing progress bar spinners leaves newlines #645

Open msrd0 opened 5 months ago

msrd0 commented 5 months ago

When displaying a progress bar at the bottom and spinner(s) above, the spinners leave newlines when they get cleared:

rec

This is the source code for this test:

use std::{thread, time::Duration};
use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget};

fn main() {
    let pbs = MultiProgress::with_draw_target(ProgressDrawTarget::stderr());
    let pb = ProgressBar::new(20);
    pbs.add(pb.clone());

    for i in 0..20 {
        let msg = format!("running {i}");

        let run_pb = ProgressBar::new_spinner();
        run_pb.enable_steady_tick(Duration::from_millis(100));
        run_pb.set_message(msg);
        pbs.insert_from_back(1, run_pb.clone());

        thread::sleep(Duration::from_secs(1));

        run_pb.finish_and_clear();
        pb.inc(1);
    }

    pb.finish();
}
chris-laplante commented 5 months ago

The issue is that you called some methods on the ProgressBar which caused draws, before adding them to the MultiProgress. MultiProgress assumes ProgressBars that have been added to it have not been drawn yet. (Perhaps we can enforce this in code...)

let run_pb = ProgressBar::new_spinner();
run_pb.enable_steady_tick(Duration::from_millis(100));
run_pb.set_message(msg);   // <- draw happened
pbs.insert_from_back(1, run_pb.clone());  // <- add to MultiProgress, but it's too late...

If you re-order those lines, everything works:

let run_pb = ProgressBar::new_spinner();
pbs.insert_from_back(1, run_pb.clone());
run_pb.enable_steady_tick(Duration::from_millis(100));
run_pb.set_message(msg);

Also, you can make the code simpler by avoiding the clones:

use std::{thread, time::Duration};
use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget};

fn main() {
    let pbs = MultiProgress::with_draw_target(ProgressDrawTarget::stderr());
    let pb = pbs.add(ProgressBar::new(20));

    for i in 0..20 {
        let msg = format!("running {i}");

        let run_pb = pbs.insert_from_back(1, ProgressBar::new_spinner());
        run_pb.enable_steady_tick(Duration::from_millis(100));
        run_pb.set_message(msg);

        thread::sleep(Duration::from_secs(1));

        run_pb.finish_and_clear();
        pb.inc(1);
    }

    pb.finish();
}

It also removes the temptation to call methods before adding them to the MultiProgress :).

msrd0 commented 5 months ago

Thanks, that indeed fixed my problem :)

Maybe you could at least store a flag (in debug mode) for progress bars whether they have been drawn already, and print a warning when you add one to a multi progress that had already been drawn?

chris-laplante commented 5 months ago

Thanks, that indeed fixed my problem :)

Maybe you could at least store a flag (in debug mode) for progress bars whether they have been drawn already, and print a warning when you add one to a multi progress that had already been drawn?

I agree, that would be helpful. I will submit an issue for myself to consider adding this feature.