Unify Author type, optimize performance and fix parsing accuracy

CLAUDE.md

@@ -105,11 +105,11 @@
   - Repository::create_tag_with_options(name, target, options) -> Result<Tag> - create tag with options
   - Repository::delete_tag(name) -> Result<()> - delete tag
   - Repository::show_tag(name) -> Result<Tag> - detailed tag information
-  - Tag struct: name, hash, tag_type, message, tagger, timestamp
+  - Tag struct: name, hash, tag_type, message, tagger (may default), timestamp (may default)
   - TagType enum: Lightweight, Annotated
   - TagList: Box<[Tag]> with iterator methods (iter, lightweight, annotated), search (find, find_containing, for_commit), counting (len, lightweight_count, annotated_count)
   - TagOptions builder: annotated, force, message, sign with builder pattern (with_annotated, with_force, with_message, with_sign)
-  - Author struct: name, email, timestamp for annotated tag metadata
+  - Uses unified Author struct from log module for tagger metadata
 - **Stash operations**: Complete stash management with type-safe API
   - Repository::stash_list() -> Result<StashList> - list all stashes with comprehensive filtering
   - Repository::stash_save(message) -> Result<Stash> - create simple stash

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustic-git"
-version = "0.4.0"
+version = "0.5.0"
 edition = "2024"
 license = "MIT"
 description = "A Rustic Git - clean type-safe API over git cli"

src/commands/stash.rs

@@ -31,7 +31,7 @@
 use crate::error::{GitError, Result};
 use crate::repository::Repository;
 use crate::types::Hash;
-use crate::utils::git;
+use crate::utils::{git, parse_unix_timestamp};
 use chrono::{DateTime, Utc};
 use std::fmt;
 use std::path::PathBuf;
