refactor

Author: Byron

gix-object/src/commit/message/body.rs

@@ -23,10 +23,10 @@ pub struct Trailers<'a> {
 #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct TrailerRef<'a> {
-    /// The name of the trailer, like "Signed-off-by", up to the separator ": "
+    /// The name of the trailer, like "Signed-off-by", up to the separator `: `.
     #[cfg_attr(feature = "serde", serde(borrow))]
     pub token: &'a BStr,
-    /// The value right after the separator ": ", with leading and trailing whitespace trimmed.
+    /// The value right after the separator `: `, with leading and trailing whitespace trimmed.
     /// Note that multi-line values aren't currently supported.
     pub value: &'a BStr,
 }
@@ -93,7 +93,7 @@ impl<'a> BodyRef<'a> {
         self.body_without_trailer
     }
 
-    /// Return an iterator over the trailers parsed from the last paragraph of the body. May be empty.
+    /// Return an iterator over the trailers parsed from the last paragraph of the body. Maybe empty.
     pub fn trailers(&self) -> Trailers<'a> {
         Trailers {
             cursor: self.start_of_trailer,
@@ -115,72 +115,65 @@ impl Deref for BodyRef<'_> {
     }
 }
 
+/// Convenience methods
 impl TrailerRef<'_> {
-    /// Check if this trailer is a "Signed-off-by" trailer (case-insensitive).
+    /// Check if this trailer is a `Signed-off-by` trailer (case-insensitive).
     pub fn is_signed_off_by(&self) -> bool {
         self.token.eq_ignore_ascii_case(b"Signed-off-by")
     }
 
-    /// Check if this trailer is a "Co-authored-by" trailer (case-insensitive).
+    /// Check if this trailer is a `Co-authored-by` trailer (case-insensitive).
     pub fn is_co_authored_by(&self) -> bool {
         self.token.eq_ignore_ascii_case(b"Co-authored-by")
     }
 
-    /// Check if this trailer is an "Acked-by" trailer (case-insensitive).
+    /// Check if this trailer is an `Acked-by` trailer (case-insensitive).
     pub fn is_acked_by(&self) -> bool {
         self.token.eq_ignore_ascii_case(b"Acked-by")
     }
 
-    /// Check if this trailer is a "Reviewed-by" trailer (case-insensitive).
+    /// Check if this trailer is a `Reviewed-by` trailer (case-insensitive).
     pub fn is_reviewed_by(&self) -> bool {
         self.token.eq_ignore_ascii_case(b"Reviewed-by")
     }
 
-    /// Check if this trailer is a "Tested-by" trailer (case-insensitive).
+    /// Check if this trailer is a `Tested-by` trailer (case-insensitive).
     pub fn is_tested_by(&self) -> bool {
         self.token.eq_ignore_ascii_case(b"Tested-by")
     }
 
     /// Check if this trailer represents any kind of authorship or attribution
-    /// (Signed-off-by, Co-authored-by, etc.).
+    /// (`Signed-off-by`, `Co-authored-by`, etc.).
     pub fn is_attribution(&self) -> bool {
         self.is_signed_off_by()
             || self.is_co_authored_by()
             || self.is_acked_by()
             || self.is_reviewed_by()
             || self.is_tested_by()
     }
-
-    /// Get the token as a case-normalized string for comparison purposes.
-    /// This can be useful for custom filtering logic.
-    pub fn token_normalized(&self) -> String {
-        String::from_utf8_lossy(self.token).to_ascii_lowercase()
-    }
 }
 
+/// Convenience methods
 impl<'a> Trailers<'a> {
-    /// Filter trailers to only include "Signed-off-by" entries.
+    /// Filter trailers to only include `Signed-off-by` entries.
     pub fn signed_off_by(self) -> impl Iterator<Item = TrailerRef<'a>> {
         self.filter(TrailerRef::is_signed_off_by)
     }
 
-    /// Filter trailers to only include "Co-authored-by" entries.
+    /// Filter trailers to only include `Co-authored-by` entries.
     pub fn co_authored_by(self) -> impl Iterator<Item = TrailerRef<'a>> {
         self.filter(TrailerRef::is_co_authored_by)
     }
 
-    /// Filter trailers to only include attribution-related entries
-    /// (Signed-off-by, Co-authored-by, Acked-by, Reviewed-by, Tested-by).
+    /// Filter trailers to only include attribution-related entries.
+    /// (`Signed-off-by`, `Co-authored-by`, `Acked-by`, `Reviewed-by`, `Tested-by`).
     pub fn attributions(self) -> impl Iterator<Item = TrailerRef<'a>> {
         self.filter(TrailerRef::is_attribution)
     }
 
-    /// Collect all unique authors from Signed-off-by and Co-authored-by trailers.
-    /// Returns a Vec of author strings.
-    pub fn collect_authors(self) -> Vec<&'a BStr> {
+    /// Filter trailers to only include authors from `Signed-off-by` and `Co-authored-by` entries.
+    pub fn authors(self) -> impl Iterator<Item = TrailerRef<'a>> {
         self.filter(|trailer| trailer.is_signed_off_by() || trailer.is_co_authored_by())
