Repository: j178/prefligit (language: python)

Issue:
Add progress bar for hook clone and install



Discussion / hints:

Fix patch:
diff --git a/src/cli/install.rs b/src/cli/install.rs
index 75a2c26..0481718 100644
--- a/src/cli/install.rs
+++ b/src/cli/install.rs
@@ -7,6 +7,7 @@ use indoc::indoc;
 use owo_colors::OwoColorize;
 use same_file::is_same_file;
 
+use crate::cli::reporter::{HookInitReporter, HookInstallReporter};
 use crate::cli::run;
 use crate::cli::{ExitStatus, HookType};
 use crate::fs::Simplified;
@@ -64,8 +65,10 @@ pub(crate) async fn install(
         let store = Store::from_settings()?.init()?;
         let _lock = store.lock_async().await?;
 
-        let hooks = project.init_hooks(&store, printer).await?;
-        run::install_hooks(&hooks, printer).await?;
+        let reporter = HookInitReporter::from(printer);
+        let hooks = project.init_hooks(&store, &reporter).await?;
+        let reporter = HookInstallReporter::from(printer);
+        run::install_hooks(&hooks, &reporter).await?;
     }
 
     Ok(ExitStatus::Success)
diff --git a/src/cli/mod.rs b/src/cli/mod.rs
index 8a6ed47..de20325 100644
--- a/src/cli/mod.rs
+++ b/src/cli/mod.rs
@@ -9,6 +9,7 @@ use crate::config::{HookType, Stage};
 mod clean;
 mod hook_impl;
 mod install;
+mod reporter;
 mod run;
 mod sample_config;
 mod self_update;
