fix: improve message displayed in transfer progress bars. (#23)

peterhuene · web-flow · commit dd11ab991e5a · 2025-10-10T10:45:21.000-05:00
* fix: improve message displayed in transfer progress bars. This commit improves the message displayed in transfer progress bars to show the source file for uploads and source URL for downloads. Additionally, this shortens the progress bar so that the message isn't too far to the right given the rest of the information displayed. Fixes #22. * chore: update CHANGELOG.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+#### Fixed
+
+* Improved progress bar message ([#23](https://github.com/stjude-rust-labs/cloud-copy/pull/23)).
+
 ## 0.3.0 - 09-15-2025
 
 #### Added
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -65,6 +65,7 @@ walkdir = "2.5.0"
 
 [dev-dependencies]
 anyhow = { version = "1.0.99" }
+pretty_assertions = "1.4.1"
 
 [lints.rust]
 missing_docs = "warn"
diff --git a/src/cli.rs b/src/cli.rs
@@ -12,7 +12,9 @@ use tracing::warn_span;
 use tracing_indicatif::span_ext::IndicatifSpanExt;
 use tracing_indicatif::style::ProgressStyle;
 
+use crate::Location;
 use crate::TransferEvent;
+use crate::UrlExt;
 
 /// Extension methods for [`TimeDelta`].
 #[cfg_attr(docsrs, doc(cfg(feature = "cli")))]
@@ -135,14 +137,14 @@ pub async fn handle_events(
             _ = cancel.cancelled() => break,
             event = events.recv() => match event {
                 Ok(event) if indeterminate.is_none() => match event {
-                    TransferEvent::TransferStarted { id, path, size, .. } => {
+                    TransferEvent::TransferStarted { id, source, destination, size, .. } => {
                         let bar = warn_span!("progress");
 
                         let style = match size {
                             Some(size) => {
                                 bar.pb_set_length(size);
                                 ProgressStyle::with_template(
-                                    "[{elapsed_precise:.cyan/blue}] {bar:40.cyan/blue} \
+                                    "[{elapsed_precise:.cyan/blue}] {bar:20.cyan/blue} \
                                     {bytes:.cyan/blue} / {total_bytes:.cyan/blue} \
                                     ({bytes_per_sec:.cyan/blue}) [ETA {eta_precise:.cyan/blue}]: \
                                     {msg}",
@@ -156,8 +158,17 @@ pub async fn handle_events(
                             .unwrap(),
                         };
 
+                        let message = match (&source, &destination) {
+                            // Use the source path for local file copies
+                            (Location::Path(path), Location::Path(_)) => format!("copying `{path}`", path = path.to_str().unwrap_or("<path not UTF-8>")),
+                            // Use the source path for uploads
+                            (Location::Path(path), _) => format!("uploading `{path}`", path = path.to_str().unwrap_or("<path not UTF-8>")),
+                            // Use the remote URL for downloads
+                            (Location::Url(url), _) => format!("downloading `{url}`", url = url.display())
+                        };
+
                         bar.pb_set_style(&style);
-                        bar.pb_set_message(path.to_str().unwrap_or("<path not UTF-8>"));
+                        bar.pb_set_message(&message);
                         bar.pb_start();
                         transfers.insert(
                             id,
diff --git a/src/lib.rs b/src/lib.rs
@@ -94,24 +94,24 @@ fn notify_retry(e: &Error, duration: Duration) {
 
 /// Represents either a local or remote location.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum Location<'a> {
+pub enum Location {
     /// The location is a local path.
-    Path(&'a Path),
+    Path(PathBuf),
     /// The location is a URL.
-    Url(Cow<'a, Url>),
+    Url(Url),
 }
 
-impl<'a> Location<'a> {
+impl Location {
     /// Constructs a new location from a string.
-    pub fn new(s: &'a str) -> Self {
+    pub fn new(s: &str) -> Self {
         match s.parse::<Url>() {
-            Ok(url) => Self::Url(Cow::Owned(url)),
-            Err(_) => Self::Path(Path::new(s)),
+            Ok(url) => Self::Url(url),
+            Err(_) => Self::Path(s.into()),
         }
     }
 }
 
-impl fmt::Display for Location<'_> {
+impl fmt::Display for Location {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::Path(path) => write!(f, "{path}", path = path.display()),
@@ -120,39 +120,45 @@ impl fmt::Display for Location<'_> {
     }
 }
 
-impl<'a> From<&'a str> for Location<'a> {
-    fn from(value: &'a str) -> Self {
+impl From<&str> for Location {
+    fn from(value: &str) -> Self {
         Self::new(value)
     }
 }
 
-impl<'a> From<&'a String> for Location<'a> {
-    fn from(value: &'a String) -> Self {
+impl From<&String> for Location {
+    fn from(value: &String) -> Self {
         Self::new(value)
     }
 }
 
-impl<'a> From<&'a Path> for Location<'a> {
-    fn from(value: &'a Path) -> Self {
-        Self::Path(value)
+impl From<&Path> for Location {
+    fn from(value: &Path) -> Self {
+        Self::Path(value.to_path_buf())
     }
 }
 
-impl<'a> From<&'a PathBuf> for Location<'a> {
-    fn from(value: &'a PathBuf) -> Self {
-        Self::Path(value.as_path())
+impl From<&PathBuf> for Location {
+    fn from(value: &PathBuf) -> Self {
+        Self::Path(value.clone())
+    }
+}
+
+impl From<PathBuf> for Location {
+    fn from(value: PathBuf) -> Self {
+        Self::Path(value)
     }
 }
 
-impl<'a> From<&'a Url> for Location<'a> {
-    fn from(value: &'a Url) -> Self {
-        Self::Url(Cow::Borrowed(value))
+impl From<&Url> for Location {
+    fn from(value: &Url) -> Self {
+        Self::Url(value.clone())
     }
 }
 
-impl From<Url> for Location<'_> {
+impl From<Url> for Location {
     fn from(value: Url) -> Self {
-        Self::Url(Cow::Owned(value))
+        Self::Url(value)
     }
 }
 
@@ -440,8 +446,10 @@ pub enum TransferEvent {
         ///
         /// This is a monotonic counter that is increased every transfer.
         id: u64,
-        /// The path of the file being transferred.
-        path: PathBuf,
+        /// The location of the source.
+        source: Location,
+        /// The location of the destination.
+        destination: Location,
         /// The number of blocks in the file.
         blocks: u64,
         /// The size of the file being transferred.
@@ -527,7 +535,8 @@ async fn copy_local(
         events
             .send(TransferEvent::TransferStarted {
                 id: ID,
-                path: destination.to_path_buf(),
+                source: Location::Path(source.to_path_buf()),
+                destination: Location::Path(destination.to_path_buf()),
                 blocks: 1,
                 size: Some(size),
             })
@@ -622,8 +631,8 @@ async fn copy_local(
 pub async fn copy(
     config: Config,
     client: HttpClient,
-    source: impl Into<Location<'_>>,
-    destination: impl Into<Location<'_>>,
+    source: impl Into<Location>,
+    destination: impl Into<Location>,
     cancel: CancellationToken,
     events: Option<broadcast::Sender<TransferEvent>>,
 ) -> Result<()> {
@@ -637,12 +646,12 @@ pub async fn copy(
             }
 
             // Two local locations, just perform a copy
-            Ok(copy_local(source, destination, cancel, events).await?)
+            Ok(copy_local(&source, &destination, cancel, events).await?)
         }
         (Location::Path(source), Location::Url(destination)) => {
             // Perform a copy if the the destination is a local path
             if let Some(destination) = destination.to_local_path()? {
-                return copy_local(source, &destination, cancel, events).await;
+                return copy_local(&source, &destination, cancel, events).await;
             }
 
             if AzureBlobStorageBackend::is_supported_url(&config, &destination) {
@@ -661,7 +670,7 @@ pub async fn copy(
                     FileTransfer::new(GoogleStorageBackend::new(config, client, events), cancel);
                 transfer.upload(source, destination.into_owned()).await
             } else {
-                Err(Error::UnsupportedUrl(destination.into_owned()))
+                Err(Error::UnsupportedUrl(destination))
             }
         }
         (Location::Url(source), Location::Path(destination)) => {
@@ -671,7 +680,7 @@ pub async fn copy(
 
             // Perform a copy if the the source is a local path
             if let Some(source) = source.to_local_path()? {
-                return copy_local(&source, destination, cancel, events).await;
+                return copy_local(&source, &destination, cancel, events).await;
             }
 
             if AzureBlobStorageBackend::is_supported_url(&config, &source) {
@@ -692,7 +701,7 @@ pub async fn copy(
             } else {
                 let transfer =
                     FileTransfer::new(GenericStorageBackend::new(config, client, events), cancel);
-                transfer.download(source.into_owned(), destination).await
+                transfer.download(source, destination).await
             }
         }
         (Location::Url(source), Location::Url(destination)) => {
diff --git a/src/transfer.rs b/src/transfer.rs
@@ -31,6 +31,7 @@ use url::Url;
 use walkdir::WalkDir;
 
 use crate::Error;
+use crate::Location;
 use crate::Result;
 use crate::TransferEvent;
 use crate::UrlExt;
@@ -124,6 +125,7 @@ where
             .get(header::CONTENT_LENGTH)
             .and_then(|v| v.to_str().ok()?.parse().ok());
 
+        let download_source = source.clone();
         let transfer = async {
             // Start by creating the destination's parent directory
             let parent = destination.parent().ok_or(Error::InvalidPath)?;
@@ -140,8 +142,15 @@ where
                 .into_temp_path();
 
             // Download the file with resumable retries
-            self.download_with_resume(id, source, &temp, content_length, etag, cancel.clone())
-                .await?;
+            self.download_with_resume(
+                id,
+                download_source,
+                &temp,
+                content_length,
+                etag,
+                cancel.clone(),
+            )
+            .await?;
 
             // Persist the temp file to the destination
             temp.persist_noclobber(destination)
@@ -155,7 +164,8 @@ where
             events
                 .send(TransferEvent::TransferStarted {
                     id,
-                    path: destination.to_path_buf(),
+                    source: Location::Url(source),
+                    destination: Location::Path(destination.to_path_buf()),
                     blocks: 1,
                     size: content_length,
                 })
@@ -669,7 +679,8 @@ where
             events
                 .send(TransferEvent::TransferStarted {
                     id,
-                    path: source.to_path_buf(),
+                    source: Location::Path(source.to_path_buf()),
+                    destination: Location::Url(destination),
                     blocks: num_blocks,
                     size: Some(file_size),
                 })
diff --git a/tests/copy.rs b/tests/copy.rs
