From 7c0ee522b0d0aeab9388a5109c009dd737e4d1ea Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 20 Mar 2022 18:33:00 -0700 Subject: [PATCH 1/3] Replace ProgressBar's draw target after removing it from MultiProgress While we previously removed the ProgressBar's state from the MultiProgressState, we did not remove the remote draw target from the bar's draw target, so that it could start to draw into the MultiProgress state again in surprising ways. --- src/multi.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/multi.rs b/src/multi.rs index 77d2f83f..9d1aea5f 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -113,7 +113,8 @@ impl MultiProgress { /// If the passed progress bar does not satisfy the condition above, /// the `remove` method does nothing. pub fn remove(&self, pb: &ProgressBar) { - let idx = match &pb.state().draw_target.remote() { + let mut state = pb.state(); + let idx = match &state.draw_target.remote() { Some((state, idx)) => { // Check that this progress bar is owned by the current MultiProgress. assert!(Arc::ptr_eq(&self.state, state)); @@ -122,6 +123,7 @@ impl MultiProgress { _ => return, }; + state.draw_target = ProgressDrawTarget::hidden(); self.state.write().unwrap().remove_idx(idx); } @@ -433,8 +435,8 @@ mod tests { } assert_eq!(p0.index().unwrap(), 0); - assert_eq!(p1.index().unwrap(), 1); - assert_eq!(p2.index().unwrap(), 2); + assert_eq!(p1.index(), None); + assert_eq!(p2.index(), None); assert_eq!(p3.index().unwrap(), 3); } @@ -533,7 +535,7 @@ mod tests { assert_eq!(state.free_set.last(), Some(&0)); assert_eq!(state.ordering, vec![1]); - assert_eq!(p0.index().unwrap(), 0); + assert_eq!(p0.index(), None); assert_eq!(p1.index().unwrap(), 1); } } From 4bcd61200fcd118ee359d2d26d48e78a0f738ff7 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 20 Mar 2022 20:22:25 -0700 Subject: [PATCH 2/3] Don't write to terminal while panicking This might hide the panic output, making it extremely confusing how the process might have ended. --- src/draw_target.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/draw_target.rs b/src/draw_target.rs index 378fe68f..2830c4f7 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -1,5 +1,6 @@ use std::io; use std::sync::{Arc, RwLock, RwLockWriteGuard}; +use std::thread::panicking; use std::time::{Duration, Instant}; use console::Term; @@ -378,6 +379,10 @@ impl DrawState { term: &(impl TermLike + ?Sized), last_line_count: &mut usize, ) -> io::Result<()> { + if panicking() { + return Ok(()); + } + if !self.lines.is_empty() && self.move_cursor { term.move_cursor_up(*last_line_count)?; } else { From 4dfb8d4cb95a87d177935f22802537c5f478d7c0 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 20 Mar 2022 20:36:45 -0700 Subject: [PATCH 3/3] Make sure with_style() and set_style() behave the same In #395, we made it so that message and prefix are carried over when calling ProgressBar::with_style(), but this did not change ProgressBar::set_style(). Reuse the same logic by having with_style() call set_style(), so that these cannot fall out of sync again. --- src/progress_bar.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/progress_bar.rs b/src/progress_bar.rs index 9b8dfd82..9652eae4 100644 --- a/src/progress_bar.rs +++ b/src/progress_bar.rs @@ -60,13 +60,8 @@ impl ProgressBar { } /// A convenience builder-like function for a progress bar with a given style - pub fn with_style(self, mut style: ProgressStyle) -> ProgressBar { - let mut state = self.state(); - mem::swap(&mut state.style.message, &mut style.message); - mem::swap(&mut state.style.prefix, &mut style.prefix); - state.style = style; - drop(state); - + pub fn with_style(self, style: ProgressStyle) -> ProgressBar { + self.set_style(style); self } @@ -122,8 +117,11 @@ impl ProgressBar { /// Overrides the stored style /// /// This does not redraw the bar. Call [`ProgressBar::tick()`] to force it. - pub fn set_style(&self, style: ProgressStyle) { - self.state().style = style; + pub fn set_style(&self, mut style: ProgressStyle) { + let mut state = self.state(); + mem::swap(&mut state.style.message, &mut style.message); + mem::swap(&mut state.style.prefix, &mut style.prefix); + state.style = style; } /// Spawns a background thread to tick the progress bar