diff --git a/examples/finebars.rs b/examples/finebars.rs index e570a195..d94a0208 100644 --- a/examples/finebars.rs +++ b/examples/finebars.rs @@ -1,7 +1,7 @@ use std::thread; use std::time::Duration; -use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBarBuilder, ProgressStyle}; use rand::RngExt; fn main() { @@ -18,13 +18,18 @@ fn main() { let handles: Vec<_> = styles .iter() .map(|s| { - let pb = m.add(ProgressBar::new(512)); - pb.set_style( - ProgressStyle::with_template(&format!("{{prefix:.bold}}▕{{bar:.{}}}▏{{msg}}", s.2)) - .unwrap() - .progress_chars(s.1), + let pb = m.register( + ProgressBarBuilder::new(512) + .with_style( + ProgressStyle::with_template(&format!( + "{{prefix:.bold}}▕{{bar:.{}}}▏{{msg}}", + s.2 + )) + .unwrap() + .progress_chars(s.1), + ) + .with_prefix(s.0), ); - pb.set_prefix(s.0); let wait = Duration::from_millis(rand::rng().random_range(10..30)); thread::spawn(move || { for i in 0..512 { diff --git a/examples/morebars.rs b/examples/morebars.rs index 30b664f0..c5f1f06a 100644 --- a/examples/morebars.rs +++ b/examples/morebars.rs @@ -2,21 +2,19 @@ use std::sync::Arc; use std::thread; use std::time::Duration; -use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBarBuilder, ProgressStyle}; fn main() { let m = Arc::new(MultiProgress::new()); let sty = ProgressStyle::with_template("{bar:40.green/yellow} {pos:>7}/{len:7}").unwrap(); - let pb = m.add(ProgressBar::new(5)); - pb.set_style(sty.clone()); + let pb = m.register(ProgressBarBuilder::new(5).with_style(sty.clone())); // make sure we show up at all. otherwise no rendering // event. pb.tick(); for _ in 0..5 { - let pb2 = m.add(ProgressBar::new(128)); - pb2.set_style(sty.clone()); + let pb2 = m.register(ProgressBarBuilder::new(128).with_style(sty.clone())); for _ in 0..128 { thread::sleep(Duration::from_millis(5)); pb2.inc(1); diff --git a/examples/multi-tree-ext.rs b/examples/multi-tree-ext.rs index 9187b9ca..00f8d8d8 100644 --- a/examples/multi-tree-ext.rs +++ b/examples/multi-tree-ext.rs @@ -6,7 +6,9 @@ use std::thread; use std::time::Duration; use console::style; -use indicatif::{MultiProgress, MultiProgressAlignment, ProgressBar, ProgressStyle}; +use indicatif::{ + MultiProgress, MultiProgressAlignment, ProgressBar, ProgressBarBuilder, ProgressStyle, +}; use once_cell::sync::Lazy; use rand::rngs::ThreadRng; use rand::{Rng, RngExt}; @@ -29,7 +31,6 @@ struct Item { key: String, index: usize, indent: usize, - progress_bar: ProgressBar, } #[derive(Clone, Debug)] @@ -43,55 +44,46 @@ static ELEMENTS: Lazy<[Elem; 27]> = Lazy::new(|| { Elem::AddItem(Item { indent: 9, index: 0, - progress_bar: ProgressBar::new(PB_LEN), key: "dog".to_string(), }), Elem::AddItem(Item { indent: 0, index: 0, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_1".to_string(), }), Elem::AddItem(Item { indent: 8, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "lazy".to_string(), }), Elem::AddItem(Item { indent: 0, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_2".to_string(), }), Elem::AddItem(Item { indent: 1, index: 0, - progress_bar: ProgressBar::new(PB_LEN), key: "the".to_string(), }), Elem::AddItem(Item { indent: 0, index: 0, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_3".to_string(), }), Elem::AddItem(Item { indent: 7, index: 3, - progress_bar: ProgressBar::new(PB_LEN), key: "a".to_string(), }), Elem::AddItem(Item { indent: 0, index: 3, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_4".to_string(), }), Elem::AddItem(Item { indent: 6, index: 2, - progress_bar: ProgressBar::new(PB_LEN), key: "over".to_string(), }), Elem::RemoveItem(Index(6)), @@ -101,55 +93,46 @@ static ELEMENTS: Lazy<[Elem; 27]> = Lazy::new(|| { Elem::AddItem(Item { indent: 0, index: 2, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_5".to_string(), }), Elem::AddItem(Item { indent: 4, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "fox".to_string(), }), Elem::AddItem(Item { indent: 0, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_6".to_string(), }), Elem::AddItem(Item { indent: 2, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "quick".to_string(), }), Elem::AddItem(Item { indent: 0, index: 1, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_7".to_string(), }), Elem::AddItem(Item { indent: 5, index: 5, - progress_bar: ProgressBar::new(PB_LEN), key: "jumps".to_string(), }), Elem::AddItem(Item { indent: 0, index: 5, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_8".to_string(), }), Elem::AddItem(Item { indent: 3, index: 4, - progress_bar: ProgressBar::new(PB_LEN), key: "brown".to_string(), }), Elem::AddItem(Item { indent: 0, index: 3, - progress_bar: ProgressBar::new(PB_LEN), key: "temp_9".to_string(), }), Elem::RemoveItem(Index(10)), @@ -190,25 +173,20 @@ pub fn main() { ProgressStyle::with_template("[{pos:>2}/{len:2}] {prefix}{spinner:.green} {msg}").unwrap(); let sty_fin = ProgressStyle::with_template("[{pos:>2}/{len:2}] {prefix}{msg}").unwrap(); - let pb_main = mp.add(ProgressBar::new( - ELEMENTS - .iter() - .map(|e| match e { - Elem::AddItem(item) => item.progress_bar.length().unwrap(), - Elem::RemoveItem(_) => 1, - }) - .sum(), - )); + let pb_main = mp.register( + ProgressBarBuilder::new( + ELEMENTS + .iter() + .map(|e| match e { + Elem::AddItem(_) => PB_LEN, + Elem::RemoveItem(_) => 1, + }) + .sum(), + ) + .with_style(sty_main), + ); - pb_main.set_style(sty_main); - for e in ELEMENTS.iter() { - match e { - Elem::AddItem(item) => item.progress_bar.set_style(sty_aux.clone()), - Elem::RemoveItem(_) => {} - } - } - - let mut items: Vec<&Item> = Vec::with_capacity(ELEMENTS.len()); + let mut items: Vec<(&Item, ProgressBar)> = Vec::with_capacity(ELEMENTS.len()); let mp2 = Arc::clone(&mp); let mut rng = ThreadRng::default(); @@ -222,29 +200,28 @@ pub fn main() { } Action::ModifyTree(elem_idx) => match &ELEMENTS[elem_idx] { Elem::AddItem(item) => { - let pb = mp2.insert(item.index, item.progress_bar.clone()); - pb.set_prefix(" ".repeat(item.indent)); - pb.set_message(&item.key); - items.insert(item.index, item); + let pb = mp2.register_at( + item.index, + ProgressBarBuilder::new(PB_LEN) + .with_style(sty_aux.clone()) + .with_prefix(" ".repeat(item.indent)) + .with_message(item.key.clone()), + ); + items.insert(item.index, (item, pb)); } Elem::RemoveItem(Index(index)) => { - let item = items.remove(*index); - let pb = &item.progress_bar; - mp2.remove(pb); + let (_, pb) = items.remove(*index); + mp2.remove(&pb); pb_main.inc(pb.length().unwrap() - pb.position()); } }, Action::IncProgressBar(item_idx) => { - let item = &items[item_idx]; - item.progress_bar.inc(1); - let pos = item.progress_bar.position(); - if pos >= item.progress_bar.length().unwrap() { - item.progress_bar.set_style(sty_fin.clone()); - item.progress_bar.finish_with_message(format!( - "{} {}", - style("✔").green(), - item.key - )); + let (item, pb) = &items[item_idx]; + pb.inc(1); + let pos = pb.position(); + if pos >= pb.length().unwrap() { + pb.set_style(sty_fin.clone()); + pb.finish_with_message(format!("{} {}", style("✔").green(), item.key)); } pb_main.inc(1); } @@ -254,15 +231,15 @@ pub fn main() { } /// The function guarantees to return the action, that is valid for the current tree. -fn get_action(rng: &mut dyn Rng, items: &[&Item]) -> Action { +fn get_action(rng: &mut dyn Rng, items: &[(&Item, ProgressBar)]) -> Action { let elem_idx = ELEM_IDX.load(Ordering::SeqCst); // the indices of those items, that not completed yet let uncompleted = items .iter() .enumerate() - .filter(|(_, item)| { - let pos = item.progress_bar.position(); - pos < item.progress_bar.length().unwrap() + .filter(|(_, (_, pb))| { + let pos = pb.position(); + pos < pb.length().unwrap() }) .map(|(idx, _)| idx) .collect::>(); diff --git a/examples/multi-tree.rs b/examples/multi-tree.rs index 96a306d6..a6ec0078 100644 --- a/examples/multi-tree.rs +++ b/examples/multi-tree.rs @@ -3,7 +3,7 @@ use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; -use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBar, ProgressBarBuilder, ProgressStyle}; use once_cell::sync::Lazy; use rand::rngs::ThreadRng; use rand::{Rng, RngExt}; @@ -19,7 +19,7 @@ struct Elem { key: String, index: usize, indent: usize, - progress_bar: ProgressBar, + len: u64, } static ELEMENTS: Lazy<[Elem; 9]> = Lazy::new(|| { @@ -27,55 +27,55 @@ static ELEMENTS: Lazy<[Elem; 9]> = Lazy::new(|| { Elem { indent: 1, index: 0, - progress_bar: ProgressBar::new(32), + len: 32, key: "jumps".to_string(), }, Elem { indent: 2, index: 1, - progress_bar: ProgressBar::new(32), + len: 32, key: "lazy".to_string(), }, Elem { indent: 0, index: 0, - progress_bar: ProgressBar::new(32), + len: 32, key: "the".to_string(), }, Elem { indent: 3, index: 3, - progress_bar: ProgressBar::new(32), + len: 32, key: "dog".to_string(), }, Elem { indent: 2, index: 2, - progress_bar: ProgressBar::new(32), + len: 32, key: "over".to_string(), }, Elem { indent: 2, index: 1, - progress_bar: ProgressBar::new(32), + len: 32, key: "brown".to_string(), }, Elem { indent: 1, index: 1, - progress_bar: ProgressBar::new(32), + len: 32, key: "quick".to_string(), }, Elem { indent: 3, index: 5, - progress_bar: ProgressBar::new(32), + len: 32, key: "a".to_string(), }, Elem { indent: 3, index: 3, - progress_bar: ProgressBar::new(32), + len: 32, key: "fox".to_string(), }, ] @@ -91,18 +91,12 @@ fn main() { let sty_main = ProgressStyle::with_template("{bar:40.green/yellow} {pos:>4}/{len:4}").unwrap(); let sty_aux = ProgressStyle::with_template("{spinner:.green} {msg} {pos:>4}/{len:4}").unwrap(); - let pb_main = mp.add(ProgressBar::new( - ELEMENTS - .iter() - .map(|e| e.progress_bar.length().unwrap()) - .sum(), - )); - pb_main.set_style(sty_main); - for elem in ELEMENTS.iter() { - elem.progress_bar.set_style(sty_aux.clone()); - } + let pb_main = mp.register( + ProgressBarBuilder::new(ELEMENTS.iter().map(|e| e.len).sum()).with_style(sty_main), + ); - let tree: Arc>> = Arc::new(Mutex::new(Vec::with_capacity(ELEMENTS.len()))); + let tree: Arc>> = + Arc::new(Mutex::new(Vec::with_capacity(ELEMENTS.len()))); let tree2 = Arc::clone(&tree); let mp2 = Arc::clone(&mp); @@ -119,16 +113,21 @@ fn main() { } Some(Action::AddProgressBar(el_idx)) => { let elem = &ELEMENTS[el_idx]; - let pb = mp2.insert(elem.index + 1, elem.progress_bar.clone()); - pb.set_message(format!("{} {}", " ".repeat(elem.indent), elem.key)); - tree.lock().unwrap().insert(elem.index, elem); + let pb = mp2.register_at( + elem.index + 1, + ProgressBarBuilder::new(elem.len) + .with_style(sty_aux.clone()) + .with_message(format!("{} {}", " ".repeat(elem.indent), elem.key)), + ); + tree.lock().unwrap().insert(elem.index, (elem, pb)); } Some(Action::IncProgressBar(el_idx)) => { - let elem = &tree.lock().unwrap()[el_idx]; - elem.progress_bar.inc(1); - let pos = elem.progress_bar.position(); - if pos >= elem.progress_bar.length().unwrap() { - elem.progress_bar.finish_with_message(format!( + let tree = tree.lock().unwrap(); + let (elem, pb) = &tree[el_idx]; + pb.inc(1); + let pos = pb.position(); + if pos >= pb.length().unwrap() { + pb.finish_with_message(format!( "{}{} {}", " ".repeat(elem.indent), "✔", @@ -144,22 +143,22 @@ fn main() { println!("==============================="); println!("the tree should be the same as:"); - for elem in tree2.lock().unwrap().iter() { + for (elem, _) in tree2.lock().unwrap().iter() { println!("{} {}", " ".repeat(elem.indent), elem.key); } } /// The function guarantees to return the action, that is valid for the current tree. -fn get_action(rng: &mut dyn Rng, tree: &Mutex>) -> Option { +fn get_action(rng: &mut dyn Rng, tree: &Mutex>) -> Option { let elem_len = ELEMENTS.len() as u64; let list_len = tree.lock().unwrap().len() as u64; let sum_free = tree .lock() .unwrap() .iter() - .map(|e| { - let pos = e.progress_bar.position(); - let len = e.progress_bar.length().unwrap(); + .map(|(_, pb)| { + let pos = pb.position(); + let len = pb.length().unwrap(); len - pos }) .sum::(); @@ -178,8 +177,9 @@ fn get_action(rng: &mut dyn Rng, tree: &Mutex>) -> Option { return Some(Action::AddProgressBar(list.len())); } else { let l = (k % list_len) as usize; - let pos = list[l].progress_bar.position(); - let len = list[l].progress_bar.length(); + let (_, pb) = &list[l]; + let pos = pb.position(); + let len = pb.length(); if pos < len.unwrap() { return Some(Action::IncProgressBar(l)); } diff --git a/examples/multi.rs b/examples/multi.rs index 06c5a4d0..6e9d248c 100644 --- a/examples/multi.rs +++ b/examples/multi.rs @@ -1,7 +1,7 @@ use std::thread; use std::time::Duration; -use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{MultiProgress, ProgressBarBuilder, ProgressStyle}; use rand::RngExt; @@ -14,15 +14,17 @@ fn main() { .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); + let pb = m.register( + ProgressBarBuilder::new(n) + .with_style(sty.clone()) + .with_message("todo"), + ); + let pb2 = m.register( + ProgressBarBuilder::new(n) + .with_style(sty.clone()) + .with_message("finished"), + ); + let pb3 = m.register_after(&pb2, ProgressBarBuilder::new(1024).with_style(sty)); m.println("starting!").unwrap(); diff --git a/examples/yarnish.rs b/examples/yarnish.rs index f0afc3b0..9e4a6c25 100644 --- a/examples/yarnish.rs +++ b/examples/yarnish.rs @@ -2,7 +2,7 @@ use std::thread; use std::time::{Duration, Instant}; use console::{style, Emoji}; -use indicatif::{HumanDuration, MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{HumanDuration, MultiProgress, ProgressBar, ProgressBarBuilder, ProgressStyle}; use rand::prelude::IndexedRandom; use rand::RngExt; @@ -72,9 +72,11 @@ pub fn main() { let handles: Vec<_> = (0..4u32) .map(|i| { let count = rng.random_range(30..80); - let pb = m.add(ProgressBar::new(count)); - pb.set_style(spinner_style.clone()); - pb.set_prefix(format!("[{}/?]", i + 1)); + let pb = m.register( + ProgressBarBuilder::new(count) + .with_style(spinner_style.clone()) + .with_prefix(format!("[{}/?]", i + 1)), + ); thread::spawn(move || { let mut rng = rand::rng(); let pkg = PACKAGES.choose(&mut rng).unwrap(); diff --git a/src/draw_target.rs b/src/draw_target.rs index 8fbdd812..f186d8d4 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -698,13 +698,13 @@ impl PartialEq for LineType { #[cfg(test)] mod tests { use crate::draw_target::LineType; - use crate::{MultiProgress, ProgressBar, ProgressDrawTarget}; + use crate::{MultiProgress, ProgressBarBuilder, ProgressDrawTarget}; #[test] fn multi_is_hidden() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); - let pb = mp.add(ProgressBar::new(100)); + let pb = mp.register(ProgressBarBuilder::new(100)); assert!(mp.is_hidden()); assert!(pb.is_hidden()); } diff --git a/src/lib.rs b/src/lib.rs index 0378f091..a2742f1a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,6 +17,7 @@ //! //! * **Progress bars** //! * [`ProgressBar`](struct.ProgressBar.html) for bars and spinners +//! * [`ProgressBarBuilder`](struct.ProgressBarBuilder.html) for deferred configuration with [`MultiProgress`] //! * [`MultiProgress`](struct.MultiProgress.html) for multiple bars //! * **Data Formatting** //! * [`HumanBytes`](struct.HumanBytes.html) for formatting bytes @@ -35,6 +36,13 @@ //! //! Additionally a [`MultiProgress`] utility is provided that can manage //! rendering multiple progress bars at once (eg: from multiple threads). +//! When adding bars to a [`MultiProgress`], use [`ProgressBarBuilder`] with +//! [`MultiProgress::register`] — it captures settings without triggering +//! premature draws. +//! +//! Passing a [`ProgressBar`] directly to [`MultiProgress::add`] is deprecated +//! and will be removed in a future release. New code should use +//! [`ProgressBarBuilder`] with [`MultiProgress::register`]. //! //! To whet your appetite, this is what this can look like: //! @@ -254,6 +262,7 @@ mod in_memory; mod iter; mod multi; mod progress_bar; +mod progress_bar_builder; #[cfg(feature = "rayon")] mod rayon; mod state; @@ -270,6 +279,7 @@ pub use crate::in_memory::InMemoryTerm; pub use crate::iter::{ProgressBarIter, ProgressIterator}; pub use crate::multi::{MultiProgress, MultiProgressAlignment}; pub use crate::progress_bar::{ProgressBar, WeakProgressBar}; +pub use crate::progress_bar_builder::ProgressBarBuilder; #[cfg(feature = "rayon")] pub use crate::rayon::ParallelProgressIterator; pub use crate::state::{ProgressFinish, ProgressState}; @@ -292,4 +302,5 @@ mod tests { impl MustBeThreadSafe for ProgressState {} impl MustBeThreadSafe for ProgressStyle {} impl MustBeThreadSafe for WeakProgressBar {} + impl MustBeThreadSafe for ProgressBarBuilder {} } diff --git a/src/multi.rs b/src/multi.rs index 195c9f9d..de0fa77f 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -10,6 +10,7 @@ use crate::draw_target::{ VisualLines, }; use crate::progress_bar::ProgressBar; +use crate::progress_bar_builder::ProgressBarBuilder; #[cfg(all(target_arch = "wasm32", feature = "wasmbind"))] use web_time::Instant; @@ -71,6 +72,51 @@ impl MultiProgress { self.state.write().unwrap().alignment = alignment; } + /// Registers a [`ProgressBarBuilder`] with this [`MultiProgress`]. + /// + /// The builder is materialized into a [`ProgressBar`] whose draw target is + /// a remote target intercepted by this [`MultiProgress`]. The resulting bar + /// is positioned below all other bars currently in the [`MultiProgress`]. + /// + /// Using a [`ProgressBarBuilder`] avoids the footgun of configuring a + /// [`ProgressBar`] (which triggers draws to stderr) before adding it to a + /// [`MultiProgress`]. See [#677] for details. + /// + /// [#677]: https://github.com/console-rs/indicatif/issues/677 + pub fn register(&self, builder: ProgressBarBuilder) -> ProgressBar { + self.internalize_builder(InsertLocation::End, builder) + } + + /// Registers a [`ProgressBarBuilder`] at the given index. + /// + /// If `index` is greater than or equal to the number of currently tracked + /// progress bars, the bar is added to the end of the list. + pub fn register_at(&self, index: usize, builder: ProgressBarBuilder) -> ProgressBar { + self.internalize_builder(InsertLocation::Index(index), builder) + } + + /// Registers a [`ProgressBarBuilder`] at the given index, counting from the back. + /// + /// If `index` is greater than or equal to the number of currently tracked + /// progress bars, the bar is added to the start of the list. + pub fn register_from_back(&self, index: usize, builder: ProgressBarBuilder) -> ProgressBar { + self.internalize_builder(InsertLocation::IndexFromBack(index), builder) + } + + /// Registers a [`ProgressBarBuilder`] before an existing progress bar. + pub fn register_before( + &self, + before: &ProgressBar, + builder: ProgressBarBuilder, + ) -> ProgressBar { + self.internalize_builder(InsertLocation::Before(before.index().unwrap()), builder) + } + + /// Registers a [`ProgressBarBuilder`] after an existing progress bar. + pub fn register_after(&self, after: &ProgressBar, builder: ProgressBarBuilder) -> ProgressBar { + self.internalize_builder(InsertLocation::After(after.index().unwrap()), builder) + } + /// Adds a progress bar. /// /// The progress bar added will have the draw target changed to a @@ -80,73 +126,86 @@ impl MultiProgress { /// The progress bar will be positioned below all other bars currently /// in the [`MultiProgress`]. /// - /// Adding a progress bar that is already a member of the [`MultiProgress`] + /// Adding a [`ProgressBar`] that is already a member of the [`MultiProgress`] /// will have no effect. + #[deprecated( + note = "use `MultiProgress::register` with a `ProgressBarBuilder` instead to avoid premature draws (see #677)" + )] pub fn add(&self, pb: ProgressBar) -> ProgressBar { - self.internalize(InsertLocation::End, pb) + self.internalize_pb(InsertLocation::End, pb) } - /// Inserts a progress bar. + /// Inserts a progress bar at the given index. /// /// The progress bar inserted at position `index` will have the draw /// target changed to a remote draw target that is intercepted by the /// multi progress object overriding custom [`ProgressDrawTarget`] settings. /// - /// If `index >= MultiProgressState::objects.len()`, the progress bar - /// is added to the end of the list. + /// If `index` is greater than or equal to the number of currently tracked + /// progress bars, the bar is added to the end of the list. /// - /// Inserting a progress bar that is already a member of the [`MultiProgress`] + /// Inserting a [`ProgressBar`] that is already a member of the [`MultiProgress`] /// will have no effect. + #[deprecated( + note = "use `MultiProgress::register_at` with a `ProgressBarBuilder` instead to avoid premature draws (see #677)" + )] pub fn insert(&self, index: usize, pb: ProgressBar) -> ProgressBar { - self.internalize(InsertLocation::Index(index), pb) + self.internalize_pb(InsertLocation::Index(index), pb) } - /// Inserts a progress bar from the back. + /// Inserts a progress bar at the given index, counting from the back. /// - /// The progress bar inserted at position `MultiProgressState::objects.len() - index` - /// will have the draw target changed to a remote draw target that is - /// intercepted by the multi progress object overriding custom - /// [`ProgressDrawTarget`] settings. + /// The progress bar is inserted counting from the end of the list. /// - /// If `index >= MultiProgressState::objects.len()`, the progress bar - /// is added to the start of the list. + /// If `index` is greater than or equal to the number of currently tracked + /// progress bars, the bar is added to the start of the list. /// - /// Inserting a progress bar that is already a member of the [`MultiProgress`] + /// Inserting a [`ProgressBar`] that is already a member of the [`MultiProgress`] /// will have no effect. + #[deprecated( + note = "use `MultiProgress::register_from_back` with a `ProgressBarBuilder` instead to avoid premature draws (see #677)" + )] pub fn insert_from_back(&self, index: usize, pb: ProgressBar) -> ProgressBar { - self.internalize(InsertLocation::IndexFromBack(index), pb) + self.internalize_pb(InsertLocation::IndexFromBack(index), pb) } /// Inserts a progress bar before an existing one. /// - /// The progress bar added will have the draw target changed to a + /// The resulting progress bar will have the draw target changed to a /// remote draw target that is intercepted by the multi progress /// object overriding custom [`ProgressDrawTarget`] settings. /// - /// Inserting a progress bar that is already a member of the [`MultiProgress`] + /// Inserting a [`ProgressBar`] that is already a member of the [`MultiProgress`] /// will have no effect. + #[deprecated( + note = "use `MultiProgress::register_before` with a `ProgressBarBuilder` instead to avoid premature draws (see #677)" + )] pub fn insert_before(&self, before: &ProgressBar, pb: ProgressBar) -> ProgressBar { - self.internalize(InsertLocation::Before(before.index().unwrap()), pb) + self.internalize_pb(InsertLocation::Before(before.index().unwrap()), pb) } /// Inserts a progress bar after an existing one. /// - /// The progress bar added will have the draw target changed to a + /// The resulting progress bar will have the draw target changed to a /// remote draw target that is intercepted by the multi progress /// object overriding custom [`ProgressDrawTarget`] settings. /// - /// Inserting a progress bar that is already a member of the [`MultiProgress`] + /// Inserting a [`ProgressBar`] that is already a member of the [`MultiProgress`] /// will have no effect. + #[deprecated( + note = "use `MultiProgress::register_after` with a `ProgressBarBuilder` instead to avoid premature draws (see #677)" + )] pub fn insert_after(&self, after: &ProgressBar, pb: ProgressBar) -> ProgressBar { - self.internalize(InsertLocation::After(after.index().unwrap()), pb) + self.internalize_pb(InsertLocation::After(after.index().unwrap()), pb) } /// Removes a progress bar. /// - /// The progress bar is removed only if it was previously inserted or added - /// by the methods [`MultiProgress::insert`] or [`MultiProgress::add`]. - /// If the passed progress bar does not satisfy the condition above, - /// the `remove` method does nothing. + /// The progress bar is removed only if it was previously added to this + /// [`MultiProgress`] via any of the [`register`](MultiProgress::register) / + /// `register_*` methods (or the deprecated [`add`](MultiProgress::add) / + /// `insert*` methods). If the passed progress bar does not satisfy the + /// condition above, the `remove` method does nothing. pub fn remove(&self, pb: &ProgressBar) { let mut state = pb.state(); let idx = match &state.draw_target.remote() { @@ -162,12 +221,38 @@ impl MultiProgress { self.state.write().unwrap().remove_idx(idx); } - fn internalize(&self, location: InsertLocation, pb: ProgressBar) -> ProgressBar { + fn internalize_pb(&self, location: InsertLocation, pb: ProgressBar) -> ProgressBar { let mut state = self.state.write().unwrap(); let idx = state.insert(location); drop(state); - pb.set_draw_target(ProgressDrawTarget::new_remote(self.state.clone(), idx)); + let draw_target = ProgressDrawTarget::new_remote(self.state.clone(), idx); + pb.set_draw_target(draw_target); + pb + } + + fn internalize_builder( + &self, + location: InsertLocation, + builder: ProgressBarBuilder, + ) -> ProgressBar { + // Phase 1: read is_stderr under a brief read lock. + let is_stderr = self.state.read().unwrap().draw_target.is_stderr(); + + // Phase 2: build the ProgressBar (panic-safe — no MultiState slot is held, + // so a panic here cannot leak a slot). + let (pb, steady_tick) = builder.build_unregistered(is_stderr); + + // Phase 3: reserve the slot and wire up the remote draw target. + let idx = self.state.write().unwrap().insert(location); + let draw_target = ProgressDrawTarget::new_remote(self.state.clone(), idx); + pb.set_draw_target(draw_target); + + // Phase 4: start steady tick LAST (spawns a thread that reads the draw target). + if let Some(interval) = steady_tick { + pb.enable_steady_tick(interval); + } + pb } @@ -518,9 +603,10 @@ enum InsertLocation { #[cfg(test)] mod tests { - use crate::{MultiProgress, ProgressBar, ProgressDrawTarget}; + use crate::{MultiProgress, ProgressBar, ProgressBarBuilder, ProgressDrawTarget}; #[test] + #[allow(deprecated)] fn late_pb_drop() { let pb = ProgressBar::new(10); let mpb = MultiProgress::new(); @@ -541,20 +627,20 @@ mod tests { #[test] fn multi_progress_hidden() { let mpb = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); - let pb = mpb.add(ProgressBar::new(123)); + let pb = mpb.register(ProgressBarBuilder::new(123)); pb.finish(); } #[test] fn multi_progress_modifications() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); - let p2 = mp.add(ProgressBar::new(1)); - let p3 = mp.add(ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); + let p2 = mp.register(ProgressBarBuilder::new(1)); + let p3 = mp.register(ProgressBarBuilder::new(1)); mp.remove(&p2); mp.remove(&p1); - let p4 = mp.insert(1, ProgressBar::new(1)); + let p4 = mp.register_at(1, ProgressBarBuilder::new(1)); let state = mp.state.read().unwrap(); // the removed place for p1 is reused @@ -583,13 +669,13 @@ mod tests { } #[test] - fn multi_progress_insert_from_back() { + fn multi_progress_register_from_back() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); - let p2 = mp.add(ProgressBar::new(1)); - let p3 = mp.insert_from_back(1, ProgressBar::new(1)); - let p4 = mp.insert_from_back(10, ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); + let p2 = mp.register(ProgressBarBuilder::new(1)); + let p3 = mp.register_from_back(1, ProgressBarBuilder::new(1)); + let p4 = mp.register_from_back(10, ProgressBarBuilder::new(1)); let state = mp.state.read().unwrap(); assert_eq!(state.ordering, vec![4, 0, 1, 3, 2]); @@ -601,13 +687,13 @@ mod tests { } #[test] - fn multi_progress_insert_after() { + fn multi_progress_register_after() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); - let p2 = mp.add(ProgressBar::new(1)); - let p3 = mp.insert_after(&p2, ProgressBar::new(1)); - let p4 = mp.insert_after(&p0, ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); + let p2 = mp.register(ProgressBarBuilder::new(1)); + let p3 = mp.register_after(&p2, ProgressBarBuilder::new(1)); + let p4 = mp.register_after(&p0, ProgressBarBuilder::new(1)); let state = mp.state.read().unwrap(); assert_eq!(state.ordering, vec![0, 4, 1, 2, 3]); @@ -619,13 +705,13 @@ mod tests { } #[test] - fn multi_progress_insert_before() { + fn multi_progress_register_before() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); - let p2 = mp.add(ProgressBar::new(1)); - let p3 = mp.insert_before(&p0, ProgressBar::new(1)); - let p4 = mp.insert_before(&p2, ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); + let p2 = mp.register(ProgressBarBuilder::new(1)); + let p3 = mp.register_before(&p0, ProgressBarBuilder::new(1)); + let p4 = mp.register_before(&p2, ProgressBarBuilder::new(1)); let state = mp.state.read().unwrap(); assert_eq!(state.ordering, vec![3, 0, 1, 4, 2]); @@ -637,15 +723,15 @@ mod tests { } #[test] - fn multi_progress_insert_before_and_after() { + fn multi_progress_register_before_and_after() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); - let p2 = mp.add(ProgressBar::new(1)); - let p3 = mp.insert_before(&p0, ProgressBar::new(1)); - let p4 = mp.insert_after(&p3, ProgressBar::new(1)); - let p5 = mp.insert_after(&p3, ProgressBar::new(1)); - let p6 = mp.insert_before(&p1, ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); + let p2 = mp.register(ProgressBarBuilder::new(1)); + let p3 = mp.register_before(&p0, ProgressBarBuilder::new(1)); + let p4 = mp.register_after(&p3, ProgressBarBuilder::new(1)); + let p5 = mp.register_after(&p3, ProgressBarBuilder::new(1)); + let p6 = mp.register_before(&p1, ProgressBarBuilder::new(1)); let state = mp.state.read().unwrap(); assert_eq!(state.ordering, vec![3, 5, 4, 0, 6, 1, 2]); @@ -661,8 +747,8 @@ mod tests { #[test] fn multi_progress_multiple_remove() { let mp = MultiProgress::new(); - let p0 = mp.add(ProgressBar::new(1)); - let p1 = mp.add(ProgressBar::new(1)); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(1)); // double remove beyond the first one have no effect mp.remove(&p0); mp.remove(&p0); @@ -682,6 +768,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn mp_no_crash_double_add() { let mp = MultiProgress::new(); let pb = mp.add(ProgressBar::new(10)); diff --git a/src/progress_bar_builder.rs b/src/progress_bar_builder.rs new file mode 100644 index 00000000..818316df --- /dev/null +++ b/src/progress_bar_builder.rs @@ -0,0 +1,436 @@ +use std::borrow::Cow; +use std::fmt; +use std::time::Duration; + +use crate::draw_target::ProgressDrawTarget; +use crate::progress_bar::ProgressBar; +use crate::state::ProgressFinish; +use crate::style::ProgressStyle; + +/// A deferred progress bar configuration for use with [`MultiProgress`]. +/// +/// Unlike [`ProgressBar`], a `ProgressBarBuilder` never draws to the terminal on +/// its own. Configuration is captured and applied only when the builder is +/// registered with a [`MultiProgress`] via methods like +/// [`MultiProgress::register`]. +/// +/// This avoids a common footgun where a [`ProgressBar`] is configured (triggering +/// premature draws to stderr) before being added to a [`MultiProgress`], causing +/// screen corruption. See [#677] for details. +/// +/// Dropping a `ProgressBarBuilder` without registering it is a no-op — no +/// resources are allocated until materialization. +/// +/// # Migration +/// +/// Passing a [`ProgressBar`] directly to [`MultiProgress::add`] is deprecated. +/// To migrate, replace: +/// +/// ```rust,ignore +/// let pb = mp.add(ProgressBar::new(100)); +/// pb.set_message("downloading"); +/// pb.enable_steady_tick(Duration::from_millis(100)); +/// ``` +/// +/// with: +/// +/// ```rust,ignore +/// let pb = mp.register( +/// ProgressBarBuilder::new(100) +/// .with_message("downloading") +/// .with_steady_tick(Duration::from_millis(100)) +/// ); +/// ``` +/// +/// ## Behavior difference: style colors for non-stderr draw targets +/// +/// When migrating code that configures a style on a [`MultiProgress`] whose +/// draw target is **not** stderr (e.g. [`ProgressDrawTarget::term_like`]), +/// there is a subtle behavior difference. The old path created a +/// [`ProgressBar`] that defaulted to stderr, then applied the style via +/// [`ProgressBar::set_style`]; because that method's stderr-color handling +/// checks the pb's *current* draw target, a freshly-constructed bar always +/// took the stderr branch and the stderr-color variant of the style was +/// applied before the remote draw target was swapped in. The new path reads +/// the MultiProgress's draw target to decide whether to apply stderr color +/// handling, so custom draw targets get their natural (non-stderr) color +/// treatment. This is usually the desired behavior, but it can produce +/// different colors than the deprecated path for the same style. +/// +/// [`ProgressDrawTarget::term_like`]: crate::ProgressDrawTarget::term_like +/// [`ProgressBar::set_style`]: crate::ProgressBar::set_style +/// +/// # Example +/// +/// ```rust +/// use std::time::Duration; +/// use indicatif::{ProgressBarBuilder, MultiProgress, ProgressStyle}; +/// +/// let mp = MultiProgress::new(); +/// let pb = mp.register( +/// ProgressBarBuilder::new(100) +/// .with_style(ProgressStyle::with_template("{bar:40} {pos}/{len} {msg}").unwrap()) +/// .with_message("downloading") +/// .with_steady_tick(Duration::from_millis(100)) +/// ); +/// ``` +/// +/// [`MultiProgress`]: crate::MultiProgress +/// [`MultiProgress::register`]: crate::MultiProgress::register +/// [`MultiProgress::add`]: crate::MultiProgress::add +/// [`ProgressBar`]: crate::ProgressBar +/// [#677]: https://github.com/console-rs/indicatif/issues/677 +#[derive(Clone)] +pub struct ProgressBarBuilder { + len: Option, + style: Option, + message: Option>, + prefix: Option>, + position: Option, + elapsed: Option, + tab_width: Option, + on_finish: Option, + steady_tick: Option, +} + +// ProgressStyle doesn't implement Debug, so we print all other fields +impl fmt::Debug for ProgressBarBuilder { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ProgressBarBuilder") + .field("len", &self.len) + .field("message", &self.message) + .field("prefix", &self.prefix) + .field("position", &self.position) + .field("elapsed", &self.elapsed) + .field("tab_width", &self.tab_width) + .field("on_finish", &self.on_finish) + .field("steady_tick", &self.steady_tick) + .finish_non_exhaustive() + } +} + +impl ProgressBarBuilder { + fn base() -> Self { + Self { + len: None, + style: None, + message: None, + prefix: None, + position: None, + elapsed: None, + tab_width: None, + on_finish: None, + steady_tick: None, + } + } + + /// Creates a new `ProgressBarBuilder` with a given length. + pub fn new(len: u64) -> Self { + Self { + len: Some(len), + ..Self::base() + } + } + + /// Creates a new `ProgressBarBuilder` without a specified length. + pub fn no_length() -> Self { + Self::base() + } + + /// Creates a new spinner-style `ProgressBarBuilder` (no length, default spinner style). + pub fn new_spinner() -> Self { + Self { + style: Some(ProgressStyle::default_spinner()), + ..Self::base() + } + } + + /// Sets the style for the progress bar. + pub fn with_style(mut self, style: ProgressStyle) -> Self { + self.style = Some(style); + self + } + + /// Sets the tab width for the progress bar. + pub fn with_tab_width(mut self, tab_width: usize) -> Self { + self.tab_width = Some(tab_width); + self + } + + /// Sets the prefix for the progress bar. + /// + /// For the prefix to be visible, the `{prefix}` placeholder must be present in the template + /// (see [`ProgressStyle`]). + pub fn with_prefix(mut self, prefix: impl Into>) -> Self { + self.prefix = Some(prefix.into()); + self + } + + /// Sets the message for the progress bar. + /// + /// For the message to be visible, the `{msg}` placeholder must be present in the template + /// (see [`ProgressStyle`]). + pub fn with_message(mut self, message: impl Into>) -> Self { + self.message = Some(message.into()); + self + } + + /// Sets the initial position for the progress bar. + pub fn with_position(mut self, pos: u64) -> Self { + self.position = Some(pos); + self + } + + /// Sets the elapsed time for the progress bar. + /// + /// # Panics + /// + /// Panics during materialization if `elapsed` is larger than the time since system boot + /// (inherited from [`ProgressBar::with_elapsed`]). + pub fn with_elapsed(mut self, elapsed: Duration) -> Self { + self.elapsed = Some(elapsed); + self + } + + /// Sets the finish behavior for the progress bar. + pub fn with_finish(mut self, finish: ProgressFinish) -> Self { + self.on_finish = Some(finish); + self + } + + /// Enables steady tick with the given interval after materialization. + /// + /// The tick thread will only be started when this builder is registered + /// with a [`MultiProgress`]. + /// + /// [`MultiProgress`]: crate::MultiProgress + pub fn with_steady_tick(mut self, interval: Duration) -> Self { + self.steady_tick = Some(interval); + self + } + + /// Build an unregistered [`ProgressBar`] with a hidden draw target and return + /// it along with the optional steady_tick interval to apply later. + /// + /// This is **not** a user-facing "build a hidden progress bar" helper — it is + /// the panic-safe phase of materialization used internally by + /// [`MultiProgress::register`]. It applies all builder configuration (any of + /// which may panic — e.g. [`ProgressBar::with_elapsed`] panics if the elapsed + /// duration exceeds the time since system boot) BEFORE the [`MultiProgress`] + /// reserves a slot for the bar, so a panic here cannot leak a slot. The + /// caller is expected to reserve a slot, then call + /// [`ProgressBar::set_draw_target`] and [`ProgressBar::enable_steady_tick`] + /// to complete materialization. + /// + /// `is_stderr` indicates whether the [`MultiProgress`]'s draw target is stderr, + /// used for style color detection. Another thread could call + /// [`MultiProgress::set_draw_target`] after we read `is_stderr` but before the + /// style is applied, making the stderr detection stale. This matches the + /// TOCTOU window that exists on the deprecated [`MultiProgress::add`] path. + /// + /// [`MultiProgress`]: crate::MultiProgress + /// [`MultiProgress::register`]: crate::MultiProgress::register + /// [`MultiProgress::add`]: crate::MultiProgress::add + /// [`MultiProgress::set_draw_target`]: crate::MultiProgress::set_draw_target + pub(crate) fn build_unregistered(self, is_stderr: bool) -> (ProgressBar, Option) { + // PARITY: every `ProgressBar::with_*` method that has a corresponding + // field on `ProgressBarBuilder` must also be applied here. When adding a + // new `with_*` method to `ProgressBar`, mirror it onto `ProgressBarBuilder` + // and extend this chain. There is no compile-time enforcement of this + // parity. + // + // ORDER (defense-in-depth, not strictly load-bearing today): + // `BarState::set_tab_width` currently propagates the new tab_width into + // the existing prefix/message/style, and `BarState::set_style` re-applies + // `state.tab_width` to the new style — so swapping the order of + // `with_tab_width`/`with_style`/`with_prefix`/`with_message` would + // produce the same output under today's implementation. We keep + // tab_width first anyway so a future refactor of either `set_tab_width` + // or `set_style` that drops the cross-propagation doesn't silently + // change tab-expansion behavior. + + // 1. Create with a hidden target (no draws possible during construction). + let pb = ProgressBar::with_draw_target(self.len, ProgressDrawTarget::hidden()); + + // 2. Apply tab_width first so subsequent steps that read `state.tab_width` + // (prefix/message tab expansion, style tab_width) observe the user value. + let pb = if let Some(tw) = self.tab_width { + pb.with_tab_width(tw) + } else { + pb + }; + + // 3. Apply style, adjusting for stderr color detection if needed. + let pb = if let Some(mut style) = self.style { + if is_stderr { + style.set_for_stderr(); + } + pb.with_style(style) + } else { + pb + }; + + // 4. Apply remaining configuration. + let pb = if let Some(prefix) = self.prefix { + pb.with_prefix(prefix) + } else { + pb + }; + let pb = if let Some(msg) = self.message { + pb.with_message(msg) + } else { + pb + }; + let pb = if let Some(pos) = self.position { + pb.with_position(pos) + } else { + pb + }; + let pb = if let Some(elapsed) = self.elapsed { + pb.with_elapsed(elapsed) + } else { + pb + }; + let pb = if let Some(finish) = self.on_finish { + pb.with_finish(finish) + } else { + pb + }; + + (pb, self.steady_tick) + } +} + +#[cfg(test)] +mod tests { + use crate::multi::MultiProgress; + use crate::style::ProgressStyle; + + use super::*; + + #[test] + fn builder_new_sets_length() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(42)); + assert_eq!(pb.length(), Some(42)); + } + + #[test] + fn builder_no_length() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::no_length()); + assert_eq!(pb.length(), None); + } + + #[test] + fn builder_new_spinner_has_no_length() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new_spinner()); + assert_eq!(pb.length(), None); + } + + #[test] + fn builder_with_message() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(10).with_message("hello")); + assert_eq!(pb.message(), "hello"); + } + + #[test] + fn builder_with_prefix() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(10).with_prefix("[1/3]")); + assert_eq!(pb.prefix(), "[1/3]"); + } + + #[test] + fn builder_with_position() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(100).with_position(50)); + assert_eq!(pb.position(), 50); + } + + #[test] + fn builder_with_style() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let style = ProgressStyle::with_template("{msg}").unwrap(); + let pb = mp.register(ProgressBarBuilder::new(10).with_style(style)); + pb.set_message("test"); + assert_eq!(pb.message(), "test"); + } + + #[test] + fn builder_with_elapsed() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(100).with_elapsed(Duration::from_secs(42))); + assert!(pb.elapsed() >= Duration::from_secs(42)); + } + + #[test] + fn builder_with_tab_width() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register( + ProgressBarBuilder::new(10) + .with_tab_width(4) + .with_message("a\tb"), + ); + assert_eq!(pb.tab_width(), 4); + } + + #[test] + fn builder_chaining() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register( + ProgressBarBuilder::new(100) + .with_message("downloading") + .with_prefix("[1/3]") + .with_position(25), + ); + assert_eq!(pb.length(), Some(100)); + assert_eq!(pb.message(), "downloading"); + assert_eq!(pb.prefix(), "[1/3]"); + assert_eq!(pb.position(), 25); + } + + #[test] + #[allow(deprecated)] + fn backwards_compat_progress_bar_still_works() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = crate::ProgressBar::new(100); + let pb = mp.add(pb); + assert_eq!(pb.length(), Some(100)); + } + + #[test] + fn builder_with_finish() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); + pb.finish(); + assert!(pb.is_finished()); + } + + #[test] + fn builder_clone() { + let builder = ProgressBarBuilder::new(100) + .with_message("test") + .with_prefix("pfx"); + let builder2 = builder.clone(); + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let pb = mp.register(builder2); + assert_eq!(pb.message(), "test"); + assert_eq!(pb.prefix(), "pfx"); + } + + #[test] + fn builder_register_before_after() { + let mp = MultiProgress::with_draw_target(ProgressDrawTarget::hidden()); + let p0 = mp.register(ProgressBarBuilder::new(1)); + let p1 = mp.register(ProgressBarBuilder::new(2)); + let p2 = mp.register_after(&p1, ProgressBarBuilder::new(3)); + let p3 = mp.register_before(&p0, ProgressBarBuilder::new(4)); + assert_eq!(p0.length(), Some(1)); + assert_eq!(p1.length(), Some(2)); + assert_eq!(p2.length(), Some(3)); + assert_eq!(p3.length(), Some(4)); + } +} diff --git a/tests/multi-autodrop.rs b/tests/multi-autodrop.rs index 5589ad60..12641197 100644 --- a/tests/multi-autodrop.rs +++ b/tests/multi-autodrop.rs @@ -1,13 +1,13 @@ use std::thread; use std::time::Duration; -use indicatif::{MultiProgress, ProgressBar}; +use indicatif::{MultiProgress, ProgressBarBuilder}; #[test] fn main() { let pb = { let m = MultiProgress::new(); - m.add(ProgressBar::new(10)) + m.register(ProgressBarBuilder::new(10)) // The MultiProgress is dropped here. }; diff --git a/tests/render.rs b/tests/render.rs index c6fc18e1..5304e61b 100644 --- a/tests/render.rs +++ b/tests/render.rs @@ -3,8 +3,8 @@ use std::time::Duration; use indicatif::{ - InMemoryTerm, MultiProgress, MultiProgressAlignment, ProgressBar, ProgressDrawTarget, - ProgressFinish, ProgressStyle, TermLike, + InMemoryTerm, MultiProgress, MultiProgressAlignment, ProgressBar, ProgressBarBuilder, + ProgressDrawTarget, ProgressFinish, ProgressStyle, TermLike, }; use pretty_assertions::assert_eq; @@ -133,7 +133,7 @@ fn multi_progress_single_bar_and_leave() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); assert_eq!(in_mem.contents(), String::new()); @@ -156,7 +156,7 @@ fn multi_progress_single_bar_and_clear() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10)); + let pb1 = mp.register(ProgressBarBuilder::new(10)); assert_eq!(in_mem.contents(), String::new()); @@ -176,8 +176,8 @@ fn multi_progress_two_bars() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); - let pb2 = mp.add(ProgressBar::new(5)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); + let pb2 = mp.register(ProgressBarBuilder::new(5)); assert_eq!(in_mem.contents(), String::new()); @@ -220,9 +220,9 @@ fn multi_progress() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); - let pb2 = mp.add(ProgressBar::new(5)); - let pb3 = mp.add(ProgressBar::new(100)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); + let pb2 = mp.register(ProgressBarBuilder::new(5)); + let pb3 = mp.register(ProgressBarBuilder::new(100)); assert_eq!(in_mem.contents(), String::new()); @@ -285,9 +285,9 @@ fn multi_progress_println() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10)); - let pb2 = mp.add(ProgressBar::new(5)); - let pb3 = mp.add(ProgressBar::new(100)); + let pb1 = mp.register(ProgressBarBuilder::new(10)); + let pb2 = mp.register(ProgressBarBuilder::new(5)); + let pb3 = mp.register(ProgressBarBuilder::new(100)); assert_eq!(in_mem.contents(), ""); @@ -351,8 +351,8 @@ fn multi_progress_suspend() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10)); - let pb2 = mp.add(ProgressBar::new(10)); + let pb1 = mp.register(ProgressBarBuilder::new(10)); + let pb2 = mp.register(ProgressBarBuilder::new(10)); assert_eq!(in_mem.contents(), ""); @@ -425,7 +425,7 @@ fn multi_progress_move_cursor() { MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); mp.set_move_cursor(true); - let pb1 = mp.add(ProgressBar::new(10)); + let pb1 = mp.register(ProgressBarBuilder::new(10)); pb1.tick(); assert_eq!( in_mem.moves_since_last_check(), @@ -436,7 +436,7 @@ Flush "# ); - let pb2 = mp.add(ProgressBar::new(10)); + let pb2 = mp.register(ProgressBarBuilder::new(10)); pb2.tick(); assert_eq!( in_mem.moves_since_last_check(), @@ -471,10 +471,7 @@ fn multi_progress_println_bar_with_target() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb = mp.add(ProgressBar::with_draw_target( - Some(10), - ProgressDrawTarget::term_like(Box::new(in_mem.clone())), - )); + let pb = mp.register(ProgressBarBuilder::new(10)); assert_eq!(in_mem.contents(), ""); @@ -499,8 +496,8 @@ fn ticker_drop() { let mut spinner: Option = None; for i in 0..5 { - let new_spinner = mp.add( - ProgressBar::new_spinner() + let new_spinner = mp.register( + ProgressBarBuilder::new_spinner() .with_finish(ProgressFinish::AndLeave) .with_message(format!("doing stuff {i}")), ); @@ -515,13 +512,52 @@ fn ticker_drop() { ); } +#[test] +fn builder_with_steady_tick() { + // Happy-path test for ProgressBarBuilder::with_steady_tick: after registration, + // the ticker thread spawns and draws to the MultiProgress's draw target (here + // an InMemoryTerm), so the configured message eventually appears. The + // structural fix for https://github.com/console-rs/indicatif/issues/677 is + // that ProgressBarBuilder owns no ProgressBar, so it cannot draw before + // `register` — that guarantee is enforced by the type system, not this test. + let in_mem = InMemoryTerm::new(10, 80); + let mp = + MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); + + // Building the builder does not touch the terminal. + let builder = ProgressBarBuilder::new_spinner() + .with_steady_tick(Duration::from_millis(10)) + .with_message("ticking"); + assert_eq!(in_mem.contents(), ""); + + // Registering materializes the bar and starts the ticker thread drawing + // into the MultiProgress's InMemoryTerm target. + let pb = mp.register(builder); + + // Wait for the background ticker to produce at least one draw. + let deadline = std::time::Instant::now() + Duration::from_secs(1); + loop { + if in_mem.contents().contains("ticking") { + break; + } + assert!( + std::time::Instant::now() < deadline, + "timed out waiting for steady_tick to draw; got: {:?}", + in_mem.contents() + ); + std::thread::sleep(Duration::from_millis(5)); + } + + pb.finish_with_message("done"); +} + #[test] fn manually_inc_ticker() { let in_mem = InMemoryTerm::new(10, 80); let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let spinner = mp.add(ProgressBar::new_spinner().with_message("msg")); + let spinner = mp.register(ProgressBarBuilder::new_spinner().with_message("msg")); assert_eq!(in_mem.contents(), ""); @@ -544,9 +580,9 @@ fn multi_progress_prune_zombies() { MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); let pb0 = mp - .add(ProgressBar::new(10)) + .register(ProgressBarBuilder::new(10)) .with_finish(ProgressFinish::AndLeave); - let pb1 = mp.add(ProgressBar::new(15)); + let pb1 = mp.register(ProgressBarBuilder::new(15)); pb0.tick(); assert_eq!( in_mem.contents(), @@ -582,15 +618,15 @@ fn multi_progress_prune_zombies_2() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); - let pb2 = mp.add(ProgressBar::new(5)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); + let pb2 = mp.register(ProgressBarBuilder::new(5)); let pb3 = mp - .add(ProgressBar::new(100)) + .register(ProgressBarBuilder::new(100)) .with_finish(ProgressFinish::Abandon); let pb4 = mp - .add(ProgressBar::new(500)) + .register(ProgressBarBuilder::new(500)) .with_finish(ProgressFinish::AndLeave); - let pb5 = mp.add(ProgressBar::new(7)); + let pb5 = mp.register(ProgressBarBuilder::new(7)); assert_eq!(in_mem.contents(), String::new()); @@ -737,7 +773,7 @@ fn basic_tab_expansion() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let spinner = mp.add(ProgressBar::new_spinner().with_message("Test\t:)")); + let spinner = mp.register(ProgressBarBuilder::new_spinner().with_message("Test\t:)")); spinner.tick(); // 8 is the default number of spaces @@ -753,8 +789,8 @@ fn tab_expansion_in_template() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let spinner = mp.add( - ProgressBar::new_spinner() + let spinner = mp.register( + ProgressBarBuilder::new_spinner() .with_message("Test\t:)") .with_prefix("Pre\tfix!") .with_style(ProgressStyle::with_template("{spinner}{prefix}\t{msg}").unwrap()), @@ -779,8 +815,8 @@ fn progress_style_tab_width_unification() { // Style will have default of 8 spaces for tabs let style = ProgressStyle::with_template("{msg}\t{msg}").unwrap(); - let spinner = mp.add( - ProgressBar::new_spinner() + let spinner = mp.register( + ProgressBarBuilder::new_spinner() .with_message("OK") .with_tab_width(4), ); @@ -791,6 +827,37 @@ fn progress_style_tab_width_unification() { assert_eq!(in_mem.contents(), "OK OK"); } +#[test] +fn builder_tab_width_propagates_to_style() { + // Pins user-visible behavior: with `with_tab_width(4)` and a style whose + // template contains a literal tab, that tab is rendered as 4 spaces. + // + // This is a behavioral test, not an ordering-invariant test. Because + // `BarState::set_tab_width` propagates to the existing style and + // `BarState::set_style` re-applies `state.tab_width` to the new style, + // swapping the order in which `with_tab_width` and `with_style` are + // applied would produce the same output under the current implementation. + // A future refactor that drops either propagation could silently change + // tab rendering for builder-constructed bars; this assertion is the + // behavioral line that would catch such a regression. + let in_mem = InMemoryTerm::new(10, 80); + let mp = + MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); + + let spinner = mp.register( + ProgressBarBuilder::new_spinner() + .with_tab_width(4) + .with_style(ProgressStyle::with_template("{prefix}\t{msg}").unwrap()) + .with_prefix("a") + .with_message("b"), + ); + spinner.tick(); + // The literal \t in the template is expanded to 4 spaces (tab_width), + // not the default 8. Tab expansion replaces \t with a literal run of + // spaces equal to tab_width (see TabExpandedString in src/state.rs). + assert_eq!(in_mem.contents(), "a b"); +} + #[test] fn multi_progress_clear_println() { let in_mem = InMemoryTerm::new(10, 80); @@ -826,15 +893,15 @@ fn _multi_progress_clear_zombies(ticks: usize) { MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); let style = ProgressStyle::with_template("{msg}").unwrap(); - let pb1 = mp.add( - ProgressBar::new_spinner() + let pb1 = mp.register( + ProgressBarBuilder::new_spinner() .with_style(style.clone()) .with_message("pb1"), ); pb1.tick(); - let pb2 = mp.add( - ProgressBar::new_spinner() + let pb2 = mp.register( + ProgressBarBuilder::new_spinner() .with_style(style) .with_message("pb2"), ); @@ -862,20 +929,20 @@ fn multi_zombie_handling() { MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); let style = ProgressStyle::with_template("{msg}").unwrap(); - let pb1 = mp.add( - ProgressBar::new_spinner() + let pb1 = mp.register( + ProgressBarBuilder::new_spinner() .with_style(style.clone()) .with_message("pb1"), ); pb1.tick(); - let pb2 = mp.add( - ProgressBar::new_spinner() + let pb2 = mp.register( + ProgressBarBuilder::new_spinner() .with_style(style.clone()) .with_message("pb2"), ); pb2.tick(); - let pb3 = mp.add( - ProgressBar::new_spinner() + let pb3 = mp.register( + ProgressBarBuilder::new_spinner() .with_style(style) .with_message("pb3"), ); @@ -921,8 +988,8 @@ fn multi_progress_multiline_msg() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new_spinner().with_message("test1")); - let pb2 = mp.add(ProgressBar::new_spinner().with_message("test2")); + let pb1 = mp.register(ProgressBarBuilder::new_spinner().with_message("test1")); + let pb2 = mp.register(ProgressBarBuilder::new_spinner().with_message("test2")); assert_eq!(in_mem.contents(), ""); @@ -1008,8 +1075,8 @@ fn multi_progress_bottom_alignment() { MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); mp.set_alignment(MultiProgressAlignment::Bottom); - let pb1 = mp.add(ProgressBar::new_spinner().with_message("test1")); - let pb2 = mp.add(ProgressBar::new_spinner().with_message("test2")); + let pb1 = mp.register(ProgressBarBuilder::new_spinner().with_message("test1")); + let pb2 = mp.register(ProgressBarBuilder::new_spinner().with_message("test2")); pb1.tick(); pb2.tick(); @@ -1113,9 +1180,9 @@ fn multi_progress_println_terminal_wrap() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10)); - let pb2 = mp.add(ProgressBar::new(5)); - let pb3 = mp.add(ProgressBar::new(100)); + let pb1 = mp.register(ProgressBarBuilder::new(10)); + let pb2 = mp.register(ProgressBarBuilder::new(5)); + let pb3 = mp.register(ProgressBarBuilder::new(100)); assert_eq!(in_mem.contents(), ""); @@ -1216,11 +1283,10 @@ fn multi_progress_many_bars() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); let mut spinners = vec![]; for i in 0..7 { - let spinner = ProgressBar::new_spinner().with_message(i.to_string()); - mp.add(spinner.clone()); + let spinner = mp.register(ProgressBarBuilder::new_spinner().with_message(i.to_string())); spinners.push(spinner); } @@ -1527,11 +1593,10 @@ fn multi_progress_many_spinners() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb1 = mp.add(ProgressBar::new(10).with_finish(ProgressFinish::AndLeave)); + let pb1 = mp.register(ProgressBarBuilder::new(10).with_finish(ProgressFinish::AndLeave)); let mut spinners = vec![]; for i in 0..7 { - let spinner = ProgressBar::new_spinner().with_message(i.to_string()); - mp.add(spinner.clone()); + let spinner = mp.register(ProgressBarBuilder::new_spinner().with_message(i.to_string())); spinners.push(spinner); } @@ -1896,7 +1961,7 @@ fn orphan_lines_message_above_multi_progress_bar() { let mp = MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone()))); - let pb = mp.add(ProgressBar::new(10)); + let pb = mp.register(ProgressBarBuilder::new(10)); orphan_lines_message_above_progress_bar_test(&pb, &in_mem); }