-            .map(|trailer| trailer.value)
-            .collect()
     }
 }
 
@@ -221,68 +214,3 @@ mod test_parse_trailer {
         assert_eq!(parse("foo: bar\r\n"), ("foo".into(), "bar".into()));
     }
 }
-
-#[cfg(test)]
-mod test_trailer_convenience {
-    use super::*;
-
-    #[test]
-    fn test_signed_off_by_detection() {
-        let trailer = TrailerRef {
-            token: "Signed-off-by".into(),
-            value: "John Doe <john@example.com>".into(),
-        };
-        assert!(trailer.is_signed_off_by());
-        assert!(!trailer.is_co_authored_by());
-        assert!(trailer.is_attribution());
-    }
-
-    #[test]
-    fn test_case_insensitive_detection() {
-        let trailer = TrailerRef {
-            token: "signed-off-by".into(),
-            value: "John Doe <john@example.com>".into(),
-        };
-        assert!(trailer.is_signed_off_by());
-
-        let trailer2 = TrailerRef {
-            token: "CO-AUTHORED-BY".into(),
-            value: "Jane Smith <jane@example.com>".into(),
-        };
-        assert!(trailer2.is_co_authored_by());
-        assert!(trailer2.is_attribution());
-    }
-
-    #[test]
-    fn test_multiple_trailer_types() {
-        let trailer1 = TrailerRef {
-            token: "Reviewed-by".into(),
-            value: "Reviewer <reviewer@example.com>".into(),
-        };
-        let trailer2 = TrailerRef {
-            token: "Custom-Field".into(),
-            value: "Some value".into(),
-        };
-
-        assert!(trailer1.is_reviewed_by());
-        assert!(trailer1.is_attribution());
-        assert!(!trailer2.is_attribution());
-    }
-
-    #[test]
-    fn test_collect_authors() {
-        // This would need to be tested with actual Trailers iterator
-        // but shows the expected behavior
-        let trailer1 = TrailerRef {
-            token: "Signed-off-by".into(),
-            value: "John Doe <john@example.com>".into(),
-        };
-        let trailer2 = TrailerRef {
-            token: "Co-authored-by".into(),
-            value: "Jane Smith <jane@example.com>".into(),
-        };
-
-        assert!(trailer1.is_signed_off_by());
-        assert!(trailer2.is_co_authored_by());
-    }
-}

gix-object/src/commit/message/mod.rs

@@ -17,40 +17,44 @@ impl<'a> CommitRef<'a> {
     }
 
     /// Return an iterator over message trailers as obtained from the last paragraph of the commit message.
-    /// May be empty.
+    /// Maybe empty.
     pub fn message_trailers(&self) -> body::Trailers<'a> {
         BodyRef::from_bytes(self.message).trailers()
     }
+}
 
-    /// Get an iterator over all Signed-off-by trailers in the commit message.
+/// Convenience methods
+impl<'a> CommitRef<'a> {
+    /// Get an iterator over all `Signed-off-by` trailers in the commit message.
     /// This is useful for finding who signed off on the commit.
     pub fn signed_off_by_trailers(&self) -> impl Iterator<Item = body::TrailerRef<'a>> {
         self.message_trailers().signed_off_by()
     }
 
-    /// Get an iterator over all Co-authored-by trailers in the commit message.
+    /// Get an iterator over `Co-authored-by` trailers in the commit message.
     /// This is useful for squashed commits that contain multiple authors.
     pub fn co_authored_by_trailers(&self) -> impl Iterator<Item = body::TrailerRef<'a>> {
         self.message_trailers().co_authored_by()
     }
 
-    /// Get all authors mentioned in Signed-off-by and Co-authored-by trailers.
+    /// Get all authors mentioned in `Signed-off-by` and `Co-authored-by` trailers.
     /// This is useful for squashed commits that contain multiple authors.
     /// Returns a Vec of author strings that can include both signers and co-authors.
-    pub fn all_authors(&self) -> Vec<&'a BStr> {
-        self.message_trailers().collect_authors()
+    pub fn author_trailers(&self) -> impl Iterator<Item = body::TrailerRef<'a>> {
+        self.message_trailers().authors()
     }
 
     /// Get an iterator over all attribution-related trailers
-    /// (Signed-off-by, Co-authored-by, Acked-by, Reviewed-by, Tested-by).
+    /// (`Signed-off-by,` `Co-authored-by`, `Acked-by`, `Reviewed-by`, `Tested-by`).
     /// This provides a comprehensive view of everyone who contributed to or reviewed the commit.
+    /// Note that the same name may occur multiple times, it's not a unified list.
     pub fn attribution_trailers(&self) -> impl Iterator<Item = body::TrailerRef<'a>> {
         self.message_trailers().attributions()
     }
 }
 
 impl<'a> MessageRef<'a> {
-    /// Parse the given `input` as message.
+    /// Parse the given `input` as a message.
     ///
     /// Note that this cannot fail as everything will be interpreted as title if there is no body separator.
     pub fn from_bytes(input: &'a [u8]) -> Self {