From b142a3c25bbd40da63a5419298149cf1b2068e06 Mon Sep 17 00:00:00 2001 From: Nikita Galaiko Date: Wed, 15 Nov 2023 10:21:33 +0100 Subject: [PATCH] fix: handle binary files and prioritize binary hunks --- packages/tauri/src/git/diff.rs | 133 ++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 26 deletions(-) diff --git a/packages/tauri/src/git/diff.rs b/packages/tauri/src/git/diff.rs index 1e768c122..b3d14fa23 100644 --- a/packages/tauri/src/git/diff.rs +++ b/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!( - "{}{}", - line.origin(), - str::from_utf8(line.content()) - .map_err(|error| tracing::error!(?error, ?file_path)) - .unwrap_or_default() - ), - false, - )), + '+' | '-' | ' ' => { + if let Ok(content) = str::from_utf8(line.content()) { + Some((format!("{}{}", line.origin(), content), false)) + } else { + let full_path = repository.workdir().unwrap().join(file_path); + // 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)) + } + } '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()) - .map_err(|error| tracing::error!(?error, ?file_path)) - .unwrap_or_default() - .to_string(), - false, - )), + _ => { + if let Ok(content) = str::from_utf8(line.content()) { + Some((content.to_string(), false)) + } else { + let full_path = repository.workdir().unwrap().join(file_path); + // 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 hunk.old_start == old_start - && hunk.old_lines == old_lines - && hunk.new_start == new_start - && hunk.new_lines == new_lines - { - hunk.diff.push_str(&line); - hunk.binary |= is_binary; + if let Some(previous_hunk) = hunks.last_mut() { + let hunk_did_not_change = previous_hunk.old_start == old_start + && previous_hunk.old_lines == old_lines + && previous_hunk.new_start == new_start + && previous_hunk.new_lines == new_lines; + + if hunk_did_not_change { + 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, + }] + ); + } }