feat!: add DiffLineType and HunkHeader

This commit modifies the public API of `ConsumeHunk::consume_hunk` to
use the new types `DiffLineType` and `HunkHeader`. It also shifts
responsibility for adding newlines to the API's consumer.

Author: cruessler

gix-diff/src/blob/unified_diff.rs

@@ -26,6 +26,28 @@ impl ContextSize {
     }
 }
 
+/// Represents the type of a line in a unified diff.
+#[doc(alias = "git2")]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DiffLineType {
+    /// A line that exists in both the old and the new version is called a context line.
+    Context,
+    /// A line that was added in the new version.
+    Add,
+    /// A line that was removed from the old version.
+    Remove,
+}
+
+impl DiffLineType {
+    const fn to_prefix(self) -> char {
+        match self {
+            DiffLineType::Context => ' ',
+            DiffLineType::Add => '+',
+            DiffLineType::Remove => '-',
+        }
+    }
+}
+
 /// Specify where to put a newline.
 #[derive(Debug, Copy, Clone)]
 pub enum NewlineSeparator<'a> {
@@ -39,31 +61,45 @@ pub enum NewlineSeparator<'a> {
     AfterHeaderAndWhenNeeded(&'a str),
 }
 
+/// Holds information about a unified diff hunk, specifically with respect to line numbers.
+pub struct HunkHeader {
+    /// The 1-based start position in the 'before' lines.
+    pub before_hunk_start: u32,
+    /// The size of the 'before' hunk in lines.
+    pub before_hunk_len: u32,
+    /// The 1-based start position in the 'after' lines.
+    pub after_hunk_start: u32,
+    /// The size of the 'after' hunk in lines.
+    pub after_hunk_len: u32,
+}
+
+impl std::fmt::Display for HunkHeader {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "@@ -{},{} +{},{} @@",
+            self.before_hunk_start, self.before_hunk_len, self.after_hunk_start, self.after_hunk_len
+        )
+    }
+}
+
 /// A utility trait for use in [`UnifiedDiff`](super::UnifiedDiff).
 pub trait ConsumeHunk {
     /// The item this instance produces after consuming all hunks.
     type Out;
 
-    /// Consume a single `hunk` in unified diff format, that would be prefixed with `header`.
-    /// Note that all newlines are added.
+    /// Consume a single hunk. Note that it is the implementation's responsibility to add newlines
+    /// where requested by `newline`.
     ///
     /// Note that the [`UnifiedDiff`](super::UnifiedDiff) sink will wrap its output in an [`std::io::Result`].
     /// After this method returned its first error, it will not be called anymore.
-    ///
-    /// The following is hunk-related information and the same that is used in the `header`.
-    /// * `before_hunk_start` is the 1-based first line of this hunk in the old file.
-    /// * `before_hunk_len` the amount of lines of this hunk in the old file.
-    /// * `after_hunk_start` is the 1-based first line of this hunk in the new file.
-    /// * `after_hunk_len` the amount of lines of this hunk in the new file.
     fn consume_hunk(
         &mut self,
-        before_hunk_start: u32,
-        before_hunk_len: u32,
-        after_hunk_start: u32,
-        after_hunk_len: u32,
-        header: &str,
-        hunk: &[u8],
+        header: HunkHeader,
+        lines: &[(DiffLineType, &[u8])],
+        newline: NewlineSeparator<'_>,
     ) -> std::io::Result<()>;
+
     /// Called after the last hunk is consumed to produce an output.
     fn finish(self) -> Self::Out;
 }
