refactor: improve types to better convey intent

crates/pop-cli/src/commands/up/parachain.rs

@@ -2,9 +2,11 @@
 
 use crate::style::{style, Theme};
 use clap::Args;
-use cliclack::{clear_screen, confirm, intro, log, multi_progress, outro, outro_cancel, set_theme};
+use cliclack::{
+	clear_screen, confirm, intro, log, multi_progress, outro, outro_cancel, set_theme, ProgressBar,
+};
 use console::{Emoji, Style};
-use pop_parachains::{NetworkNode, Zombienet};
+use pop_parachains::{NetworkNode, Status, Zombienet};
 use std::time::Duration;
 use tokio::time::sleep;
 
@@ -61,25 +63,24 @@ impl ZombienetCommand {
 			// Source binaries
 			for binary in missing {
 				let multi = multi_progress(format!("📦 Sourcing {}...", binary.name));
-				let spinner = multi.add(cliclack::spinner());
+				let progress = multi.add(cliclack::spinner());
+				let progress_reporter = ProgressReporter(&progress);
 				for attempt in (0..=1).rev() {
-					if let Err(e) =
-						binary.source(&cache, &|status| spinner.start(format(status))).await
-					{
+					if let Err(e) = binary.source(&cache, progress_reporter).await {
 						match attempt {
 							0 => {
-								spinner.error(format!("🚫 Sourcing failed: {e}"));
+								progress.error(format!("🚫 Sourcing failed: {e}"));
 								multi.stop();
 								return Ok(());
 							},
 							_ => {
-								spinner.error("🚫 Sourcing attempt failed, retrying...");
+								progress.error("🚫 Sourcing attempt failed, retrying...");
 								sleep(Duration::from_secs(1)).await;
 							},
 						}
 					}
 				}
-				spinner.stop(format!("✅ Sourcing {} complete.", binary.name));
+				progress.stop(format!("✅ Sourcing {} complete.", binary.name));
 				multi.stop();
 			}
 		}
@@ -149,6 +150,12 @@ impl ZombienetCommand {
 	}
 }
 
-fn format(status: &str) -> String {
-	status.replace("   Compiling", "Compiling")
+/// Reports any observed status updates to a progress bar.
+#[derive(Copy, Clone)]
+struct ProgressReporter<'a>(&'a ProgressBar);
+
+impl Status for ProgressReporter<'_> {
+	fn update(&self, status: &str) {
+		self.0.start(status.replace("   Compiling", "Compiling"))
+	}
 }

crates/pop-parachains/src/lib.rs

@@ -12,7 +12,7 @@ pub use build::build_parachain;
 pub use new_pallet::{create_pallet_template, TemplatePalletConfig};
 pub use new_parachain::instantiate_template_dir;
 pub use templates::{Config, Provider, Template};
-pub use up::Zombienet;
+pub use up::{Status, Zombienet};
 pub use utils::git::{Git, GitHub, Release};
 pub use utils::pallet_helpers::resolve_pallet_path;
 // External exports

crates/pop-parachains/src/up.rs

@@ -398,10 +398,10 @@ impl Binary {
 	/// # Arguments
 	///
 	/// * `cache` - path to the local cache
-	/// * `callback` - a callback used to report status updates
-	pub async fn source(&self, cache: &PathBuf, callback: &impl Fn(&str)) -> Result<(), Error> {
+	/// * `status` - used to observe status updates
+	pub async fn source(&self, cache: &PathBuf, status: impl Status) -> Result<(), Error> {
 		for source in &self.sources {
-			source.process(cache, callback).await?;
+			source.process(cache, status).await?;
 		}
 		Ok(())
 	}
@@ -435,7 +435,7 @@ impl Source {
 		path: &Path,
 		package: &str,
 		names: impl Iterator<Item = (&'b String, PathBuf)>,
-		callback: impl Fn(&str),
+		status: impl Status,
 	) -> Result<(), Error> {
 		// Build binaries and then copy to cache and target
 		let reader = cmd("cargo", vec!["build", "--release", "-p", package])
@@ -444,7 +444,7 @@ impl Source {
 			.reader()?;
 		let mut output = std::io::BufReader::new(reader).lines();
 		while let Some(Ok(line)) = output.next() {
-			callback(&line);
+			status.update(&line);
 		}
 		for (name, dest) in names {
 			copy(path.join(format!("target/release/{name}")), dest)?;
@@ -470,11 +470,11 @@ impl Source {
 	/// # Arguments
 	///
 	/// * `cache` - path to the local cache
-	/// * `callback` - a callback used to report status updates
+	/// * `status` - used to observe status updates
 	pub async fn process(
 		&self,
 		cache: &Path,
-		callback: impl Fn(&str),
+		status: impl Status,
 	) -> Result<Option<Vec<PathBuf>>, Error> {
 		// Download or clone and build from source
 		match self {
@@ -486,7 +486,7 @@ impl Source {
 				}
 
 				// Download required version of binaries
-				callback(&format!("Downloading from {url}..."));
+				status.update(&format!("Downloading from {url}..."));
 				Self::download(&url, &cache.join(&versioned_name)).await?;
 				Ok(None)
 			},
@@ -506,7 +506,7 @@ impl Source {
 
 				// Clone repository into working directory
 				if !working_dir.exists() {
-					callback(&format!("Cloning {url}..."));
+					status.update(&format!("Cloning {url}..."));
 					if let Err(e) = Git::clone(url, working_dir, branch.as_deref()) {
 						if working_dir.exists() {
 							// Preserve original error
@@ -522,7 +522,7 @@ impl Source {
 					versioned_names
 						.iter()
 						.map(|(binary, versioned)| (*binary, cache.join(versioned))),
-					callback,
+					status,
 				)
 				.await
 				{
@@ -562,6 +562,17 @@ impl Source {
 	}
 }
 
+/// Trait for observing status updates.
+pub trait Status: Copy {
+	/// Update the observer with the provided `status`.
+	fn update(&self, status: &str);
+}
+
+impl Status for () {
+	// no-op: status updates are ignored
+	fn update(&self, _: &str) {}
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -837,7 +848,7 @@ mod tests {
 		.await?;
 		let missing_binaries = zombienet.missing_binaries();
 		for binary in missing_binaries {
-			binary.source(&cache, &noop).await?;
+			binary.source(&cache, ()).await?;
 		}
 
 		let spawn = zombienet.spawn().await;
@@ -876,7 +887,7 @@ mod tests {
 			version: TESTING_POLKADOT_VERSION.to_string(),
 			url: "https://github.com/paritytech/polkadot-sdk/releases/download/polkadot-v1.7.0/polkadot".to_string()
 		};
-		let result = source.process(&cache, noop).await;
+		let result = source.process(&cache, ()).await;
 		assert!(result.is_ok());
 		assert!(temp_dir.path().join(POLKADOT_BINARY).exists());
 
@@ -902,7 +913,7 @@ mod tests {
 			version: Some(version),
 		};
 
-		let result = source.process(&cache, noop).await;
+		let result = source.process(&cache, ()).await;
 		assert!(result.is_ok());
 		assert!(temp_dir.path().join(POLKADOT_BINARY).exists());
 
@@ -965,7 +976,4 @@ mod tests {
 		)?;
 		Ok(file_path)
 	}
-
-	// no operation
-	fn noop(_: &str) {}
 }