diff --git a/src/cli/reporter.rs b/src/cli/reporter.rs
new file mode 100644
index 0000000..e0a6095
--- /dev/null
+++ b/src/cli/reporter.rs
@@ -0,0 +1,167 @@
+use std::collections::HashMap;
+use std::sync::{Arc, Mutex};
+use std::time::Duration;
+
+use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
+use owo_colors::OwoColorize;
+
+use crate::hook;
+use crate::hook::Hook;
+use crate::printer::Printer;
+
+#[derive(Default, Debug)]
+struct BarState {
+    /// A map of progress bars, by ID.
+    bars: HashMap<usize, ProgressBar>,
+    /// A monotonic counter for bar IDs.
+    id: usize,
+}
+
+impl BarState {
+    /// Returns a unique ID for a new progress bar.
+    fn id(&mut self) -> usize {
+        self.id += 1;
+        self.id
+    }
+}
+
+struct SubProgress {
+    state: Arc<Mutex<BarState>>,
+    multi: MultiProgress,
+}
+
+pub(crate) struct HookInitReporter {
+    printer: Printer,
+    root: ProgressBar,
+    sub: SubProgress,
+}
+
+impl HookInitReporter {
+    pub(crate) fn new(root: ProgressBar, multi_progress: MultiProgress, printer: Printer) -> Self {
+        Self {
+            printer,
+            root,
+            sub: SubProgress {
+                state: Arc::default(),
+                multi: multi_progress,
+            },
+        }
+    }
+}
+
+impl From<Printer> for HookInitReporter {
+    fn from(printer: Printer) -> Self {
+        let multi = MultiProgress::with_draw_target(printer.target());
+        let root = multi.add(ProgressBar::with_draw_target(None, printer.target()));
+        root.enable_steady_tick(Duration::from_millis(200));
+        root.set_style(
+            ProgressStyle::with_template("{spinner:.white} {msg:.dim}")
+                .unwrap()
+                .tick_strings(&["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]),
+        );
+        root.set_message("Initializing hooks...");
+        Self::new(root, multi, printer)
+    }
+}
+
+impl hook::HookInitReporter for HookInitReporter {
+    fn on_repo_clone_start(&self, repo: &str) -> usize {
+        let mut state = self.sub.state.lock().unwrap();
+        let id = state.id();
+
+        let progress = self.sub.multi.insert_before(
+            &self.root,
+            ProgressBar::with_draw_target(None, self.printer.target()),
+        );
+
+        progress.set_style(ProgressStyle::with_template("{wide_msg}").unwrap());
+        progress.set_message(format!("{} {}", "Cloning".bold().cyan(), repo.dimmed(),));
+
+        state.bars.insert(id, progress);
+        id
+    }
+
+    fn on_repo_clone_complete(&self, id: usize) {
+        let progress = {
+            let mut state = self.sub.state.lock().unwrap();
+            state.bars.remove(&id).unwrap()
+        };
+
+        self.root.inc(1);
+        progress.finish_and_clear();
+    }
+
+    fn on_complete(&self) {
+        self.root.set_message("");
+        self.root.finish_and_clear();
+    }
+}
+
+pub struct HookInstallReporter {
+    printer: Printer,
+    root: ProgressBar,
+    sub: SubProgress,
+}
+
+impl HookInstallReporter {
+    pub fn new(root: ProgressBar, multi_progress: MultiProgress, printer: Printer) -> Self {
+        Self {
+            printer,
+            root,
+            sub: SubProgress {
+                state: Arc::default(),
+                multi: multi_progress,
+            },
+        }
+    }
+
+    pub fn on_install_start(&self, hook: &Hook) -> usize {
+        let mut state = self.sub.state.lock().unwrap();
+        let id = state.id();
+
+        let progress = self.sub.multi.insert_before(
+            &self.root,
+            ProgressBar::with_draw_target(None, self.printer.target()),
+        );
+
+        progress.set_style(ProgressStyle::with_template("{wide_msg}").unwrap());
+        progress.set_message(format!(
+            "{} {}",
+            "Installing".bold().cyan(),
+            hook.id.dimmed(),
+        ));
+
+        state.bars.insert(id, progress);
+        id
+    }
+
+    pub fn on_install_complete(&self, id: usize) {
+        let progress = {
+            let mut state = self.sub.state.lock().unwrap();
+            state.bars.remove(&id).unwrap()
+        };
+
+        self.root.inc(1);
+        progress.finish_and_clear();
+    }
+
+    pub fn on_complete(&self) {
+        self.root.set_message("");
+        self.root.finish_and_clear();
+    }
+}
+
+impl From<Printer> for HookInstallReporter {
+    fn from(printer: Printer) -> Self {
+        let multi = MultiProgress::with_draw_target(printer.target());
+        let root = multi.add(ProgressBar::with_draw_target(None, printer.target()));
+        root.enable_steady_tick(Duration::from_millis(200));
+        root.set_style(
+            ProgressStyle::with_template("{spinner:.white} {msg:.dim}")
+                .unwrap()
+                .tick_strings(&["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]),
+        );
+        root.set_message("Installing hooks...");
+        Self::new(root, multi, printer)
+    }
+}
diff --git a/src/cli/run.rs b/src/cli/run.rs
index 32f4ba8..9ba0bf3 100644
--- a/src/cli/run.rs
+++ b/src/cli/run.rs
@@ -10,6 +10,7 @@ use owo_colors::OwoColorize;
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
 use tracing::{debug, trace};
 
+use crate::cli::reporter::{HookInitReporter, HookInstallReporter};
 use crate::cli::{ExitStatus, RunExtraArgs};
 use crate::config::Stage;
 use crate::fs::{normalize_path, Simplified};
@@ -67,8 +68,10 @@ pub(crate) async fn run(
     let mut project = Project::new(config_file)?;
     let store = Store::from_settings()?.init()?;
 
+    let reporter = HookInitReporter::from(printer);
+
     let lock = store.lock_async().await?;
-    let hooks = project.init_hooks(&store, printer).await?;
+    let hooks = project.init_hooks(&store, &reporter).await?;
 
     let hooks: Vec<_> = hooks
         .into_iter()
@@ -117,7 +120,8 @@ pub(crate) async fn run(
         "Hooks going to run: {:?}",
         to_run.iter().map(|h| &h.id).collect::<Vec<_>>()
     );
-    install_hooks(&to_run, printer).await?;
+    let reporter = HookInstallReporter::from(printer);
+    install_hooks(&to_run, &reporter).await?;
     drop(lock);
 
     // Clear any unstaged changes from the git working directory.
@@ -302,12 +306,7 @@ async fn all_filenames(
     Ok(files)
 }
 
-async fn install_hook(hook: &Hook, env_dir: PathBuf, printer: Printer) -> Result<()> {
-    writeln!(
-        printer.stdout(),
-        "Installing environment for {}",
-        hook.repo(),
-    )?;
+async fn install_hook(hook: &Hook, env_dir: PathBuf) -> Result<()> {
     debug!(%hook, target = %env_dir.display(), "Install environment");
 
     if env_dir.try_exists()? {
@@ -324,8 +323,7 @@ async fn install_hook(hook: &Hook, env_dir: PathBuf, printer: Printer) -> Result
     Ok(())
 }
 
-// TODO: progress bar
-pub async fn install_hooks(hooks: &[Hook], printer: Printer) -> Result<()> {
+pub async fn install_hooks(hooks: &[Hook], reporter: &HookInstallReporter) -> Result<()> {
     let to_install = hooks
         .iter()
         .filter(|&hook| !hook.installed())
@@ -334,12 +332,19 @@ pub async fn install_hooks(hooks: &[Hook], printer: Printer) -> Result<()> {
     let mut tasks = FuturesUnordered::new();
     for hook in to_install {
         if let Some(env_dir) = hook.environment_dir() {
-            tasks.push(async move { install_hook(hook, env_dir, printer).await });
+            tasks.push(async move {
+                let progress = reporter.on_install_start(hook);
+                let result = install_hook(hook, env_dir).await;
+                (result, progress)
+            });
         }
     }
-    while let Some(result) = tasks.next().await {
+    while let Some((result, progress)) = tasks.next().await {
+        reporter.on_install_complete(progress);
         result?;
     }
 
+    reporter.on_complete();
+
     Ok(())
 }
diff --git a/src/config.rs b/src/config.rs
index 4275680..3442e90 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -381,18 +381,36 @@ pub struct ConfigRemoteRepo {
     pub hooks: Vec<ConfigRemoteHook>,
 }
 
+impl Display for ConfigRemoteRepo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}@{}", self.repo, self.rev)
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct ConfigLocalRepo {
     pub repo: String,
     pub hooks: Vec<ConfigLocalHook>,
 }
 
+impl Display for ConfigLocalRepo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("local")
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct ConfigMetaRepo {
     pub repo: String,
     pub hooks: Vec<ConfigMetaHook>,
 }
 
+impl Display for ConfigMetaRepo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("meta")
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum ConfigRepo {
     Remote(ConfigRemoteRepo),
diff --git a/src/hook.rs b/src/hook.rs
index 500b8c0..894a0f9 100644
--- a/src/hook.rs
+++ b/src/hook.rs
@@ -17,7 +17,6 @@ use crate::config::{
 };
 use crate::fs::{Simplified, CWD};
 use crate::languages::DEFAULT_VERSION;
-use crate::printer::Printer;
 use crate::store::Store;
 use crate::warn_user;
 
@@ -151,17 +150,22 @@ impl Project {
         &self.config_path
     }
 
-    async fn init_repos(&mut self, store: &Store, printer: Printer) -> Result<(), Error> {
+    async fn init_repos(
+        &mut self,
+        store: &Store,
+        reporter: &dyn HookInitReporter,
+    ) -> Result<(), Error> {
+        // TODO: unique repos
         let mut repos = Vec::with_capacity(self.config.repos.len());
 
-        // TODO: progress bar
         let mut tasks = FuturesUnordered::new();
         for (idx, repo) in self.config.repos.iter().enumerate() {
             match repo {
                 ConfigRepo::Remote(repo) => {
                     tasks.push(async move {
-                        let path = store.prepare_remote_repo(repo, &[], printer).await;
-                        (idx, path)
+                        let progress = reporter.on_repo_clone_start(&format!("{repo}"));
+                        let path = store.prepare_remote_repo(repo, &[]).await;
+                        (idx, path, progress)
                     });
                 }
                 ConfigRepo::Local(repo) => {
@@ -174,7 +178,7 @@ impl Project {
             }
         }
 
-        while let Some((idx, repo_path)) = tasks.next().await {
+        while let Some((idx, repo_path, progress)) = tasks.next().await {
             let repo_path = repo_path.map_err(Box::new)?;
             let ConfigRepo::Remote(repo_config) = &self.config.repos[idx] else {
                 unreachable!();
@@ -184,6 +188,7 @@ impl Project {
                 &repo_config.rev,
                 &repo_path.to_string_lossy(),
             )?;
+            reporter.on_repo_clone_complete(progress);
             repos.push((idx, Rc::new(repo)));
         }
 
@@ -197,13 +202,12 @@ impl Project {
     pub async fn init_hooks(
         &mut self,
         store: &Store,
-        printer: Printer,
+        reporter: &dyn HookInitReporter,
     ) -> Result<Vec<Hook>, Error> {
-        self.init_repos(store, printer).await?;
+        self.init_repos(store, reporter).await?;
 
         let mut hooks = Vec::new();
 
-        // TODO: progress bar
         for (repo_config, repo) in zip_eq(self.config.repos.iter(), self.repos.iter()) {
             match repo_config {
                 ConfigRepo::Remote(repo_config) => {
@@ -229,11 +233,7 @@ impl Project {
                         } else {
                             // Prepare hooks with `additional_dependencies` (they need separate environments).
                             let path = store
-                                .prepare_remote_repo(
-                                    repo_config,
-                                    &hook.additional_dependencies,
-                                    printer,
-                                )
+                                .prepare_remote_repo(repo_config, &hook.additional_dependencies)
                                 .await
                                 .map_err(Box::new)?;
 
@@ -253,7 +253,7 @@ impl Project {
                         // If the hook doesn't need an environment, don't do any preparation.
                         if hook.language.environment_dir().is_some() {
                             let path = store
-                                .prepare_local_repo(&hook, &hook.additional_dependencies, printer)
+                                .prepare_local_repo(&hook, &hook.additional_dependencies)
                                 .map_err(Box::new)?;
 
                             hook = hook.with_path(path);
@@ -271,10 +271,18 @@ impl Project {
             }
         }
 
+        reporter.on_complete();
+
         Ok(hooks)
     }
 }
 
+pub trait HookInitReporter {
+    fn on_repo_clone_start(&self, repo: &str) -> usize;
+    fn on_repo_clone_complete(&self, id: usize);
+    fn on_complete(&self);
+}
+
 struct HookBuilder {
     repo: Rc<Repo>,
     config: ManifestHook,
diff --git a/src/store.rs b/src/store.rs
index ffd442f..becf173 100644
--- a/src/store.rs
+++ b/src/store.rs
@@ -1,4 +1,3 @@
-use std::fmt::Write;
 use std::path::{Path, PathBuf};
 
 use anyhow::Result;
@@ -10,7 +9,6 @@ use crate::config::ConfigRemoteRepo;
 use crate::fs::{copy_dir_all, LockedFile};
 use crate::git::clone_repo;
 use crate::hook::{Hook, Repo};
-use crate::printer::Printer;
 
 #[derive(Debug, Error)]
 pub enum Error {
@@ -177,12 +175,7 @@ impl Store {
     /// Prepare a local repo for a local hook.
     /// All local hooks with same additional dependencies, e.g. no dependencies,
     /// are stored in the same directory (even they use different language).
-    pub fn prepare_local_repo(
-        &self,
-        hook: &Hook,
-        deps: &[String],
-        printer: Printer,
-    ) -> Result<PathBuf, Error> {
+    pub fn prepare_local_repo(&self, hook: &Hook, deps: &[String]) -> Result<PathBuf, Error> {
         const LOCAL_NAME: &str = "local";
         const LOCAL_REV: &str = "1";
 
@@ -199,7 +192,6 @@ impl Store {
                 .tempdir_in(&self.path)?;
 
             let path = temp.path().to_string_lossy().to_string();
-            writeln!(printer.stdout(), "Preparing local repo {}", hook.id)?;
             debug!(hook = hook.id, path, "Preparing local repo");
             make_local_repo(LOCAL_NAME, temp.path())?;
             self.insert_repo(LOCAL_NAME, LOCAL_REV, &path, deps)?;
@@ -214,7 +206,6 @@ impl Store {
         &self,
         repo_config: &ConfigRemoteRepo,
         deps: &[String],
-        printer: Printer,
     ) -> Result<PathBuf, Error> {
         if let Some((_, _, path)) = self.get_repo(
             repo_config.repo.as_str(),
@@ -232,12 +223,6 @@ impl Store {
         let path = temp.path().to_string_lossy().to_string();
 
         if deps.is_empty() {
-            writeln!(
-                printer.stdout(),
-                "Cloning {}@{}",
-                repo_config.repo,
-                repo_config.rev
-            )?;
             debug!(
                 target = path,
                 repo = format!("{}@{}", repo_config.repo, repo_config.rev),
@@ -251,13 +236,6 @@ impl Store {
             let (_, _, base_repo_path) = self
                 .get_repo(repo_config.repo.as_str(), repo_config.rev.as_str(), &[])?
                 .expect("base repo should be cloned before");
-            writeln!(
-                printer.stdout(),
-                "Preparing {}@{} with dependencies {}",
-                repo_config.repo,
-                repo_config.rev,
-                deps.join(","),
-            )?;
             debug!(
                 source = base_repo_path,
                 target = path,


Test patch:
diff --git a/tests/languages/docker.rs b/tests/languages/docker.rs
index b652f74..a76121c 100644
--- a/tests/languages/docker.rs
+++ b/tests/languages/docker.rs
@@ -23,8 +23,6 @@ fn docker() {
     success: true
     exit_code: 0
     ----- stdout -----
-    Cloning https://github.com/j178/pre-commit-docker-hooks@master
-    Installing environment for https://github.com/j178/pre-commit-docker-hooks@master
     Hello World..............................................................Passed
     - hook id: hello-world
     - duration: [TIME]
diff --git a/tests/run.rs b/tests/run.rs
index 61bd2e4..ba2a99e 100644
--- a/tests/run.rs
+++ b/tests/run.rs
@@ -37,8 +37,6 @@ fn run_basic() -> Result<()> {
     success: false
     exit_code: 1
     ----- stdout -----
-    Cloning https://github.com/pre-commit/pre-commit-hooks@v5.0.0
-    Installing environment for https://github.com/pre-commit/pre-commit-hooks@v5.0.0
     trim trailing whitespace.................................................Failed
     - hook id: trailing-whitespace
     - exit code: 1
@@ -132,8 +130,6 @@ fn local_need_install() {
     success: true
     exit_code: 0
     ----- stdout -----
-    Preparing local repo local
-    Installing environment for local
     local....................................................................Passed
 
     ----- stderr -----