From 63dc9b75b17bdc084d4f45d68a8f8aaebb4282cc Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Fri, 19 Nov 2021 16:38:34 -0500 Subject: [PATCH 1/7] MultiProgres: add insert_after/insert_before Introduce internal InsertLocation enum to clarify MultiProgress:push --- examples/multi-insert-relative.rs | 75 +++++++++++++++++++++++++++++++ src/progress_bar.rs | 62 ++++++++++++++++++++----- 2 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 examples/multi-insert-relative.rs diff --git a/examples/multi-insert-relative.rs b/examples/multi-insert-relative.rs new file mode 100644 index 00000000..4f09ba15 --- /dev/null +++ b/examples/multi-insert-relative.rs @@ -0,0 +1,75 @@ +use indicatif::{MultiProgress, ProgressBar, ProgressFinish, ProgressStyle}; +use std::sync::Arc; +use std::time::Duration; + +struct Item { + name: &'static str, + steps: Vec<&'static str>, +} + +fn main() { + let mp = Arc::new(MultiProgress::new()); + let item_spinner_style = ProgressStyle::default_spinner().on_finish(ProgressFinish::AndLeave); + + let sty_main = ProgressStyle::default_bar().on_finish(ProgressFinish::AndLeave); + let step_progress_style = ProgressStyle::default_bar() + .template("{msg}: {bar:40.green/yellow} {pos:>4}/{len:4}") + .on_finish(ProgressFinish::AndLeave); + + let items_to_process = vec![ + Item { + name: "apples", + steps: vec!["removing stem", "washing", "peeling", "coring", "slicing"], + }, + Item { + name: "pears", + steps: vec!["removing stem", "washing", "coring", "dicing"], + }, + Item { + name: "oranges", + steps: vec!["peeling", "segmenting", "removing pith"], + }, + Item { + name: "cherries", + steps: vec!["washing", "pitting", "removing stems"], + }, + Item { + name: "bananas", + steps: vec!["peeling", "slicing"], + }, + ]; + + let overall_progress = + mp.add(ProgressBar::new(items_to_process.len() as u64).with_style(sty_main)); + overall_progress.tick(); + + for item in items_to_process { + let item_spinner = mp.insert_before( + overall_progress.clone(), + ProgressBar::new_spinner() + .with_message(item.name) + .with_style(item_spinner_style.clone()), + ); + item_spinner.enable_steady_tick(100); + + std::thread::sleep(Duration::from_secs(3)); + + let mut next_step_target = item_spinner.clone(); + + for step in item.steps { + let step_progress = mp.insert_after( + next_step_target.clone(), + ProgressBar::new(10) + .with_message(step) + .with_style(step_progress_style.clone()), + ); + next_step_target = step_progress.clone(); + for _ in 0..9 { + step_progress.inc(1); + std::thread::sleep(Duration::from_millis(100)); + } + } + + overall_progress.inc(1); + } +} diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 4cc0cf03..76fd6051 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -558,6 +558,14 @@ impl Default for MultiProgress { } } +enum InsertLocation { + End, + Index(usize), + IndexFromBack(usize), + After(ProgressBar), + Before(ProgressBar), +} + impl MultiProgress { /// Creates a new multi progress object. /// @@ -608,7 +616,7 @@ impl MultiProgress { /// remote draw target that is intercepted by the multi progress /// object overriding custom `ProgressDrawTarget` settings. pub fn add(&self, pb: ProgressBar) -> ProgressBar { - self.push(None, false, pb) + self.push(InsertLocation::End, pb) } /// Inserts a progress bar. @@ -620,7 +628,7 @@ impl MultiProgress { /// If `index >= MultiProgressState::objects.len()`, the progress bar /// is added to the end of the list. pub fn insert(&self, index: usize, pb: ProgressBar) -> ProgressBar { - self.push(Some(index), false, pb) + self.push(InsertLocation::Index(index), pb) } /// Inserts a progress bar from the back. @@ -633,10 +641,28 @@ impl MultiProgress { /// If `index >= MultiProgressState::objects.len()`, the progress bar /// is added to the start of the list. pub fn insert_from_back(&self, index: usize, pb: ProgressBar) -> ProgressBar { - self.push(Some(index), true, pb) + self.push(InsertLocation::IndexFromBack(index), pb) + } + + /// Inserts a progress bar before an existing one. + /// + /// The progress bar added will have the draw target changed to a + /// remote draw target that is intercepted by the multi progress + /// object overriding custom `ProgressDrawTarget` settings. + pub fn insert_before(&self, before: ProgressBar, pb: ProgressBar) -> ProgressBar { + self.push(InsertLocation::Before(before), pb) + } + + /// Inserts a progress bar after an existing one. + /// + /// The progress bar added will have the draw target changed to a + /// remote draw target that is intercepted by the multi progress + /// object overriding custom `ProgressDrawTarget` settings. + pub fn insert_after(&self, after: ProgressBar, pb: ProgressBar) -> ProgressBar { + self.push(InsertLocation::After(after), pb) } - fn push(&self, pos: Option, from_back: bool, pb: ProgressBar) -> ProgressBar { + fn push(&self, pos: InsertLocation, pb: ProgressBar) -> ProgressBar { let mut state = self.state.write().unwrap(); let idx = match state.free_set.pop() { Some(idx) => { @@ -650,15 +676,29 @@ impl MultiProgress { }; match pos { - Some(pos) => { - let pos = match from_back { - true => state.ordering.len().saturating_sub(pos), - false => Ord::min(pos, state.ordering.len()), - }; - + InsertLocation::End => state.ordering.push(idx), + InsertLocation::Index(idx) => { + let pos = Ord::min(idx, state.ordering.len()); + state.ordering.insert(pos, idx); + } + InsertLocation::IndexFromBack(idx) => { + let pos = state.ordering.len().saturating_sub(idx); + state.ordering.insert(pos, idx); + } + InsertLocation::After(after) => { + let after_idx = after.state.lock().unwrap().draw_target.remote().unwrap().1; + let pos = state.ordering.iter().position(|i| *i == after_idx).unwrap(); + state.ordering.insert(pos + 1, idx); + } + InsertLocation::Before(before) => { + let before_idx = before.state.lock().unwrap().draw_target.remote().unwrap().1; + let pos = state + .ordering + .iter() + .position(|i| *i == before_idx) + .unwrap(); state.ordering.insert(pos, idx); } - _ => state.ordering.push(idx), } assert!( From 65430ac5b08efd7798a018abe653a9541cf7c781 Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Tue, 14 Dec 2021 17:12:12 -0500 Subject: [PATCH 2/7] MultiProgress: change insert_after/insert_before to take &ProgressBar Otherwise the user risks dropping the ProgressBar they are using for relative placement. --- examples/multi-insert-relative.rs | 4 ++-- src/progress_bar.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/multi-insert-relative.rs b/examples/multi-insert-relative.rs index 4f09ba15..2128aa46 100644 --- a/examples/multi-insert-relative.rs +++ b/examples/multi-insert-relative.rs @@ -45,7 +45,7 @@ fn main() { for item in items_to_process { let item_spinner = mp.insert_before( - overall_progress.clone(), + &overall_progress, ProgressBar::new_spinner() .with_message(item.name) .with_style(item_spinner_style.clone()), @@ -58,7 +58,7 @@ fn main() { for step in item.steps { let step_progress = mp.insert_after( - next_step_target.clone(), + &next_step_target, ProgressBar::new(10) .with_message(step) .with_style(step_progress_style.clone()), diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 76fd6051..319dec7d 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -558,12 +558,12 @@ impl Default for MultiProgress { } } -enum InsertLocation { +enum InsertLocation<'a> { End, Index(usize), IndexFromBack(usize), - After(ProgressBar), - Before(ProgressBar), + After(&'a ProgressBar), + Before(&'a ProgressBar), } impl MultiProgress { @@ -649,7 +649,7 @@ impl MultiProgress { /// The progress bar added will have the draw target changed to a /// remote draw target that is intercepted by the multi progress /// object overriding custom `ProgressDrawTarget` settings. - pub fn insert_before(&self, before: ProgressBar, pb: ProgressBar) -> ProgressBar { + pub fn insert_before(&self, before: &ProgressBar, pb: ProgressBar) -> ProgressBar { self.push(InsertLocation::Before(before), pb) } @@ -658,7 +658,7 @@ impl MultiProgress { /// The progress bar added will have the draw target changed to a /// remote draw target that is intercepted by the multi progress /// object overriding custom `ProgressDrawTarget` settings. - pub fn insert_after(&self, after: ProgressBar, pb: ProgressBar) -> ProgressBar { + pub fn insert_after(&self, after: &ProgressBar, pb: ProgressBar) -> ProgressBar { self.push(InsertLocation::After(after), pb) } From 5331e3562260344327fb28100bd19917f8eb393f Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Tue, 14 Dec 2021 17:18:03 -0500 Subject: [PATCH 3/7] MultiProgress: fix bug caused by shadowing idx variable --- src/progress_bar.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 319dec7d..aefc95b0 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -677,12 +677,12 @@ impl MultiProgress { match pos { InsertLocation::End => state.ordering.push(idx), - InsertLocation::Index(idx) => { - let pos = Ord::min(idx, state.ordering.len()); + InsertLocation::Index(pos) => { + let pos = Ord::min(pos, state.ordering.len()); state.ordering.insert(pos, idx); } - InsertLocation::IndexFromBack(idx) => { - let pos = state.ordering.len().saturating_sub(idx); + InsertLocation::IndexFromBack(pos) => { + let pos = state.ordering.len().saturating_sub(pos); state.ordering.insert(pos, idx); } InsertLocation::After(after) => { From d9971bb7f75ce430b7eea38942c47964deb19f43 Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Tue, 14 Dec 2021 17:24:52 -0500 Subject: [PATCH 4/7] Add tests for insert_after/insert_before --- src/progress_bar.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/progress_bar.rs b/src/progress_bar.rs index aefc95b0..0267a9b0 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -920,6 +920,64 @@ mod tests { assert_eq!(extract_index(&p4), 4); } + #[test] + fn multi_progress_insert_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 state = mp.state.read().unwrap(); + assert_eq!(state.ordering, vec![0, 4, 1, 2, 3]); + assert_eq!(extract_index(&p0), 0); + assert_eq!(extract_index(&p1), 1); + assert_eq!(extract_index(&p2), 2); + assert_eq!(extract_index(&p3), 3); + assert_eq!(extract_index(&p4), 4); + } + + #[test] + fn multi_progress_insert_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 state = mp.state.read().unwrap(); + assert_eq!(state.ordering, vec![3, 0, 1, 4, 2]); + assert_eq!(extract_index(&p0), 0); + assert_eq!(extract_index(&p1), 1); + assert_eq!(extract_index(&p2), 2); + assert_eq!(extract_index(&p3), 3); + assert_eq!(extract_index(&p4), 4); + } + + #[test] + fn multi_progress_insert_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 state = mp.state.read().unwrap(); + assert_eq!(state.ordering, vec![3, 5, 4, 0, 6, 1, 2]); + assert_eq!(extract_index(&p0), 0); + assert_eq!(extract_index(&p1), 1); + assert_eq!(extract_index(&p2), 2); + assert_eq!(extract_index(&p3), 3); + assert_eq!(extract_index(&p4), 4); + assert_eq!(extract_index(&p5), 5); + assert_eq!(extract_index(&p6), 6); + } + #[test] fn multi_progress_multiple_remove() { let mp = MultiProgress::new(); From cbffaf6411b1ac6006c9970408463196664fd700 Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Wed, 15 Dec 2021 11:56:32 -0500 Subject: [PATCH 5/7] Tweaks based on review feedback --- src/progress_bar.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 0267a9b0..85f97b9f 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -558,14 +558,6 @@ impl Default for MultiProgress { } } -enum InsertLocation<'a> { - End, - Index(usize), - IndexFromBack(usize), - After(&'a ProgressBar), - Before(&'a ProgressBar), -} - impl MultiProgress { /// Creates a new multi progress object. /// @@ -662,7 +654,7 @@ impl MultiProgress { self.push(InsertLocation::After(after), pb) } - fn push(&self, pos: InsertLocation, pb: ProgressBar) -> ProgressBar { + fn push(&self, location: InsertLocation, pb: ProgressBar) -> ProgressBar { let mut state = self.state.write().unwrap(); let idx = match state.free_set.pop() { Some(idx) => { @@ -675,7 +667,7 @@ impl MultiProgress { } }; - match pos { + match location { InsertLocation::End => state.ordering.push(idx), InsertLocation::Index(pos) => { let pos = Ord::min(pos, state.ordering.len()); @@ -764,6 +756,14 @@ impl WeakProgressBar { } } +enum InsertLocation<'a> { + End, + Index(usize), + IndexFromBack(usize), + After(&'a ProgressBar), + Before(&'a ProgressBar), +} + #[cfg(test)] mod tests { use super::*; From 07f8103e4f5900e4811976fb3955de213966276c Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Wed, 15 Dec 2021 12:01:53 -0500 Subject: [PATCH 6/7] remove multi-insert-relative.rs example --- examples/multi-insert-relative.rs | 75 ------------------------------- 1 file changed, 75 deletions(-) delete mode 100644 examples/multi-insert-relative.rs diff --git a/examples/multi-insert-relative.rs b/examples/multi-insert-relative.rs deleted file mode 100644 index 2128aa46..00000000 --- a/examples/multi-insert-relative.rs +++ /dev/null @@ -1,75 +0,0 @@ -use indicatif::{MultiProgress, ProgressBar, ProgressFinish, ProgressStyle}; -use std::sync::Arc; -use std::time::Duration; - -struct Item { - name: &'static str, - steps: Vec<&'static str>, -} - -fn main() { - let mp = Arc::new(MultiProgress::new()); - let item_spinner_style = ProgressStyle::default_spinner().on_finish(ProgressFinish::AndLeave); - - let sty_main = ProgressStyle::default_bar().on_finish(ProgressFinish::AndLeave); - let step_progress_style = ProgressStyle::default_bar() - .template("{msg}: {bar:40.green/yellow} {pos:>4}/{len:4}") - .on_finish(ProgressFinish::AndLeave); - - let items_to_process = vec![ - Item { - name: "apples", - steps: vec!["removing stem", "washing", "peeling", "coring", "slicing"], - }, - Item { - name: "pears", - steps: vec!["removing stem", "washing", "coring", "dicing"], - }, - Item { - name: "oranges", - steps: vec!["peeling", "segmenting", "removing pith"], - }, - Item { - name: "cherries", - steps: vec!["washing", "pitting", "removing stems"], - }, - Item { - name: "bananas", - steps: vec!["peeling", "slicing"], - }, - ]; - - let overall_progress = - mp.add(ProgressBar::new(items_to_process.len() as u64).with_style(sty_main)); - overall_progress.tick(); - - for item in items_to_process { - let item_spinner = mp.insert_before( - &overall_progress, - ProgressBar::new_spinner() - .with_message(item.name) - .with_style(item_spinner_style.clone()), - ); - item_spinner.enable_steady_tick(100); - - std::thread::sleep(Duration::from_secs(3)); - - let mut next_step_target = item_spinner.clone(); - - for step in item.steps { - let step_progress = mp.insert_after( - &next_step_target, - ProgressBar::new(10) - .with_message(step) - .with_style(step_progress_style.clone()), - ); - next_step_target = step_progress.clone(); - for _ in 0..9 { - step_progress.inc(1); - std::thread::sleep(Duration::from_millis(100)); - } - } - - overall_progress.inc(1); - } -} From 1ac5b0678a192b05dd28a692a0b23e6effe393b4 Mon Sep 17 00:00:00 2001 From: Chris Laplante Date: Wed, 15 Dec 2021 12:02:10 -0500 Subject: [PATCH 7/7] modify multi.rs example to use insert_after --- examples/multi.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/examples/multi.rs b/examples/multi.rs index 55a4c950..cc16e95e 100644 --- a/examples/multi.rs +++ b/examples/multi.rs @@ -11,6 +11,13 @@ fn main() { let pb = m.add(ProgressBar::new(128)); pb.set_style(sty.clone()); + + let pb2 = m.insert_after(&pb, ProgressBar::new(128)); + pb2.set_style(sty.clone()); + + let pb3 = m.insert_after(&pb2, ProgressBar::new(1024)); + pb3.set_style(sty); + let h1 = thread::spawn(move || { for i in 0..128 { pb.set_message(format!("item #{}", i + 1)); @@ -20,29 +27,25 @@ fn main() { pb.finish_with_message("done"); }); - let pb = m.add(ProgressBar::new(128)); - pb.set_style(sty.clone()); let h2 = thread::spawn(move || { for _ in 0..3 { - pb.set_position(0); + pb2.set_position(0); for i in 0..128 { - pb.set_message(format!("item #{}", i + 1)); - pb.inc(1); + pb2.set_message(format!("item #{}", i + 1)); + pb2.inc(1); thread::sleep(Duration::from_millis(8)); } } - pb.finish_with_message("done"); + pb2.finish_with_message("done"); }); - let pb = m.add(ProgressBar::new(1024)); - pb.set_style(sty); let _ = thread::spawn(move || { for i in 0..1024 { - pb.set_message(format!("item #{}", i + 1)); - pb.inc(1); + pb3.set_message(format!("item #{}", i + 1)); + pb3.inc(1); thread::sleep(Duration::from_millis(2)); } - pb.finish_with_message("done"); + pb3.finish_with_message("done"); }); let _ = h1.join();