-
-
Notifications
You must be signed in to change notification settings - Fork 235
fix: Don't use hidden progress bar #2830
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
9f529c0
a721703
e78c462
32c2ac7
287646c
6f033ba
a94a938
f1deb83
0794f30
adbebfa
496ce41
12795b6
912bb2d
cea6740
0f75ca1
43e12ce
fddd245
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,76 +1,141 @@ | ||||||||||
use parking_lot::RwLock; | ||||||||||
use std::env; | ||||||||||
use std::ops::Deref; | ||||||||||
use std::io::IsTerminal as _; | ||||||||||
use std::sync::Arc; | ||||||||||
use std::time::Instant; | ||||||||||
|
||||||||||
use crate::utils::logging; | ||||||||||
|
||||||||||
pub use indicatif::ProgressStyle; | ||||||||||
|
||||||||||
pub fn is_progress_bar_visible() -> bool { | ||||||||||
env::var("SENTRY_NO_PROGRESS_BAR") != Ok("1".into()) | ||||||||||
/// By default, use the progress bar when we can detect we're running in a terminal. | ||||||||||
/// `SENTRY_NO_PROGRESS_BAR` takes precedence and can be used to disable the progress bar | ||||||||||
/// regardless of whether or not we're in a terminal. | ||||||||||
pub fn use_progress_bar() -> bool { | ||||||||||
if env::var("SENTRY_NO_PROGRESS_BAR") == Ok("1".into()) { | ||||||||||
return false; | ||||||||||
} | ||||||||||
std::io::stdout().is_terminal() && std::io::stderr().is_terminal() | ||||||||||
} | ||||||||||
|
||||||||||
/// Wrapper that optionally holds a progress bar. | ||||||||||
/// If there's a progress bar, forward calls to it. | ||||||||||
/// Otherwise, forward messages to `log` calls. | ||||||||||
pub struct ProgressBar { | ||||||||||
inner: Arc<indicatif::ProgressBar>, | ||||||||||
inner: Option<Arc<indicatif::ProgressBar>>, | ||||||||||
start: Instant, | ||||||||||
prefix: Option<String>, | ||||||||||
} | ||||||||||
|
||||||||||
impl ProgressBar { | ||||||||||
pub fn new(len: usize) -> Self { | ||||||||||
if is_progress_bar_visible() { | ||||||||||
indicatif::ProgressBar::new(len as u64).into() | ||||||||||
if use_progress_bar() { | ||||||||||
Self::from_indicatif(indicatif::ProgressBar::new(len as u64)) | ||||||||||
} else { | ||||||||||
Self::hidden() | ||||||||||
Self::no_progress_bar() | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn new_spinner() -> Self { | ||||||||||
if is_progress_bar_visible() { | ||||||||||
indicatif::ProgressBar::new_spinner().into() | ||||||||||
if use_progress_bar() { | ||||||||||
Self::from_indicatif(indicatif::ProgressBar::new_spinner()) | ||||||||||
} else { | ||||||||||
Self::hidden() | ||||||||||
Self::no_progress_bar() | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn hidden() -> Self { | ||||||||||
indicatif::ProgressBar::hidden().into() | ||||||||||
fn from_indicatif(pb: indicatif::ProgressBar) -> Self { | ||||||||||
let inner = Arc::new(pb); | ||||||||||
logging::set_progress_bar(Some(Arc::downgrade(&inner))); | ||||||||||
ProgressBar { | ||||||||||
inner: Some(inner), | ||||||||||
start: Instant::now(), | ||||||||||
prefix: None, | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
fn no_progress_bar() -> Self { | ||||||||||
ProgressBar { | ||||||||||
inner: None, | ||||||||||
start: Instant::now(), | ||||||||||
prefix: None, | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn finish_with_duration(&self, op: &str) { | ||||||||||
let dur = self.start.elapsed(); | ||||||||||
// We could use `dur.as_secs_f64()`, but its unnecessarily precise (micros). Millis are enough for our purpose. | ||||||||||
let msg = format!("{op} completed in {}s", dur.as_millis() as f64 / 1000.0); | ||||||||||
let progress_style = ProgressStyle::default_bar().template("{prefix:.dim} {msg}"); | ||||||||||
self.inner.set_style(progress_style); | ||||||||||
self.inner.set_prefix(">"); | ||||||||||
self.inner.finish_with_message(&msg); | ||||||||||
logging::set_progress_bar(None); | ||||||||||
|
||||||||||
if let Some(inner) = &self.inner { | ||||||||||
let progress_style = ProgressStyle::default_bar().template("{prefix:.dim} {msg}"); | ||||||||||
inner.set_style(progress_style); | ||||||||||
inner.set_prefix(">"); | ||||||||||
inner.finish_with_message(&msg); | ||||||||||
logging::set_progress_bar(None); | ||||||||||
} else { | ||||||||||
log::info!("> {msg}"); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn finish_and_clear(&self) { | ||||||||||
self.inner.finish_and_clear(); | ||||||||||
logging::set_progress_bar(None); | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.finish_and_clear(); | ||||||||||
logging::set_progress_bar(None); | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
impl From<indicatif::ProgressBar> for ProgressBar { | ||||||||||
fn from(pb: indicatif::ProgressBar) -> Self { | ||||||||||
let inner = Arc::new(pb); | ||||||||||
logging::set_progress_bar(Some(Arc::downgrade(&inner))); | ||||||||||
ProgressBar { | ||||||||||
inner, | ||||||||||
start: Instant::now(), | ||||||||||
pub fn set_style(&self, style: ProgressStyle) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.set_style(style); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn set_message<S: AsRef<str>>(&self, msg: S) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.set_message(msg.as_ref()); | ||||||||||
} else { | ||||||||||
log::debug!( | ||||||||||
"{}{}", | ||||||||||
self.prefix | ||||||||||
.as_deref() | ||||||||||
.map(|s| format!("{s} ")) | ||||||||||
.unwrap_or_else(|| String::new()), | ||||||||||
msg.as_ref() | ||||||||||
); | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
impl Deref for ProgressBar { | ||||||||||
type Target = indicatif::ProgressBar; | ||||||||||
pub fn tick(&self) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.tick(); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
fn deref(&self) -> &Self::Target { | ||||||||||
&self.inner | ||||||||||
pub fn set_prefix<S: AsRef<str>>(&mut self, prefix: S) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.set_prefix(prefix.as_ref()); | ||||||||||
} else { | ||||||||||
self.prefix = Some(prefix.as_ref().to_owned()); | ||||||||||
} | ||||||||||
Comment on lines
+118
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going along with my previous suggestion, getting rid of logging in the no progress bar case lets us simplify here. We can probably even remove the
Suggested change
|
||||||||||
} | ||||||||||
|
||||||||||
pub fn set_position(&self, pos: u64) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.set_position(pos); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn inc(&self, delta: u64) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.inc(delta); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
pub fn enable_steady_tick(&self, interval: u64) { | ||||||||||
if let Some(inner) = &self.inner { | ||||||||||
inner.enable_steady_tick(interval); | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to become
mut
to handleset_prefix