Fix issues from code review: glob handling, I/O optimization, validation, and UX

Author: lookbusy1344

CLAUDE.md

@@ -30,9 +30,10 @@ HashRust is a CLI file hashing utility written in Rust that supports multiple ha
 
 ### Module Structure
 - `main.rs` - CLI argument parsing, main worker function, and threading coordination
-- `classes.rs` - Core types: `HashAlgorithm` enum, `OutputEncoding` enum, `ConfigSettings` struct, `BasicHash` wrapper
-- `hasher.rs` - Generic hashing implementation using Digest trait, file I/O optimization for small/large files
-- `crc32.rs` - Custom CRC32 implementation (separate from other hash algorithms)
+- `core/types.rs` - Core types: `HashAlgorithm` enum, `OutputEncoding` enum, `BasicHash` wrapper
+- `cli/config.rs` - Configuration: `ConfigSettings` struct
+- `hash/` - Hashing logic and algorithm implementations
+- `io/` - File handling and glob pattern matching
 - `unit_tests.rs` - Unit test module
 
 ### Key Design Patterns
@@ -69,7 +70,7 @@ HashRust is a CLI file hashing utility written in Rust that supports multiple ha
 ### Performance Considerations
 - Multi-threading is default behavior
 - Buffer size optimized for typical file sizes
-- Uses BufReader for efficient file I/O
+- Direct file I/O with large buffers for performance
 - Release builds use LTO and panic=abort for size/speed
 
 ## Development Workflow

src/cli/args.rs

@@ -25,7 +25,9 @@ pub fn process_command_line(mut pargs: Arguments) -> Result<ConfigSettings> {
         other => other,
     };
 
