fix: handle binary files and prioritize binary hunks

packages/tauri/src/git/diff.rs

@@ -93,16 +93,19 @@ fn hunks_by_filepath(
         let old_lines = hunk.as_ref().map_or(0, git2::DiffHunk::old_lines);
 
         if let Some((line, is_binary)) = match line.origin() {
-            '+' | '-' | ' ' => Some((
+            '+' | '-' | ' ' => {
-                format!(
+                if let Ok(content) = str::from_utf8(line.content()) {
-                    "{}{}",
+                    Some((format!("{}{}", line.origin(), content), false))
-                    line.origin(),
+                } else {
-                    str::from_utf8(line.content())
+                    let full_path = repository.workdir().unwrap().join(file_path);
-                        .map_err(|error| tracing::error!(?error, ?file_path))
+                    // save the file_path to the odb
-                        .unwrap_or_default()
+                    if !delta.new_file().id().is_zero() && full_path.exists() {
-                ),
+                        // the binary file wasnt deleted
-                false,
+                        repository.blob_path(full_path.as_path()).unwrap();
-            )),
+                    }
+                    Some((delta.new_file().id().to_string(), true))
+                }
+            }
             'B' => {
                 let full_path = repository.workdir().unwrap().join(file_path);
                 // save the file_path to the odb
@@ -113,26 +116,43 @@ fn hunks_by_filepath(
                 Some((delta.new_file().id().to_string(), true))
             }
             'F' => None,
-            _ => Some((
+            _ => {
-                str::from_utf8(line.content())
+                if let Ok(content) = str::from_utf8(line.content()) {
-                    .map_err(|error| tracing::error!(?error, ?file_path))
+                    Some((content.to_string(), false))
-                    .unwrap_or_default()
+                } else {
-                    .to_string(),
+                    let full_path = repository.workdir().unwrap().join(file_path);
-                false,
+                    // save the file_path to the odb
-            )),
+                    if !delta.new_file().id().is_zero() && full_path.exists() {
+                        // the binary file wasnt deleted
+                        repository.blob_path(full_path.as_path()).unwrap();
+                    }
+                    Some((delta.new_file().id().to_string(), true))
+                }
+            }
         } {
             let hunks = hunks_by_filepath
                 .entry(file_path.to_path_buf())
                 .or_default();
 
-            if let Some(hunk) = hunks.last_mut() {
+            if let Some(previous_hunk) = hunks.last_mut() {
-                if hunk.old_start == old_start
+                let hunk_did_not_change = previous_hunk.old_start == old_start
-                    && hunk.old_lines == old_lines
+                    && previous_hunk.old_lines == old_lines
-                    && hunk.new_start == new_start
+                    && previous_hunk.new_start == new_start
-                    && hunk.new_lines == new_lines
+                    && previous_hunk.new_lines == new_lines;
-                {
+
-                    hunk.diff.push_str(&line);
+                if hunk_did_not_change {
-                    hunk.binary |= is_binary;
+                    if is_binary {
+                        // binary overrides the diff
+                        previous_hunk.binary = true;
+                        previous_hunk.old_start = 0;
+                        previous_hunk.old_lines = 0;
+                        previous_hunk.new_start = 0;
+                        previous_hunk.new_lines = 0;
+                        previous_hunk.diff = line;
+                    } else if !previous_hunk.binary {
+                        // append non binary hunks
+                        previous_hunk.diff.push_str(&line);
+                    }
                 } else {
                     hunks.push(Hunk {
                         old_start,
@@ -162,7 +182,26 @@ fn hunks_by_filepath(
     Ok(hunks_by_filepath
         .into_iter()
         .map(|(k, v)| {
-            if v.is_empty() {
+            if let Some(binary_hunk) = v.iter().find(|hunk| hunk.binary) {
+                if v.len() > 1 {
+                    // if there are multiple hunks with binary among them, then the binary hunk
+                    // takes precedence
+                    (
+                        k,
+                        vec![Hunk {
+                            old_start: 0,
+                            old_lines: 0,
+                            new_start: 0,
+                            new_lines: 0,
+                            diff: binary_hunk.diff.clone(),
+                            binary: true,
+                        }],
+                    )
+                } else {
+                    (k, v)
+                }
+            } else if v.is_empty() {
+                // this is a new file
                 (
                     k,
                     vec![Hunk {
@@ -294,4 +333,46 @@ mod tests {
             }]
         );
     }
+
+    #[test]
+    fn diff_some_lines_are_binary() {
+        let repository = test_utils::test_repository();
+        std::fs::write(
+            repository.workdir().unwrap().join("file"),
+            [
+                // butler/test/fixtures/git/1/8e/18ec9df5-65c5-4828-97ba-d91ec4903a74/objects/1f/9d7d5dd0d3d3ced66cee36bf1dd42bd33d0aa8
+                120, 1, 101, 144, 79, 75, 195, 64, 16, 197, 61, 239, 167, 120, 160, 224, 165, 77, 3,
+                5, 17, 111, 42, 42, 245, 162, 135, 22, 60, 118, 155, 76, 179, 75, 55, 59, 97, 103,
+                182, 177, 223, 222, 77, 244, 38, 204, 97, 254, 188, 247, 155, 97, 14, 129, 15, 88,
+                223, 213, 87, 215, 120, 243, 250, 148, 53, 80, 194, 110, 131, 103, 142, 13, 13, 42,
+                198, 60, 10, 54, 183, 61, 34, 163, 99, 110, 97, 21, 175, 190, 235, 237, 98, 238,
+                102, 241, 177, 195, 214, 250, 48, 250, 216, 66, 25, 71, 223, 229, 68, 224, 172, 24,
+                93, 17, 111, 48, 218, 168, 80, 71, 5, 187, 218, 125, 77, 154, 192, 124, 66, 240,
+                39, 170, 176, 117, 94, 80, 98, 154, 147, 21, 79, 82, 124, 246, 50, 169, 90, 134,
+                215, 9, 36, 190, 45, 192, 35, 62, 131, 189, 116, 137, 115, 108, 23, 56, 20, 190,
+                78, 94, 103, 5, 103, 74, 226, 57, 162, 225, 168, 137, 67, 101, 204, 123, 46, 156,
+                148, 227, 172, 121, 48, 102, 191, 223, 155, 27, 196, 225, 27, 250, 119, 107, 35,
+                130, 165, 71, 181, 242, 113, 200, 90, 205, 37, 151, 82, 199, 223, 124, 57, 90, 109,
+                92, 49, 13, 23, 117, 28, 215, 88, 246, 112, 170, 67, 37, 148, 202, 62, 220, 215,
+                117, 61, 99, 205, 71, 90, 64, 184, 167, 114, 78, 249, 5, 5, 161, 202, 188, 156, 41,
+                162, 79, 76, 255, 38, 63, 226, 30, 123, 106,
+            ],
+        )
+        .unwrap();
+
+        let head_commit_id = repository.head().unwrap().peel_to_commit().unwrap().id();
+
+        let diff = workdir(&repository, &head_commit_id, &Options::default()).unwrap();
+        assert_eq!(
+            diff[&path::PathBuf::from("file")],
+            vec![Hunk {
+                old_start: 0,
+                old_lines: 0,
+                new_start: 0,
+                new_lines: 0,
+                diff: "3fc41b9ae6836a94f41c78b4ce69d78b6e7080f1".to_string(),
+                binary: true,
+            }]
+        );
+    }
 }