Fix multi-hunk serialization bug, clippy warnings, and documentation issues

src/lib.rs

@@ -12,7 +12,7 @@
 //! ## Quick Start
 //!
 //! A simple Vec of primitives can be diffed using Myers algorithm.
-//! The diff can be transformed into a series of [`Hunk`]s.
+//! The diff can be transformed into a series of [`patch::Hunk`]s.
 //! Hunks can be transformed into a textual diff or applied to an input.
 //!
 //! `apply(&old, hunks(diff(&old, &new))) == Ok(new)`
@@ -33,7 +33,7 @@
 //! ```
 //!
 //! For nested structures a recursive diffing algorithm is provided.
-//! The diff will return a list of [`Change`]s.
+//! The diff will return a list of [`recursive::Change`]s.
 //! Changes can be transformed into Hunks and applied.
 //! Changes cannot be serialized, since there is no consensus on a textual format.
 //!

src/myers/mod.rs

@@ -29,8 +29,8 @@ impl V {
 /// Computes the diff between two strings after breaking them into newlines
 /// and running `diff`.
 pub fn diff_lines(old: &str, new: &str) -> Diff<String> {
-    let old_lines: Vec<String> = old.split('\n').map(|l| l.to_string()).collect();
-    let new_lines: Vec<String> = new.split('\n').map(|l| l.to_string()).collect();
+    let old_lines: Vec<String> = old.split('\n').map(ToString::to_string).collect();
+    let new_lines: Vec<String> = new.split('\n').map(ToString::to_string).collect();
     diff(&old_lines, &new_lines)
 }
 
@@ -125,9 +125,9 @@ fn traceback<T: Eq + Clone>(
         }
         if d > 0 {
             if prev_k == k - 1 {
-                changes.push(Edit::Delete(old[x - 1].clone()))
+                changes.push(Edit::Delete(old[x - 1].clone()));
             } else {
-                changes.push(Edit::Insert(new[y - 1].clone()))
+                changes.push(Edit::Insert(new[y - 1].clone()));
             }
         }
         x = prev_x;

src/patch/mod.rs

@@ -62,7 +62,7 @@ impl<T: Eq + Clone> HunkBuilder<T> {
                         old_start,
                         new_start,
                         changes,
-                    });
+                    })
                 };
 
                 match modify {
@@ -111,7 +111,12 @@ pub fn hunks<T: Eq + Clone>(edits: Vec<Edit<T>>) -> Vec<Hunk<T>> {
 }
 
 /// Applies a list of hunks to an input
-/// Can return a PatchError in case of mismatches between hunks and input
+/// Can return a [`PatchError`] in case of mismatches between hunks and input.
+///
+/// # Errors
+///
+/// Returns [`PatchError::InvalidFormat`] if a hunk's context lines don't match
+/// the corresponding lines in `old`, or if hunks cannot be applied in order.
 /// ```
 ///  use patchwork::myers::{diff, Edit};
 ///  use patchwork::patch::{apply, Hunk};
@@ -154,33 +159,37 @@ pub fn apply<T: PartialEq + Display + Clone>(
 
     while old_line < old.len() {
         if let Some(hunk) = hunk_iter.peek() {
-            if old_line == hunk.old_start {
-                for change in &hunk.changes {
-                    match change {
-                        Edit::Equal(t) => {
-                            if old[old_line] != *t {
-                                return Err(PatchError::InvalidFormat(format!(
-                                    "Context mismatch at line {}: expected '{}', found '{}'",
-                                    old_line, t, old[old_line]
-                                )));
+            match old_line.cmp(&hunk.old_start) {
+                std::cmp::Ordering::Equal => {
+                    for change in &hunk.changes {
+                        match change {
+                            Edit::Equal(t) => {
+                                if old[old_line] != *t {
+                                    return Err(PatchError::InvalidFormat(format!(
+                                        "Context mismatch at line {}: expected '{}', found '{}'",
+                                        old_line, t, old[old_line]
+                                    )));
+                                }
+                                result.push(old[old_line].clone());
+                                old_line += 1;
+                            }
+                            Edit::Insert(t) => {
+                                result.push(t.clone());
+                            }
+                            Edit::Delete(_) => {
+                                old_line += 1;
                             }
-                            result.push(old[old_line].clone());
-                            old_line += 1;
-                        }
-                        Edit::Insert(t) => {
-                            result.push(t.clone());
-                        }
-                        Edit::Delete(_) => {
-                            old_line += 1;
                         }
                     }
+                    hunk_iter.next();
+                }
+                std::cmp::Ordering::Less => {
+                    result.push(old[old_line].clone());
+                    old_line += 1;
+                }
+                std::cmp::Ordering::Greater => {
+                    return Err(PatchError::InvalidFormat("Cannot apply hunks".to_string()));
                 }
-                hunk_iter.next();
-            } else if old_line < hunk.old_start {
-                result.push(old[old_line].clone());
-                old_line += 1;
-            } else {
-                return Err(PatchError::InvalidFormat("Cannot apply hunks".to_string()));
             }
         } else {
             result.push(old[old_line].clone());

src/patch/types.rs

@@ -1,7 +1,7 @@
 use crate::myers::Edit;
 
 /// Represents a Hunk resulting from a Myers diff.
-/// Please note that `changes` will include maximum 3 context elements, i.e. Edit::Equal
+/// Please note that `changes` will include maximum 3 context elements, i.e. `Edit::Equal`
 /// and this is reflected in the `old_start` value
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Hunk<T> {

src/recursive/diffable.rs

@@ -1,4 +1,4 @@
-use crate::recursive::types::*;
+use crate::recursive::types::{Node, Primitive};
 use std::collections::HashMap;
 
 /// Trait to transform a given structure into a `[Node]` tree or viceversa.
@@ -18,12 +18,12 @@ pub trait Diffable {
 impl<T: Diffable> Diffable for Vec<T> {
     type P = T::P;
     fn to_node(&self) -> Node<T::P> {
-        Node::Sequence(self.iter().map(|e| e.to_node()).collect())
+        Node::Sequence(self.iter().map(Diffable::to_node).collect())
     }
 
     fn from_node(node: Node<Self::P>) -> Self {
         match node {
-            Node::Sequence(v) => v.into_iter().map(|e| T::from_node(e)).collect(),
+            Node::Sequence(v) => v.into_iter().map(T::from_node).collect(),
             _ => unreachable!(),
         }
     }

src/serialization.rs

@@ -18,14 +18,20 @@ pub trait ToPatch: Sized {
 ///
 /// Implemented for `Edit<String>` and `Vec<Hunk<String>>`.
 pub trait FromPatch: Sized {
+    /// Parse a unified diff patch string into a structured representation.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`PatchError::InvalidFormat`] if the patch header is missing or malformed.
+    /// Returns [`PatchError::UnexpectedToken`] if a line starts with an unexpected character.
     fn from_patch(s: &str) -> Result<Self, PatchError>;
 }
 
-/// Represents an error parsing or applying a diff
+/// Represents an error parsing or applying a diff.
 #[derive(Debug, PartialEq)]
 pub enum PatchError {
-    /// The patch is structurally invalid, e.g. missing `---`/`+++` header.
-    /// The patch cannot be applied to the given structure.
+    /// The patch is structurally invalid, e.g. missing `---`/`+++` header,
+    /// or the patch cannot be applied to the given structure.
     InvalidFormat(String),
     /// A line in the patch starts with an unexpected character.
     UnexpectedToken(String),
@@ -92,7 +98,8 @@ impl<T: ToString> ToPatch for Vec<Hunk<T>> {
         let hunks = self
             .iter()
             .map(|h| h.to_patch(None, None))
-            .collect::<String>();
+            .collect::<Vec<String>>()
+            .join("\n");
         format!("{}{}", header, hunks)
     }
 }
@@ -187,4 +194,42 @@ mod tests {
             prop_assert_eq!(Vec::<Hunk<String>>::from_patch(&patch).unwrap(), hunks);
         }
     }
+
+    #[test]
+    fn test_multi_hunk_patch_format() {
+        let old: Vec<&str> = vec!["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"];
+        let new: Vec<&str> = vec!["X", "b", "c", "d", "e", "f", "g", "h", "i", "Y"];
+        let edits = diff(&old, &new);
+        let h = hunks(edits);
+        assert_eq!(h.len(), 2, "expected 2 hunks");
+        let patch = h.to_patch(Some("old.txt"), Some("new.txt"));
+        // Each @@ header must start on its own line
+        for line in patch.lines() {
+            if line.starts_with("@@") || line.starts_with("---") || line.starts_with("+++") {
+                continue;
+            }
+            assert!(
+                !line.contains("@@"),
+                "@@ header is not on its own line: {:?}",
+                line
+            );
+        }
+    }
+
+    #[test]
+    fn test_multi_hunk_roundtrip() {
+        let old: Vec<String> = vec!["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]
+            .into_iter()
+            .map(String::from)
+            .collect();
+        let new: Vec<String> = vec!["X", "b", "c", "d", "e", "f", "g", "h", "i", "Y"]
+            .into_iter()
+            .map(String::from)
+            .collect();
+        let edits = diff(&old, &new);
+        let h = hunks(edits);
+        let patch = h.to_patch(Some("old.txt"), Some("new.txt"));
+        let parsed = Vec::<Hunk<String>>::from_patch(&patch).unwrap();
+        assert_eq!(parsed, h);
+    }
 }