@@ -75,14 +111,10 @@ pub(super) mod _impl {
     use imara_diff::{intern, Sink};
     use intern::{InternedInput, Interner, Token};
 
-    use super::{ConsumeHunk, ContextSize, NewlineSeparator};
-
-    const CONTEXT: char = ' ';
-    const ADDITION: char = '+';
-    const REMOVAL: char = '-';
+    use super::{ConsumeHunk, ContextSize, DiffLineType, HunkHeader, NewlineSeparator};
 
-    /// A [`Sink`] that creates a textual diff in the format typically output by git or `gnu-diff` if the `-u` option is used,
-    /// and passes it in full to a consumer.
+    /// A [`Sink`] that creates a unified diff. It can be used to create a textual diff in the
+    /// format typically output by `git` or `gnu-diff` if the `-u` option is used.
     pub struct UnifiedDiff<'a, T, D>
     where
         T: Hash + Eq + AsRef<[u8]>,
@@ -108,8 +140,8 @@ pub(super) mod _impl {
         ctx_size: u32,
         newline: NewlineSeparator<'a>,
 
-        buffer: Vec<u8>,
-        header_buf: String,
+        buffer: Vec<(DiffLineType, Vec<u8>)>,
+
         delegate: D,
 
         err: Option<std::io::Error>,
@@ -120,12 +152,13 @@ pub(super) mod _impl {
         T: Hash + Eq + AsRef<[u8]>,
         D: ConsumeHunk,
     {
-        /// Create a new instance to create unified diff using the lines in `input`,
+        /// Create a new instance to create a unified diff using the lines in `input`,
         /// which also must be used when running the diff algorithm.
         /// `context_size` is the amount of lines around each hunk which will be passed
-        ///to `consume_hunk`.
+        /// to `consume_hunk`.
         ///
-        /// `consume_hunk` is called for each hunk in unified-diff format, as created from each line separated by `newline_separator`.
+        /// `consume_hunk` is called for each hunk with all the information required to create a
+        /// unified diff.
         pub fn new(
             input: &'a InternedInput<T>,
             consume_hunk: D,
@@ -147,28 +180,16 @@ pub(super) mod _impl {
                 newline: newline_separator,
 
                 buffer: Vec::with_capacity(8),
-                header_buf: String::new(),
                 delegate: consume_hunk,
 
                 err: None,
             }
         }
 
-        fn print_tokens(&mut self, tokens: &[Token], prefix: char) {
+        fn print_tokens(&mut self, tokens: &[Token], line_type: DiffLineType) {
             for &token in tokens {
-                self.buffer.push_char(prefix);
-                let line = &self.interner[token];
-                self.buffer.push_str(line);
-                match self.newline {
-                    NewlineSeparator::AfterHeaderAndLine(nl) => {
-                        self.buffer.push_str(nl);
-                    }
-                    NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => {
-                        if !line.as_ref().ends_with_str(nl) {
-                            self.buffer.push_str(nl);
-                        }
-                    }
-                }
+                let content = self.interner[token].as_ref().to_vec();
+                self.buffer.push((line_type, content));
             }
         }
 
@@ -183,38 +204,35 @@ pub(super) mod _impl {
 
             let hunk_start = self.before_hunk_start + 1;
             let hunk_end = self.after_hunk_start + 1;
-            self.header_buf.clear();
-            std::fmt::Write::write_fmt(
-                &mut self.header_buf,
-                format_args!(
-                    "@@ -{},{} +{},{} @@{nl}",
-                    hunk_start,
-                    self.before_hunk_len,
-                    hunk_end,
-                    self.after_hunk_len,
-                    nl = match self.newline {
-                        NewlineSeparator::AfterHeaderAndLine(nl) | NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => {
-                            nl
-                        }
-                    }
-                ),
-            )
-            .map_err(|err| std::io::Error::new(ErrorKind::Other, err))?;
-            self.delegate.consume_hunk(
-                hunk_start,
-                self.before_hunk_len,
-                hunk_end,
-                self.after_hunk_len,
-                &self.header_buf,
-                &self.buffer,
-            )?;
+
+            let header = HunkHeader {
+                before_hunk_start: hunk_start,
+                before_hunk_len: self.before_hunk_len,
+                after_hunk_start: hunk_end,
+                after_hunk_len: self.after_hunk_len,
+            };
+
+            // TODO:
+            // If I remember correctly, we wanted this buffer to be reused between calls, but I ran
+            // into lifetime issues when I tried to make it a field on `UnifiedDiff`.
+            let buffer2: Vec<(DiffLineType, &[u8])> = self
+                .buffer
+                .iter()
+                .map(|(line_type, content)| (*line_type, content.as_slice()))
+                .collect();
+
+            self.delegate.consume_hunk(header, &buffer2, self.newline)?;
 
             self.reset_hunks();
             Ok(())
         }
 
         fn print_context_and_update_pos(&mut self, print: Range<u32>, move_to: u32) {
-            self.print_tokens(&self.before[print.start as usize..print.end as usize], CONTEXT);
+            self.print_tokens(
+                &self.before[print.start as usize..print.end as usize],
+                DiffLineType::Context,
+            );
+
             let len = print.end - print.start;
             self.ctx_pos = Some(move_to);
             self.before_hunk_len += len;
@@ -270,8 +288,11 @@ pub(super) mod _impl {
             self.before_hunk_len += before.end - before.start;
             self.after_hunk_len += after.end - after.start;
 
-            self.print_tokens(&self.before[before.start as usize..before.end as usize], REMOVAL);
-            self.print_tokens(&self.after[after.start as usize..after.end as usize], ADDITION);
+            self.print_tokens(
+                &self.before[before.start as usize..before.end as usize],
+                DiffLineType::Remove,
+            );
+            self.print_tokens(&self.after[after.start as usize..after.end as usize], DiffLineType::Add);
         }
 
         fn finish(mut self) -> Self::Out {
@@ -289,12 +310,32 @@ pub(super) mod _impl {
     impl ConsumeHunk for String {
         type Out = Self;
 
-        fn consume_hunk(&mut self, _: u32, _: u32, _: u32, _: u32, header: &str, hunk: &[u8]) -> std::io::Result<()> {
-            self.push_str(header);
-            self.push_str(
-                hunk.to_str()
-                    .map_err(|err| std::io::Error::new(ErrorKind::Other, err))?,
-            );
+        fn consume_hunk(
+            &mut self,
+            header: HunkHeader,
+            lines: &[(DiffLineType, &[u8])],
+            newline: NewlineSeparator<'_>,
+        ) -> std::io::Result<()> {
+            self.push_str(&header.to_string());
+            self.push_str(match newline {
+                NewlineSeparator::AfterHeaderAndLine(nl) | NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => nl,
+            });
+
+            for &(line_type, content) in lines {
+                self.push(line_type.to_prefix());
+                self.push_str(std::str::from_utf8(content).map_err(|e| std::io::Error::new(ErrorKind::Other, e))?);
+
+                match newline {
+                    NewlineSeparator::AfterHeaderAndLine(nl) => {
+                        self.push_str(nl);
+                    }
+                    NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => {
+                        if !content.ends_with_str(nl) {
+                            self.push_str(nl);
+                        }
+                    }
+                }
+            }
             Ok(())
         }
 
@@ -307,9 +348,32 @@ pub(super) mod _impl {
     impl ConsumeHunk for Vec<u8> {
         type Out = Self;
 
-        fn consume_hunk(&mut self, _: u32, _: u32, _: u32, _: u32, header: &str, hunk: &[u8]) -> std::io::Result<()> {
-            self.push_str(header);
-            self.push_str(hunk);
+        fn consume_hunk(
+            &mut self,
+            header: HunkHeader,
+            lines: &[(DiffLineType, &[u8])],
+            newline: NewlineSeparator<'_>,
+        ) -> std::io::Result<()> {
+            self.push_str(header.to_string());
+            self.push_str(match newline {
+                NewlineSeparator::AfterHeaderAndLine(nl) | NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => nl,
+            });
+
+            for &(line_type, content) in lines {
+                self.push(line_type.to_prefix() as u8);
+                self.extend_from_slice(content);
+
+                match newline {
+                    NewlineSeparator::AfterHeaderAndLine(nl) => {
+                        self.push_str(nl);
+                    }
+                    NewlineSeparator::AfterHeaderAndWhenNeeded(nl) => {
+                        if !content.ends_with_str(nl) {
+                            self.push_str(nl);
+                        }
+                    }
+                }
+            }
             Ok(())
         }