refactor: reduce cognitive complexity of create and list operations

src/uu/tar/src/operations/create.rs

@@ -56,42 +56,10 @@ pub fn create_archive(
         }
 
         if verbose {
-            let to_print = get_tree(path)?
-                .iter()
-                .map(|p| (p.is_dir(), p.display().to_string()))
-                .map(|(is_dir, path)| {
-                    if is_dir {
-                        format!("{}{}", path, path::MAIN_SEPARATOR)
-                    } else {
-                        path
-                    }
-                })
-                .collect::<Vec<_>>()
-                .join("\n");
-            writeln!(status_output, "{to_print}").map_err(TarError::Io)?;
+            print_verbose_tree(&mut status_output, path)?;
         }
 
-        // Normalize path if needed (so far, handles only absolute paths)
-        let normalized_name = if let Some(normalized) = normalize_path(path, allow_absolute) {
-            let original_components: Vec<Component> = path.components().collect();
-            let normalized_components: Vec<Component> = normalized.components().collect();
-            if original_components.len() > normalized_components.len() {
-                let removed: PathBuf = original_components
-                    [..original_components.len() - normalized_components.len()]
-                    .iter()
-                    .collect();
-                writeln!(
-                    std::io::stderr(),
-                    "tar: Removing leading `{}' from member names",
-                    removed.display()
-                )
-                .map_err(TarError::Io)?;
-            }
-
-            normalized
-        } else {
-            path.to_path_buf()
-        };
+        let normalized_name = get_normalized_path(path, allow_absolute)?;
 
         // If it's a directory, recursively add all contents
         if path.is_dir() {
@@ -125,6 +93,44 @@ pub fn create_archive(
     Ok(())
 }
 
+fn print_verbose_tree(status_output: &mut impl Write, path: &Path) -> UResult<()> {
+    let to_print = get_tree(path)?
+        .iter()
+        .map(|p| (p.is_dir(), p.display().to_string()))
+        .map(|(is_dir, path)| {
+            if is_dir {
+                format!("{}{}", path, path::MAIN_SEPARATOR)
+            } else {
+                path
+            }
+        })
+        .collect::<Vec<_>>()
+        .join("\n");
+    writeln!(status_output, "{to_print}").map_err(TarError::Io)?;
+    Ok(())
+}
+
+fn get_normalized_path(path: &Path, allow_absolute: bool) -> Result<PathBuf, TarError> {
+    if let Some(normalized) = normalize_path(path, allow_absolute) {
+        let original_components: Vec<Component> = path.components().collect();
+        let normalized_components: Vec<Component> = normalized.components().collect();
+        if original_components.len() > normalized_components.len() {
+            let removed: PathBuf = original_components
+                [..original_components.len() - normalized_components.len()]
+                .iter()
+                .collect();
+            writeln!(
+                std::io::stderr(),
+                "tar: Removing leading `{}' from member names",
+                removed.display()
+            )?;
+        }
+        Ok(normalized)
+    } else {
+        Ok(path.to_path_buf())
+    }
+}
+
 fn get_tree(path: &Path) -> Result<Vec<PathBuf>, std::io::Error> {
     let mut paths = Vec::new();
     let mut stack = VecDeque::new();

src/uu/tar/src/operations/list.rs

@@ -28,62 +28,10 @@ pub fn list_archive(
         let entry = entry_result.map_err(TarError::CannotReadEntry)?;
 
         if verbose {
-            // Collect all header fields into owned values before borrowing entry for the path,
-            // since both header() and path() require a borrow of entry.
-            let (mode, entry_type, owner, group, size, mtime) = {
-                let header = entry.header();
-                (
-                    header.mode().unwrap_or(0),
-                    header.entry_type(),
-                    header
-                        .username()
-                        .ok()
-                        .flatten()
-                        .unwrap_or_default()
-                        .to_owned(),
-                    header
-                        .groupname()
-                        .ok()
-                        .flatten()
-                        .unwrap_or_default()
-                        .to_owned(),
-                    header.size().unwrap_or(0),
-                    header.mtime().unwrap_or(0),
-                )
-            };
-
-            let path = entry.path().map_err(TarError::CannotReadEntryPath)?;
-
-            let type_char = match entry_type {
-                tar::EntryType::Directory => 'd',
-                tar::EntryType::Symlink => 'l',
-                tar::EntryType::Char => 'c',
-                tar::EntryType::Block => 'b',
-                tar::EntryType::Fifo => 'p',
-                _ => '-',
-            };
-            // Tar headers store the type separately from the mode bits, so we get the
-            // 9-character rwx string from uucore and prepend our own type character.
-            let perm_str = display_permissions_unix(mode, false);
-            let permissions = format!("{type_char}{perm_str}");
-
-            // TODO: GNU tar displays mtime in the user's local timezone; we
-            // currently format in UTC. Convert to local time for compatibility.
-            let dt: chrono::DateTime<Utc> = Utc
-                .timestamp_opt(mtime as i64, 0)
-                .single()
-                .unwrap_or_else(Utc::now);
-            let date_str = dt.format("%Y-%m-%d %H:%M");
-
-            writeln!(
-                out,
-                "{permissions} {owner}/{group} {size:>8} {date_str} {}",
-                path.display()
-            )
-            .map_err(TarError::Io)?;
+            let formatted = format_verbose_entry(&entry)?;
+            writeln!(out, "{formatted}").map_err(TarError::Io)?;
         } else {
             let path = entry.path().map_err(TarError::CannotReadEntryPath)?;
-
             writeln!(out, "{}", path.display()).map_err(TarError::Io)?;
         }
     }
@@ -92,6 +40,54 @@ pub fn list_archive(
     Ok(())
 }
 
+fn format_verbose_entry<R: Read>(entry: &tar::Entry<'_, R>) -> Result<String, TarError> {
+    let (mode, entry_type, owner, group, size, mtime) = {
+        let header = entry.header();
+        (
+            header.mode().unwrap_or(0),
+            header.entry_type(),
+            header
+                .username()
+                .ok()
+                .flatten()
+                .unwrap_or_default()
+                .to_owned(),
+            header
+                .groupname()
+                .ok()
+                .flatten()
+                .unwrap_or_default()
+                .to_owned(),
+            header.size().unwrap_or(0),
+            header.mtime().unwrap_or(0),
+        )
+    };
+
+    let path = entry.path().map_err(TarError::CannotReadEntryPath)?;
+
+    let type_char = match entry_type {
+        tar::EntryType::Directory => 'd',
+        tar::EntryType::Symlink => 'l',
+        tar::EntryType::Char => 'c',
+        tar::EntryType::Block => 'b',
+        tar::EntryType::Fifo => 'p',
+        _ => '-',
+    };
+    let perm_str = display_permissions_unix(mode, false);
+    let permissions = format!("{type_char}{perm_str}");
+
+    let dt: chrono::DateTime<Utc> = Utc
+        .timestamp_opt(mtime as i64, 0)
+        .single()
+        .unwrap_or_else(Utc::now);
+    let date_str = dt.format("%Y-%m-%d %H:%M");
+
+    Ok(format!(
+        "{permissions} {owner}/{group} {size:>8} {date_str} {}",
+        path.display()
+    ))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;