@@ -226,7 +226,7 @@ impl Repository {
         Self::ensure_git()?;
 
         let output = git(
-            &["stash", "list", "--format=%gd %H %gs"],
+            &["stash", "list", "--format=%gd %H %ct %gs"],
             Some(self.repo_path()),
         )?;
 
@@ -487,19 +487,34 @@ impl Repository {
 
 /// Parse a stash list line into a Stash struct
 fn parse_stash_line(index: usize, line: &str) -> Result<Stash> {
-    // Format: "stash@{0} hash On branch: message"
+    // Format: "stash@{0} hash timestamp On branch: message"
     let parts: Vec<&str> = line.splitn(4, ' ').collect();
 
     if parts.len() < 4 {
-        return Err(GitError::CommandFailed(
-            "Invalid stash list format".to_string(),
-        ));
+        return Err(GitError::CommandFailed(format!(
+            "Invalid stash list format: expected 4 parts, got {}",
+            parts.len()
+        )));
     }
 
     let hash = Hash::from(parts[1]);
 
+    // Parse timestamp - if it fails, the stash metadata may be corrupted
+    // Use Unix epoch as fallback to clearly indicate corrupted/invalid timestamp data
+    let timestamp = parse_unix_timestamp(parts[2]).unwrap_or_else(|_| {
+        // Timestamp parsing failed - this indicates malformed git stash metadata
+        // Use Unix epoch (1970-01-01) as fallback to make data corruption obvious
+        DateTime::from_timestamp(0, 0).unwrap_or_else(Utc::now)
+    });
+
     // Extract branch name and message from parts[3] (should be "On branch: message")
     let remainder = parts[3];
+    if remainder.is_empty() {
+        return Err(GitError::CommandFailed(
+            "Invalid stash format: missing branch and message information".to_string(),
+        ));
+    }
+
     let (branch, message) = if let Some(colon_pos) = remainder.find(':') {
         let branch_part = &remainder[..colon_pos];
         let message_part = &remainder[colon_pos + 1..].trim();
@@ -523,7 +538,7 @@ fn parse_stash_line(index: usize, line: &str) -> Result<Stash> {
         message,
         hash,
         branch,
-        timestamp: Utc::now(), // Simplified for now
+        timestamp,
     })
 }
 
@@ -792,4 +807,68 @@ mod tests {
         assert!(display_str.contains("stash@{0}"));
         assert!(display_str.contains("Test stash message"));
     }
+
+    #[test]
+    fn test_parse_stash_line_invalid_format() {
+        // Test with insufficient parts
+        let invalid_line = "stash@{0} abc123"; // Only 2 parts instead of 4
+        let result = parse_stash_line(0, invalid_line);
+
+        assert!(result.is_err());
+        if let Err(GitError::CommandFailed(msg)) = result {
+            assert!(msg.contains("Invalid stash list format"));
+            assert!(msg.contains("expected 4 parts"));
+            assert!(msg.contains("got 2"));
+        } else {
+            panic!("Expected CommandFailed error with specific message");
+        }
+    }
+
+    #[test]
+    fn test_parse_stash_line_empty_remainder() {
+        // Test with empty remainder part
+        let invalid_line = "stash@{0} abc123 1234567890 "; // Empty 4th part
+        let result = parse_stash_line(0, invalid_line);
+
+        assert!(result.is_err());
+        if let Err(GitError::CommandFailed(msg)) = result {
+            assert!(msg.contains("missing branch and message information"));
+        } else {
+            panic!("Expected CommandFailed error for empty remainder");
+        }
+    }
+
+    #[test]
+    fn test_parse_stash_line_valid_format() {
+        // Test with valid format
+        let valid_line = "stash@{0} abc123def456 1234567890 On master: test message";
+        let result = parse_stash_line(0, valid_line);
+
+        assert!(result.is_ok());
+        let stash = result.unwrap();
+        assert_eq!(stash.index, 0);
+        assert_eq!(stash.hash.as_str(), "abc123def456");
+        assert_eq!(stash.branch, "master");
+        assert_eq!(stash.message, "test message");
+    }
+
+    #[test]
+    fn test_parse_stash_line_with_invalid_timestamp() {
+        // Test stash with invalid timestamp - should still parse but use fallback timestamp
+        let line_with_invalid_timestamp =
+            "stash@{0} abc123def456 invalid-timestamp On master: test message";
+        let result = parse_stash_line(0, line_with_invalid_timestamp);
+
+        assert!(result.is_ok());
+        let stash = result.unwrap();
+        assert_eq!(stash.index, 0);
+        assert_eq!(stash.hash.as_str(), "abc123def456");
+        assert_eq!(stash.branch, "master");
+        assert_eq!(stash.message, "test message");
+
+        // The timestamp should use Unix epoch (1970-01-01) as fallback for invalid data
+        // Verify fallback timestamp is Unix epoch (indicates data corruption)
+        assert_eq!(stash.timestamp.timestamp(), 0); // Unix epoch
+        assert_eq!(stash.timestamp.format("%Y-%m-%d").to_string(), "1970-01-01");
+    }
 }

src/commands/tag.rs

@@ -28,10 +28,11 @@
 //! # Ok::<(), rustic_git::GitError>(())
 //! ```
 
+use crate::commands::log::Author;
 use crate::error::{GitError, Result};
 use crate::repository::Repository;
 use crate::types::Hash;
-use crate::utils::git;
+use crate::utils::{git, parse_unix_timestamp};
 use chrono::{DateTime, Utc};
 use std::fmt;
 
@@ -70,23 +71,6 @@ impl fmt::Display for TagType {
     }
 }
 