-    if algo == HashAlgorithm::CRC32 && encoding != OutputEncoding::U32 {
+    if (algo == HashAlgorithm::CRC32 && encoding != OutputEncoding::U32)
+        || (algo != HashAlgorithm::CRC32 && encoding == OutputEncoding::U32)
+    {
         return Err(anyhow!(
             "CRC32 must use U32 encoding, and U32 encoding can only be used with CRC32"
         ));

src/core/worker.rs

@@ -61,7 +61,7 @@ where
     }
 
     for pathstr in paths {
-        let file_hash = hash_with_progress(config, pathstr.as_ref().to_string());
+        let file_hash = hash_with_progress(config, pathstr.as_ref().to_string(), false);
 
         match file_hash {
             Ok(basic_hash) => {
@@ -92,7 +92,11 @@ where
     };
 
     paths.par_iter().for_each(|pathstr| {
-        let file_hash = hash_with_progress(config, pathstr.as_ref().to_string());
+        let file_hash = hash_with_progress(
+            config,
+            pathstr.as_ref().to_string(),
+            overall_progress.is_some(),
+        );
 
         if let Some(ref pb) = overall_progress {
             pb.inc(1);
@@ -116,13 +120,17 @@ where
     }
 }
 
-fn hash_with_progress<S>(config: &ConfigSettings, pathstr: S) -> Result<BasicHash>
+fn hash_with_progress<S>(
+    config: &ConfigSettings,
+    pathstr: S,
+    suppress_spinner: bool,
+) -> Result<BasicHash>
 where
     S: AsRef<str> + Display + Clone + Send + 'static,
 {
     let pathstr_clone = pathstr.clone();
 
-    let progress_handle = if config.no_progress {
+    let progress_handle = if config.no_progress || suppress_spinner {
         None
     } else {
         ProgressManager::create_file_progress(pathstr_clone.clone(), config.debug_mode)

src/hash/digest_impl.rs

@@ -1,5 +1,5 @@
 use std::fs::File;
-use std::io::{BufReader, Read};
+use std::io::Read;
 use std::path::Path;
 
 use byteorder::{BigEndian, ByteOrder};
@@ -17,12 +17,11 @@ fn hash_file<D: Digest>(filename: impl AsRef<str>) -> anyhow::Result<Output<D>>
         return hash_file_whole::<D>(filename);
     }
 
-    let file = File::open(filename.as_ref())?;
-    let mut reader = BufReader::new(file);
+    let mut file = File::open(filename.as_ref())?;
     let mut buffer = build_heap_buffer(BUFFER_SIZE);
     let mut hasher = D::new();
 
-    while let Ok(bytes_read) = reader.read(&mut buffer) {
+    while let Ok(bytes_read) = file.read(&mut buffer) {
         if bytes_read == 0 {
             break;
         }

src/io/files.rs

@@ -50,28 +50,45 @@ fn get_paths_matching_glob(config: &ConfigSettings) -> Result<Vec<String>> {
     let mut result = Vec::new();
 
     for pattern in &config.supplied_paths {
-        let glob_matches: Vec<_> = glob::glob_with(pattern, glob_settings)?
-            .filter_map(|entry| match entry {
-                Ok(path) if path.is_file() => Some(path.to_string_lossy().into_owned()),
-                _ => None,
-            })
-            .collect();
+        let glob_result = glob::glob_with(pattern, glob_settings);
 
-        if glob_matches.is_empty() {
-            if file_exists(pattern) {
-                result.push(pattern.clone());
-            } else if !pattern.contains(['*', '?', '[', ']']) {
-                let path = std::path::Path::new(pattern);
-                if path.exists() && path.is_dir() {
-                    if config.debug_mode {
-                        eprintln!("Ignoring directory: {pattern}");
+        match glob_result {
+            Ok(paths) => {
+                let glob_matches: Vec<_> = paths
+                    .filter_map(|entry| match entry {
+                        Ok(path) if path.is_file() => Some(path.to_string_lossy().into_owned()),
+                        _ => None,
+                    })
+                    .collect();
+
+                if glob_matches.is_empty() {
+                    if file_exists(pattern) {
+                        result.push(pattern.clone());
+                    } else if !pattern.contains(['*', '?', '[', ']']) {
+                        let path = std::path::Path::new(pattern);
+                        if path.exists() && path.is_dir() {
+                            if config.debug_mode {
+                                eprintln!("Ignoring directory: {pattern}");
+                            }
+                        } else {
+                            return Err(anyhow::anyhow!("File not found: {}", pattern));
+                        }
                     }
                 } else {
-                    return Err(anyhow::anyhow!("File not found: {}", pattern));
+                    result.extend(glob_matches);
+                }
+            }
+            Err(_) => {
+                // If glob fails (e.g. invalid pattern), treat as literal file
+                if file_exists(pattern) {
+                    result.push(pattern.clone());
+                } else {
+                    return Err(anyhow::anyhow!(
+                        "File not found or invalid glob: {}",
+                        pattern
+                    ));
                 }
             }
-        } else {
-            result.extend(glob_matches);
         }
     }
 

src/progress.rs

@@ -7,9 +7,9 @@
 use indicatif::{ProgressBar, ProgressStyle};
 use std::fmt::Display;
 use std::sync::{
+    Arc,
     atomic::{AtomicUsize, Ordering},
     mpsc,
-    Arc,
 };
 use std::time::Duration;
 

tests/integration_tests.rs

@@ -222,6 +222,37 @@ fn test_crc32_with_invalid_encoding_error() {
     assert!(stderr.to_lowercase().contains("error"));
 }
 
+#[test]
+fn test_u32_encoding_with_non_crc32_error() {
+    let test_content = "test";
+    let temp_dir = std::env::temp_dir();
+    let test_file = temp_dir.join("test_u32_invalid.txt");
+    fs::write(&test_file, test_content).expect("Failed to write test file");
+
+    let output = Command::new("cargo")
+        .args(&[
+            "run",
+            "--",
+            "-a",
+            "MD5",
+            "-e",
+            "U32",
+            test_file.to_str().unwrap(),
+        ])
+        .output()
+        .expect("Failed to execute hash_rust");
+
+    fs::remove_file(&test_file).ok();
+
+    // Should fail with non-zero exit code
+    assert!(!output.status.success());
+    let stderr = String::from_utf8_lossy(&output.stderr);
+
+    // Should contain error message about incompatible algorithm/encoding combination
+    assert!(stderr.to_lowercase().contains("error"));
+    assert!(stderr.contains("CRC32 must use U32 encoding"));
+}
+
 #[test]
 fn test_empty_file_path_error() {
     // Test with no file arguments