From bea1ef73e64ff7d258c23137b482c6c1e20db4e6 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 12 Jun 2023 21:44:43 +0900 Subject: [PATCH] ui: create separate io wrapper for progress output I'm going to redirect progress message to stderr, but the current Ui API makes it less clear whether the underlying streams of .write*()/.flush()/ .output_guard() are related or not. --- src/commands/git.rs | 5 ++--- src/progress.rs | 38 +++++++++++++++++------------------ src/ui.rs | 49 ++++++++++++++++++++++++++++++++------------- 3 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/commands/git.rs b/src/commands/git.rs index bc7f0ffaa..c1ea3694a 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -510,11 +510,10 @@ fn do_git_clone( fn with_remote_callbacks(ui: &mut Ui, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T) -> T { let mut ui = Mutex::new(ui); let mut callback = None; - if ui.get_mut().unwrap().use_progress_indicator() { + if let Some(mut output) = ui.get_mut().unwrap().progress_output() { let mut progress = Progress::new(Instant::now()); - let ui = &ui; callback = Some(move |x: &git::Progress| { - _ = progress.update(Instant::now(), x, *ui.lock().unwrap()); + _ = progress.update(Instant::now(), x, &mut output); }); } let mut callbacks = git::RemoteCallbacks::default(); diff --git a/src/progress.rs b/src/progress.rs index 6baaa62db..54e8c7226 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -8,7 +8,7 @@ use jujutsu_lib::git; use jujutsu_lib::repo_path::RepoPath; use crate::cleanup_guard::CleanupGuard; -use crate::ui::{OutputGuard, Ui}; +use crate::ui::{OutputGuard, ProgressOutput, Ui}; pub struct Progress { next_print: Instant, @@ -31,13 +31,13 @@ impl Progress { &mut self, now: Instant, progress: &git::Progress, - ui: &mut Ui, + output: &mut ProgressOutput, ) -> io::Result<()> { use std::fmt::Write as _; if progress.overall == 1.0 { - write!(ui, "\r{}", Clear(ClearType::CurrentLine))?; - ui.flush()?; + write!(output, "\r{}", Clear(ClearType::CurrentLine))?; + output.flush()?; return Ok(()); } @@ -48,11 +48,11 @@ impl Progress { return Ok(()); } if self.guard.is_none() { - let guard = ui.output_guard(crossterm::cursor::Show.to_string()); + let guard = output.output_guard(crossterm::cursor::Show.to_string()); let guard = CleanupGuard::new(move || { drop(guard); }); - _ = write!(ui, "{}", crossterm::cursor::Hide); + _ = write!(output, "{}", crossterm::cursor::Hide); self.guard = Some(guard); } self.next_print = now.min(self.next_print + Duration::from_secs(1) / UPDATE_HZ); @@ -70,7 +70,7 @@ impl Progress { write!(self.buffer, "at {scaled: >5.1} {prefix}B/s ").unwrap(); } - let bar_width = ui + let bar_width = output .term_width() .map(usize::from) .unwrap_or(0) @@ -79,8 +79,8 @@ impl Progress { draw_progress(progress.overall, &mut self.buffer, bar_width); self.buffer.push(']'); - write!(ui, "{}", self.buffer)?; - ui.flush()?; + write!(output, "{}", self.buffer)?; + output.flush()?; Ok(()) } } @@ -170,22 +170,20 @@ impl RateEstimateState { } } -pub fn snapshot_progress(ui: &mut Ui) -> Option { - struct State<'a> { +pub fn snapshot_progress(ui: &Ui) -> Option { + struct State { guard: Option, - ui: &'a mut Ui, + output: ProgressOutput, next_display_time: Instant, } - if !ui.use_progress_indicator() { - return None; - } + let output = ui.progress_output()?; // Don't clutter the output during fast operations. let next_display_time = Instant::now() + INITIAL_DELAY; let state = Mutex::new(State { guard: None, - ui, + output, next_display_time, }); @@ -202,22 +200,22 @@ pub fn snapshot_progress(ui: &mut Ui) -> Option { if state.guard.is_none() { state.guard = Some( state - .ui + .output .output_guard(format!("\r{}", Clear(ClearType::CurrentLine))), ); } - let line_width = state.ui.term_width().map(usize::from).unwrap_or(80); + let line_width = state.output.term_width().map(usize::from).unwrap_or(80); let path_width = line_width.saturating_sub(13); // Account for "Snapshotting " _ = write!( - state.ui, + state.output, "\r{}Snapshotting {:.*}", Clear(ClearType::CurrentLine), path_width, path.to_fs_path(Path::new("")).display() ); - _ = state.ui.flush(); + _ = state.output.flush(); }) } diff --git a/src/ui.rs b/src/ui.rs index cf7e8ae60..804b23ce6 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -194,6 +194,12 @@ impl Ui { } } + pub fn progress_output(&self) -> Option { + self.use_progress_indicator().then(|| ProgressOutput { + output: io::stdout(), + }) + } + pub fn write(&mut self, text: &str) -> io::Result<()> { let data = text.as_bytes(); match &mut self.output { @@ -280,20 +286,6 @@ impl Ui { pub fn term_width(&self) -> Option { term_width() } - - /// Construct a guard object which writes `data` when dropped. Useful for - /// restoring terminal state. - pub fn output_guard(&self, text: String) -> OutputGuard { - OutputGuard { - text, - output: match self.output { - UiOutput::Terminal { .. } => io::stdout(), - // TODO we don't actually need to write in this case, so it - // might be better to no-op - UiOutput::Paged { .. } => io::stdout(), - }, - } - } } enum UiOutput { @@ -322,6 +314,35 @@ impl UiOutput { } } +#[derive(Debug)] +pub struct ProgressOutput { + output: Stdout, +} + +impl ProgressOutput { + pub fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { + self.output.write_fmt(fmt) + } + + pub fn flush(&mut self) -> io::Result<()> { + self.output.flush() + } + + pub fn term_width(&self) -> Option { + // Terminal can be resized while progress is displayed, so don't cache it. + term_width() + } + + /// Construct a guard object which writes `text` when dropped. Useful for + /// restoring terminal state. + pub fn output_guard(&self, text: String) -> OutputGuard { + OutputGuard { + text, + output: io::stdout(), + } + } +} + pub struct OutputGuard { text: String, output: Stdout,