-/// Author information for annotated tags
-#[derive(Debug, Clone, PartialEq)]
-pub struct Author {
-    /// Author name
-    pub name: String,
-    /// Author email
-    pub email: String,
-    /// Author timestamp
-    pub timestamp: DateTime<Utc>,
-}
-
-impl fmt::Display for Author {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{} <{}>", self.name, self.email)
-    }
-}
-
 /// A collection of tags with efficient iteration and filtering methods
 #[derive(Debug, Clone)]
 pub struct TagList {
@@ -228,29 +212,31 @@ impl Repository {
     pub fn tags(&self) -> Result<TagList> {
         Self::ensure_git()?;
 
-        // Get list of tag names
-        let output = git(&["tag", "-l"], Some(self.repo_path()))?;
+        // Use git for-each-ref to get all tag information in a single call
+        // Format: refname:short objecttype objectname *objectname taggername taggeremail taggerdate:unix subject body
+        let output = git(
+            &[
+                "for-each-ref",
+                "--format=%(refname:short)|%(objecttype)|%(objectname)|%(*objectname)|%(taggername)|%(taggeremail)|%(taggerdate:unix)|%(subject)|%(body)",
+                "refs/tags/",
+            ],
+            Some(self.repo_path()),
+        )?;
 
         if output.trim().is_empty() {
             return Ok(TagList::new(vec![]));
         }
 
         let mut tags = Vec::new();
 
-        for tag_name in output.lines() {
-            let tag_name = tag_name.trim();
-            if tag_name.is_empty() {
+        for line in output.lines() {
+            let line = line.trim();
+            if line.is_empty() {
                 continue;
             }
 
-            // Get tag information
-            let show_output = git(
-                &["show", "--format=fuller", tag_name],
-                Some(self.repo_path()),
-            )?;
-
-            // Parse tag information
-            if let Ok(tag) = parse_tag_info(tag_name, &show_output) {
+            // Parse tag information from for-each-ref output
+            if let Ok(tag) = parse_for_each_ref_line(line) {
                 tags.push(tag);
             }
         }
@@ -402,7 +388,86 @@ impl Repository {
     }
 }
 
-/// Parse tag information from git show output
+/// Parse tag information from git for-each-ref output
+/// Format: refname:short|objecttype|objectname|*objectname|taggername|taggeremail|taggerdate:unix|subject|body
+fn parse_for_each_ref_line(line: &str) -> Result<Tag> {
+    let parts: Vec<&str> = line.split('|').collect();
+
+    if parts.len() < 9 {
+        return Err(GitError::CommandFailed(format!(
+            "Invalid for-each-ref format: expected 9 parts, got {}",
+            parts.len()
+        )));
+    }
+
+    let name = parts[0].to_string();
+    let object_type = parts[1];
+    let object_name = parts[2];
+    let dereferenced_object = parts[3]; // For annotated tags, this is the commit hash
+    let tagger_name = parts[4];
+    let tagger_email = parts[5];
+    let tagger_date = parts[6];
+    let subject = parts[7];
+    let body = parts[8];
+
+    // Determine tag type and commit hash
+    let (tag_type, hash) = if object_type == "tag" {
+        // Annotated tag - use dereferenced object (the commit it points to)
+        (TagType::Annotated, Hash::from(dereferenced_object))
+    } else {
+        // Lightweight tag - use object name (direct commit reference)
+        (TagType::Lightweight, Hash::from(object_name))
+    };
+
+    // Build tagger information for annotated tags
+    let tagger =
+        if tag_type == TagType::Annotated && !tagger_name.is_empty() && !tagger_email.is_empty() {
+            // Parse the timestamp - if it fails, the tag metadata may be corrupted
+            // Use Unix epoch as fallback to clearly indicate corrupted/invalid timestamp data
+            let timestamp = parse_unix_timestamp(tagger_date).unwrap_or_else(|_| {
+                // Timestamp parsing failed - this indicates malformed git metadata
+                // Use Unix epoch (1970-01-01) as fallback to make data corruption obvious
+                DateTime::from_timestamp(0, 0).unwrap()
+            });
+            Some(Author {
+                name: tagger_name.to_string(),
+                email: tagger_email.to_string(),
+                timestamp,
+            })
+        } else {
+            None
+        };
+
+    // Build message for annotated tags
+    let message = if tag_type == TagType::Annotated && (!subject.is_empty() || !body.is_empty()) {
+        let full_message = if !body.is_empty() {
+            format!("{}\n\n{}", subject, body)
+        } else {
+            subject.to_string()
+        };
+        Some(full_message.trim().to_string())
+    } else {
+        None
+    };
+
+    // Timestamp for the tag
+    let timestamp = if tag_type == TagType::Annotated {
+        tagger.as_ref().map(|t| t.timestamp)
+    } else {
+        None
+    };
+
+    Ok(Tag {
+        name,
+        hash,
+        tag_type,
+        message,
+        tagger,
+        timestamp,
+    })
+}
+
+/// Parse tag information from git show output (fallback method)
 fn parse_tag_info(tag_name: &str, show_output: &str) -> Result<Tag> {
     let lines: Vec<&str> = show_output.lines().collect();
 
@@ -483,16 +548,19 @@ fn parse_lightweight_tag(tag_name: &str, lines: &[&str]) -> Result<Tag> {
     })
 }
 
-/// Parse author information from a git log line
+/// Parse author information from a git tagger line
+/// Format: "Tagger: Name <email>" (timestamp not available in this format)
 fn parse_author_line(line: &str) -> Option<Author> {
-    // Parse format: "Name <email> timestamp timezone"
+    // Parse format: "Name <email>" (no timestamp in git show --format=fuller tagger line)
     if let Some(email_start) = line.find('<')
         && let Some(email_end) = line.find('>')
     {
         let name = line[..email_start].trim().to_string();
         let email = line[email_start + 1..email_end].to_string();
 
-        // Parse timestamp (simplified - just use current time for now)
+        // Timestamp is not available in the tagger line from git show --format=fuller
+        // We use the current time as a fallback, which matches the review feedback
+        // that tagger timestamp may default
         let timestamp = Utc::now();
 
         return Some(Author {
@@ -692,4 +760,47 @@ mod tests {
         // Clean up
         fs::remove_dir_all(&test_path).unwrap();
     }
+
+    #[test]
+    fn test_parse_for_each_ref_line_invalid_format() {
+        // Test with insufficient parts (should have 9 parts minimum)
+        let invalid_line = "tag1|commit|abc123"; // Only 3 parts instead of 9
+        let result = parse_for_each_ref_line(invalid_line);
+
+        assert!(result.is_err());
+
+        if let Err(GitError::CommandFailed(msg)) = result {
+            assert!(msg.contains("Invalid for-each-ref format"));
+            assert!(msg.contains("expected 9 parts"));
+            assert!(msg.contains("got 3"));
+        } else {
+            panic!("Expected CommandFailed error with specific message");
+        }
+    }
+
+    #[test]
+    fn test_parse_for_each_ref_line_with_invalid_timestamp() {
+        // Test annotated tag with invalid timestamp - should still parse but use fallback timestamp
+        let line_with_invalid_timestamp =
+            "v1.0.0|tag|abc123|def456|John Doe|john@example.com|invalid-timestamp|Subject|Body";
+        let result = parse_for_each_ref_line(line_with_invalid_timestamp);
+
+        assert!(result.is_ok());
+        let tag = result.unwrap();
+        assert_eq!(tag.name, "v1.0.0");
+        assert_eq!(tag.tag_type, TagType::Annotated);
+        assert!(tag.tagger.is_some());
+
+        // The timestamp should use Unix epoch (1970-01-01) as fallback for invalid data
+        let tagger = tag.tagger.unwrap();
+        assert_eq!(tagger.name, "John Doe");
+        assert_eq!(tagger.email, "john@example.com");
+
+        // Verify fallback timestamp is Unix epoch (indicates data corruption)
+        assert_eq!(tagger.timestamp.timestamp(), 0); // Unix epoch
+        assert_eq!(
+            tagger.timestamp.format("%Y-%m-%d").to_string(),
+            "1970-01-01"
+        );
+    }
 }