From 2d967ef97588fe2741e3074310b2e92498196d61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 19:27:07 +0200 Subject: [PATCH 01/15] Use gitoxide in get_diff --- asyncgit/src/error.rs | 51 +++++++++++ asyncgit/src/sync/diff.rs | 185 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 229 insertions(+), 7 deletions(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index e36040d814..3c38e65733 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -7,6 +7,25 @@ use thiserror::Error; /// #[derive(Error, Debug)] pub enum GixError { + #[error("gix::config::diff::algorithm::Error error: {0}")] + ConfigDiffAlgorithm(#[from] gix::config::diff::algorithm::Error), + + /// + #[error( + "gix::diff::blob::platform::prepare_diff::Error error: {0}" + )] + DiffBlobPlatformPrepareDiff( + #[from] gix::diff::blob::platform::prepare_diff::Error, + ), + + /// + #[error( + "gix::diff::blob::platform::set_resource::Error error: {0}" + )] + DiffBlobPlatformSetResource( + #[from] gix::diff::blob::platform::set_resource::Error, + ), + /// #[error("gix::discover error: {0}")] Discover(#[from] Box), @@ -47,6 +66,14 @@ pub enum GixError { #[error("gix::reference::iter::init::Error error: {0}")] ReferenceIterInit(#[from] gix::reference::iter::init::Error), + /// + #[error( + "gix::repository::diff_resource_cache::Error error: {0}" + )] + RepositoryDiffResourceCache( + #[from] gix::repository::diff_resource_cache::Error, + ), + /// #[error("gix::revision::walk error: {0}")] RevisionWalk(#[from] gix::revision::walk::Error), @@ -207,6 +234,22 @@ impl From for GixError { } } +impl From for Error { + fn from( + error: gix::diff::blob::platform::prepare_diff::Error, + ) -> Self { + Self::Gix(GixError::from(error)) + } +} + +impl From for Error { + fn from( + error: gix::diff::blob::platform::set_resource::Error, + ) -> Self { + Self::Gix(GixError::from(error)) + } +} + impl From for Error { fn from(error: gix::discover::Error) -> Self { Self::Gix(GixError::from(error)) @@ -271,6 +314,14 @@ impl From for Error { } } +impl From for Error { + fn from( + error: gix::repository::diff_resource_cache::Error, + ) -> Self { + Self::Gix(GixError::from(error)) + } +} + impl From for Error { fn from(error: gix::revision::walk::Error) -> Self { Self::Gix(GixError::from(error)) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index d02a00a121..8fdd037f29 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -8,15 +8,22 @@ use super::{ CommitId, RepoPath, }; use crate::{ - error::Error, - error::Result, + error::{Error, Result}, hash, - sync::{get_stashes, repository::repo}, + sync::{get_stashes, gix_repo, repository::repo}, }; use easy_cast::Conv; use git2::{ Delta, Diff, DiffDelta, DiffFormat, DiffHunk, Patch, Repository, }; +use gix::{ + bstr::ByteSlice, + diff::blob::{ + unified_diff::{ConsumeHunk, ContextSize, NewlineSeparator}, + UnifiedDiff, + }, + ObjectId, +}; use scopetime::scope_time; use serde::{Deserialize, Serialize}; use std::{cell::RefCell, fs, path::Path, rc::Rc}; @@ -200,6 +207,62 @@ pub(crate) fn get_diff_raw<'a>( Ok(diff) } +pub(crate) fn tokens_for_diffing( + data: &[u8], +) -> impl gix::diff::blob::intern::TokenSource { + gix::diff::blob::sources::byte_lines(data) +} + +impl ConsumeHunk for FileDiff { + type Out = Self; + + 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], + ) -> std::io::Result<()> { + let lines = hunk + .lines() + .enumerate() + .map(|(index, line)| DiffLine { + position: DiffLinePosition { + old_lineno: Some( + before_hunk_start + index as u32, + ), + new_lineno: Some(after_hunk_start + index as u32), + }, + content: String::from_utf8_lossy(line) + .trim_matches(is_newline) + .into(), + // TODO: + // Get correct `line_type`. We could potentially do this by looking at the line's first + // character. We probably want to split it anyway as `gitui` takes care of that character + // itself later in the UI. + line_type: DiffLineType::default(), + }) + .collect(); + + self.hunks.push(Hunk { + // TODO: + // Get correct `header_hash`. + header_hash: 0, + lines, + }); + + self.lines += hunk.lines().count(); + + Ok(()) + } + + fn finish(self) -> Self::Out { + self + } +} + /// returns diff of a specific file either in `stage` or workdir pub fn get_diff( repo_path: &RepoPath, @@ -207,13 +270,121 @@ pub fn get_diff( stage: bool, options: Option, ) -> Result { + // TODO: + // Maybe move this `use` statement` closer to where it is being used by extracting the relevant + // code into a function. + use gix::diff::blob::platform::prepare_diff::Operation; + scope_time!("get_diff"); - let repo = repo(repo_path)?; - let work_dir = work_dir(&repo)?; - let diff = get_diff_raw(&repo, p, stage, false, options)?; + let mut gix_repo = gix_repo(repo_path)?; + gix_repo.object_cache_size_if_unset( + gix_repo.compute_object_cache_size_for_tree_diffs( + &**gix_repo.index_or_empty()?, + ), + ); + + let old_revspec = format!("HEAD:{p}"); + + // TODO: + // This is identical to the corresponding block that gets `(new_blob_id, new_root)`. + let (old_blob_id, old_root) = + gix_repo.rev_parse(&*old_revspec).map_or_else( + |_| { + ( + ObjectId::null(gix::hash::Kind::Sha1), + gix_repo.workdir().map(ToOwned::to_owned), + ) + }, + |resolved_revspec| { + resolved_revspec.single().map_or_else( + || (ObjectId::null(gix::hash::Kind::Sha1), None), + |id| (id.into(), None), + ) + }, + ); + + // TODO: + // Make sure that the revspec logic is correct, i. e. uses the correct syntax for all the + // relevant cases. + let new_revspec = if stage { + format!(":{p}") + } else { + p.to_string() + }; - raw_diff_to_file_diff(&diff, work_dir) + let (new_blob_id, new_root) = + gix_repo.rev_parse(&*new_revspec).map_or_else( + |_| { + ( + ObjectId::null(gix::hash::Kind::Sha1), + gix_repo.workdir().map(ToOwned::to_owned), + ) + }, + |resolved_revspec| { + resolved_revspec.single().map_or_else( + || (ObjectId::null(gix::hash::Kind::Sha1), None), + |id| (id.into(), None), + ) + }, + ); + + let worktree_roots = gix::diff::blob::pipeline::WorktreeRoots { + old_root, + new_root, + }; + + let mut resource_cache = gix_repo.diff_resource_cache(gix::diff::blob::pipeline::Mode::ToGitUnlessBinaryToTextIsPresent, worktree_roots)?; + + resource_cache.set_resource( + old_blob_id, + gix::object::tree::EntryKind::Blob, + p.as_ref(), + gix::diff::blob::ResourceKind::OldOrSource, + &gix_repo.objects, + )?; + resource_cache.set_resource( + new_blob_id, + gix::object::tree::EntryKind::Blob, + p.as_ref(), + gix::diff::blob::ResourceKind::NewOrDestination, + &gix_repo.objects, + )?; + + let outcome = resource_cache.prepare_diff()?; + + let diff_algorithm = match outcome.operation { + Operation::InternalDiff { algorithm } => algorithm, + Operation::ExternalCommand { .. } => { + unreachable!("We disabled that") + } + Operation::SourceOrDestinationIsBinary => { + todo!(); + } + }; + + let input = gix::diff::blob::intern::InternedInput::new( + tokens_for_diffing( + outcome.old.data.as_slice().unwrap_or_default(), + ), + tokens_for_diffing( + outcome.new.data.as_slice().unwrap_or_default(), + ), + ); + + let unified_diff = UnifiedDiff::new( + &input, + FileDiff::default(), + // TODO: + // Get values from `options` where necessary and possible. + NewlineSeparator::AfterHeaderAndLine("\n"), + ContextSize::symmetrical(3), + ); + + let file_diff = + gix::diff::blob::diff(diff_algorithm, &input, unified_diff)?; + + Ok(file_diff) } /// returns diff of a specific file inside a commit From f0b512cd2363f021470af022adecb4dcc59cad3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 20:54:13 +0200 Subject: [PATCH 02/15] Extract resolve_revspec --- asyncgit/src/sync/diff.rs | 53 ++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 8fdd037f29..b7a9aaa277 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -263,6 +263,26 @@ impl ConsumeHunk for FileDiff { } } +fn resolve_revspec( + gix_repo: &gix::Repository, + revspec: &str, +) -> (ObjectId, Option) { + gix_repo.rev_parse(revspec).map_or_else( + |_| { + ( + ObjectId::null(gix::hash::Kind::Sha1), + gix_repo.workdir().map(ToOwned::to_owned), + ) + }, + |resolved_revspec| { + resolved_revspec.single().map_or_else( + || (ObjectId::null(gix::hash::Kind::Sha1), None), + |id| (id.into(), None), + ) + }, + ) +} + /// returns diff of a specific file either in `stage` or workdir pub fn get_diff( repo_path: &RepoPath, @@ -285,24 +305,8 @@ pub fn get_diff( ); let old_revspec = format!("HEAD:{p}"); - - // TODO: - // This is identical to the corresponding block that gets `(new_blob_id, new_root)`. let (old_blob_id, old_root) = - gix_repo.rev_parse(&*old_revspec).map_or_else( - |_| { - ( - ObjectId::null(gix::hash::Kind::Sha1), - gix_repo.workdir().map(ToOwned::to_owned), - ) - }, - |resolved_revspec| { - resolved_revspec.single().map_or_else( - || (ObjectId::null(gix::hash::Kind::Sha1), None), - |id| (id.into(), None), - ) - }, - ); + resolve_revspec(&gix_repo, &old_revspec); // TODO: // Make sure that the revspec logic is correct, i. e. uses the correct syntax for all the @@ -314,20 +318,7 @@ pub fn get_diff( }; let (new_blob_id, new_root) = - gix_repo.rev_parse(&*new_revspec).map_or_else( - |_| { - ( - ObjectId::null(gix::hash::Kind::Sha1), - gix_repo.workdir().map(ToOwned::to_owned), - ) - }, - |resolved_revspec| { - resolved_revspec.single().map_or_else( - || (ObjectId::null(gix::hash::Kind::Sha1), None), - |id| (id.into(), None), - ) - }, - ); + resolve_revspec(&gix_repo, &new_revspec); let worktree_roots = gix::diff::blob::pipeline::WorktreeRoots { old_root, From 296e8dd16691fc3f64c12b17785f5fa5d5478757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:02:47 +0200 Subject: [PATCH 03/15] Calculate hash from HunkHeader --- asyncgit/src/sync/diff.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index b7a9aaa277..a947902182 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -219,9 +219,9 @@ impl ConsumeHunk for FileDiff { fn consume_hunk( &mut self, before_hunk_start: u32, - _before_hunk_len: u32, + before_hunk_len: u32, after_hunk_start: u32, - _after_hunk_len: u32, + after_hunk_len: u32, header: &str, hunk: &[u8], ) -> std::io::Result<()> { @@ -246,10 +246,15 @@ impl ConsumeHunk for FileDiff { }) .collect(); + let hunk_header = HunkHeader { + old_start: before_hunk_start, + old_lines: before_hunk_len, + new_start: after_hunk_start, + new_lines: after_hunk_len, + }; + self.hunks.push(Hunk { - // TODO: - // Get correct `header_hash`. - header_hash: 0, + header_hash: hash(&hunk_header), lines, }); From 3e7a8eec927bce53fa80867f5a3acc857662e70f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:07:24 +0200 Subject: [PATCH 04/15] Replace enumerate by zip to avoid cast --- asyncgit/src/sync/diff.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index a947902182..4b2acf3281 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -227,13 +227,11 @@ impl ConsumeHunk for FileDiff { ) -> std::io::Result<()> { let lines = hunk .lines() - .enumerate() - .map(|(index, line)| DiffLine { + .zip(0..) + .map(|(line, index)| DiffLine { position: DiffLinePosition { - old_lineno: Some( - before_hunk_start + index as u32, - ), - new_lineno: Some(after_hunk_start + index as u32), + old_lineno: Some(before_hunk_start + index), + new_lineno: Some(after_hunk_start + index), }, content: String::from_utf8_lossy(line) .trim_matches(is_newline) From 711ddca942abcd5edda75bf5c0927c98a4c6cfaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:37:24 +0200 Subject: [PATCH 05/15] Use first character to determine line_type --- asyncgit/src/sync/diff.rs | 81 ++++++++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 14 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 4b2acf3281..840fa26039 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -227,21 +227,74 @@ impl ConsumeHunk for FileDiff { ) -> std::io::Result<()> { let lines = hunk .lines() - .zip(0..) - .map(|(line, index)| DiffLine { - position: DiffLinePosition { - old_lineno: Some(before_hunk_start + index), - new_lineno: Some(after_hunk_start + index), + .scan( + (before_hunk_start, after_hunk_start), + |(old_lineno, new_lineno), line| { + let (line_type, content, old_lineno, new_lineno) = + match line { + [b'+', rest @ ..] => { + let result = ( + DiffLineType::Add, + rest, + None, + Some(*new_lineno), + ); + *new_lineno += 1; + result + } + [b'-', rest @ ..] => { + let result = ( + DiffLineType::Delete, + rest, + Some(*old_lineno), + None, + ); + *old_lineno += 1; + result + } + [b' ', rest @ ..] => { + let result = ( + DiffLineType::None, + rest, + Some(*old_lineno), + Some(*new_lineno), + ); + *old_lineno += 1; + *new_lineno += 1; + result + } + [b'@', ..] => ( + DiffLineType::Header, + line, + None, + None, + ), + _ => { + // Empty lines or unknown prefixes are treated as context. + let result = ( + DiffLineType::None, + line, + Some(*old_lineno), + Some(*new_lineno), + ); + *old_lineno += 1; + *new_lineno += 1; + result + } + }; + + Some(DiffLine { + position: DiffLinePosition { + old_lineno, + new_lineno, + }, + content: String::from_utf8_lossy(content) + .trim_matches(is_newline) + .into(), + line_type, + }) }, - content: String::from_utf8_lossy(line) - .trim_matches(is_newline) - .into(), - // TODO: - // Get correct `line_type`. We could potentially do this by looking at the line's first - // character. We probably want to split it anyway as `gitui` takes care of that character - // itself later in the UI. - line_type: DiffLineType::default(), - }) + ) .collect(); let hunk_header = HunkHeader { From ca5ab566dc89cdf5836d83e3be948f74f5348802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:38:38 +0200 Subject: [PATCH 06/15] Use correct revspec when stage is false --- asyncgit/src/sync/diff.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 840fa26039..367d8a5b0f 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -360,7 +360,11 @@ pub fn get_diff( ), ); - let old_revspec = format!("HEAD:{p}"); + let old_revspec = if stage { + format!("HEAD:{p}") + } else { + format!(":{p}") + }; let (old_blob_id, old_root) = resolve_revspec(&gix_repo, &old_revspec); From 75dd3cd60c08c80578608881bf8195a992a7b2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:43:33 +0200 Subject: [PATCH 07/15] Move use statement to where the type is used --- asyncgit/src/sync/diff.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 367d8a5b0f..6a16846933 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -346,11 +346,6 @@ pub fn get_diff( stage: bool, options: Option, ) -> Result { - // TODO: - // Maybe move this `use` statement` closer to where it is being used by extracting the relevant - // code into a function. - use gix::diff::blob::platform::prepare_diff::Operation; - scope_time!("get_diff"); let mut gix_repo = gix_repo(repo_path)?; @@ -404,13 +399,17 @@ pub fn get_diff( let outcome = resource_cache.prepare_diff()?; - let diff_algorithm = match outcome.operation { - Operation::InternalDiff { algorithm } => algorithm, - Operation::ExternalCommand { .. } => { - unreachable!("We disabled that") - } - Operation::SourceOrDestinationIsBinary => { - todo!(); + let diff_algorithm = { + use gix::diff::blob::platform::prepare_diff::Operation; + + match outcome.operation { + Operation::InternalDiff { algorithm } => algorithm, + Operation::ExternalCommand { .. } => { + unreachable!("We disabled that") + } + Operation::SourceOrDestinationIsBinary => { + todo!(); + } } }; From 3e97552917e411623f5c067bd5029467f7875f68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 31 Jul 2025 21:48:55 +0200 Subject: [PATCH 08/15] Get context_size from options --- asyncgit/src/sync/diff.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 6a16846933..cb0072156a 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -422,13 +422,13 @@ pub fn get_diff( ), ); + let context_size = options.map_or(3, |opts| opts.context); + let unified_diff = UnifiedDiff::new( &input, FileDiff::default(), - // TODO: - // Get values from `options` where necessary and possible. NewlineSeparator::AfterHeaderAndLine("\n"), - ContextSize::symmetrical(3), + ContextSize::symmetrical(context_size), ); let file_diff = From 2922c49971a1e5858eae8a33e5a794c08e176176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Sun, 3 Aug 2025 16:57:04 +0200 Subject: [PATCH 09/15] Merge revspec-related blocks --- asyncgit/src/sync/diff.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index cb0072156a..ef84ef664c 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -355,23 +355,17 @@ pub fn get_diff( ), ); - let old_revspec = if stage { - format!("HEAD:{p}") - } else { - format!(":{p}") - }; - let (old_blob_id, old_root) = - resolve_revspec(&gix_repo, &old_revspec); - // TODO: // Make sure that the revspec logic is correct, i. e. uses the correct syntax for all the // relevant cases. - let new_revspec = if stage { - format!(":{p}") + let (old_revspec, new_revspec) = if stage { + (format!("HEAD:{p}"), format!(":{p}")) } else { - p.to_string() + (format!(":{p}"), p.to_string()) }; + let (old_blob_id, old_root) = + resolve_revspec(&gix_repo, &old_revspec); let (new_blob_id, new_root) = resolve_revspec(&gix_repo, &new_revspec); From 62744a0839ac7da339c54d5452b1e5a75895b984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 14:24:21 +0200 Subject: [PATCH 10/15] Remove revspec workaround --- asyncgit/src/error.rs | 10 ++++++ asyncgit/src/sync/diff.rs | 64 +++++++++++++++++++++------------------ 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index 3c38e65733..17f7a5575d 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -54,6 +54,10 @@ pub enum GixError { #[from] gix::reference::find::existing::Error, ), + /// + #[error("gix::reference::head_tree::Error error: {0}")] + ReferenceHeadTree(#[from] gix::reference::head_tree::Error), + /// #[error("gix::reference::head_tree_id::Error error: {0}")] ReferenceHeadTreeId(#[from] gix::reference::head_tree_id::Error), @@ -296,6 +300,12 @@ impl From for Error { } } +impl From for Error { + fn from(error: gix::reference::head_tree::Error) -> Self { + Self::Gix(GixError::from(error)) + } +} + impl From for Error { fn from(error: gix::reference::head_tree_id::Error) -> Self { Self::Gix(GixError::from(error)) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index ef84ef664c..f634b5aa5a 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -319,26 +319,6 @@ impl ConsumeHunk for FileDiff { } } -fn resolve_revspec( - gix_repo: &gix::Repository, - revspec: &str, -) -> (ObjectId, Option) { - gix_repo.rev_parse(revspec).map_or_else( - |_| { - ( - ObjectId::null(gix::hash::Kind::Sha1), - gix_repo.workdir().map(ToOwned::to_owned), - ) - }, - |resolved_revspec| { - resolved_revspec.single().map_or_else( - || (ObjectId::null(gix::hash::Kind::Sha1), None), - |id| (id.into(), None), - ) - }, - ) -} - /// returns diff of a specific file either in `stage` or workdir pub fn get_diff( repo_path: &RepoPath, @@ -356,18 +336,42 @@ pub fn get_diff( ); // TODO: - // Make sure that the revspec logic is correct, i. e. uses the correct syntax for all the - // relevant cases. - let (old_revspec, new_revspec) = if stage { - (format!("HEAD:{p}"), format!(":{p}")) + // The lower tree is `stage == true`, the upper tree is `stage == false`. + let (old_blob_id, old_root) = if stage { + ( + gix_repo + .head_tree()? + .lookup_entry_by_path(p) + .expect("TODO") + .expect("TODO") + .object_id(), + None, + ) } else { - (format!(":{p}"), p.to_string()) + ( + gix_repo + .index()? + .entry_by_path(p.into()) + .expect("TODO") + .id, + None, + ) + }; + let (new_blob_id, new_root) = if stage { + ( + gix_repo + .index()? + .entry_by_path(p.into()) + .expect("TODO") + .id, + None, + ) + } else { + ( + ObjectId::null(gix::hash::Kind::Sha1), + gix_repo.workdir().map(ToOwned::to_owned), + ) }; - - let (old_blob_id, old_root) = - resolve_revspec(&gix_repo, &old_revspec); - let (new_blob_id, new_root) = - resolve_revspec(&gix_repo, &new_revspec); let worktree_roots = gix::diff::blob::pipeline::WorktreeRoots { old_root, From da5bd29f190205a926733316acc7f4c2c832a605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 15:12:21 +0200 Subject: [PATCH 11/15] Handle binary files --- asyncgit/src/sync/diff.rs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index f634b5aa5a..7fd7a4a50d 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -319,6 +319,35 @@ impl ConsumeHunk for FileDiff { } } +fn file_diff_for_binary_files( + old_data: gix::diff::blob::platform::resource::Data, + new_data: gix::diff::blob::platform::resource::Data, +) -> FileDiff { + use gix::diff::blob::platform::resource::Data; + + let mut file_diff = FileDiff::default(); + + let old_size = match old_data { + Data::Missing => 0, + Data::Buffer { buf, .. } => u64::conv(buf.len()), + Data::Binary { size } => size, + }; + let new_size = match new_data { + Data::Missing => 0, + Data::Buffer { buf, .. } => u64::conv(buf.len()), + Data::Binary { size } => size, + }; + + let sizes = (old_size, new_size); + let size_delta = + (i64::conv(new_size)).saturating_sub(i64::conv(old_size)); + + file_diff.sizes = sizes; + file_diff.size_delta = size_delta; + + file_diff +} + /// returns diff of a specific file either in `stage` or workdir pub fn get_diff( repo_path: &RepoPath, @@ -342,9 +371,6 @@ pub fn get_diff( gix_repo .head_tree()? .lookup_entry_by_path(p) - .expect("TODO") - .expect("TODO") - .object_id(), None, ) } else { @@ -406,7 +432,10 @@ pub fn get_diff( unreachable!("We disabled that") } Operation::SourceOrDestinationIsBinary => { - todo!(); + return Ok(file_diff_for_binary_files( + outcome.old.data, + outcome.new.data, + )); } } }; From ddd7630874c31145fe24f90be93c243afaca9521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 15:12:47 +0200 Subject: [PATCH 12/15] Handle files not existing --- asyncgit/src/sync/diff.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 7fd7a4a50d..c2b50c1445 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -371,25 +371,30 @@ pub fn get_diff( gix_repo .head_tree()? .lookup_entry_by_path(p) + .map(|entry| { + entry.map_or_else( + || ObjectId::null(gix::hash::Kind::Sha1), + |entry| entry.object_id(), + ) + }) + .unwrap_or(ObjectId::null(gix::hash::Kind::Sha1)), None, ) } else { ( - gix_repo - .index()? - .entry_by_path(p.into()) - .expect("TODO") - .id, + gix_repo.index()?.entry_by_path(p.into()).map_or( + ObjectId::null(gix::hash::Kind::Sha1), + |entry| entry.id, + ), None, ) }; let (new_blob_id, new_root) = if stage { ( - gix_repo - .index()? - .entry_by_path(p.into()) - .expect("TODO") - .id, + gix_repo.index()?.entry_by_path(p.into()).map_or( + ObjectId::null(gix::hash::Kind::Sha1), + |entry| entry.id, + ), None, ) } else { From 5c3a513908f8dc27a7666d39d34c6e3f040516f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 15:27:38 +0200 Subject: [PATCH 13/15] Box large error variant --- asyncgit/src/error.rs | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index 17f7a5575d..544572bb37 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -202,7 +202,7 @@ pub enum Error { /// #[error("gix error:{0}")] - Gix(#[from] GixError), + Gix(#[from] Box), /// #[error("amend error: config commit.gpgsign=true detected.\ngpg signing is not supported for amending non-last commits")] @@ -242,7 +242,7 @@ impl From for Error { fn from( error: gix::diff::blob::platform::prepare_diff::Error, ) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -250,19 +250,19 @@ impl From for Error { fn from( error: gix::diff::blob::platform::set_resource::Error, ) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::discover::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::head::peel::to_commit::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -272,13 +272,13 @@ impl From fn from( error: gix::object::find::existing::with_conversion::Error, ) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::objs::decode::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -290,37 +290,37 @@ impl From for GixError { impl From for Error { fn from(error: gix::pathspec::init::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::reference::find::existing::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::reference::head_tree::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::reference::head_tree_id::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::reference::iter::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::reference::iter::init::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -328,13 +328,13 @@ impl From for Error { fn from( error: gix::repository::diff_resource_cache::Error, ) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } impl From for Error { fn from(error: gix::revision::walk::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -346,7 +346,7 @@ impl From for GixError { impl From for Error { fn from(error: gix::status::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -358,7 +358,7 @@ impl From for GixError { impl From for Error { fn from(error: gix::status::iter::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -370,7 +370,7 @@ impl From for GixError { impl From for Error { fn from(error: gix::status::into_iter::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -382,7 +382,7 @@ impl From for GixError { impl From for Error { fn from(error: gix::status::index_worktree::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -394,7 +394,7 @@ impl From for GixError { impl From for Error { fn from(error: gix::status::tree_index::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } @@ -406,6 +406,6 @@ impl From for GixError { impl From for Error { fn from(error: gix::worktree::open_index::Error) -> Self { - Self::Gix(GixError::from(error)) + Self::Gix(Box::new(GixError::from(error))) } } From 25ec30e10652e13106a8fc3d97d4269642bca92c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 17:33:39 +0200 Subject: [PATCH 14/15] Handle index or head not existing --- asyncgit/src/sync/diff.rs | 69 +++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index c2b50c1445..1e3614c7b2 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -26,7 +26,12 @@ use gix::{ }; use scopetime::scope_time; use serde::{Deserialize, Serialize}; -use std::{cell::RefCell, fs, path::Path, rc::Rc}; +use std::{ + cell::RefCell, + fs, + path::{Path, PathBuf}, + rc::Rc, +}; /// type of diff of a single line #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] @@ -319,6 +324,38 @@ impl ConsumeHunk for FileDiff { } } +fn id_and_root_in_head( + gix_repo: &gix::Repository, + p: &str, +) -> (ObjectId, Option) { + let id = gix_repo + .head_tree() + .map_or(None, |head_tree| { + head_tree + .lookup_entry_by_path(p) + .ok() + .flatten() + .map(|entry| entry.object_id()) + }) + .unwrap_or(ObjectId::null(gix::hash::Kind::Sha1)); + + (id, None) +} + +fn id_and_root_in_index( + gix_repo: &gix::Repository, + p: &str, +) -> (ObjectId, Option) { + let id = gix_repo + .index() + .map_or(None, |index| { + index.entry_by_path(p.into()).map(|entry| entry.id) + }) + .unwrap_or(ObjectId::null(gix::hash::Kind::Sha1)); + + (id, None) +} + fn file_diff_for_binary_files( old_data: gix::diff::blob::platform::resource::Data, new_data: gix::diff::blob::platform::resource::Data, @@ -367,36 +404,12 @@ pub fn get_diff( // TODO: // The lower tree is `stage == true`, the upper tree is `stage == false`. let (old_blob_id, old_root) = if stage { - ( - gix_repo - .head_tree()? - .lookup_entry_by_path(p) - .map(|entry| { - entry.map_or_else( - || ObjectId::null(gix::hash::Kind::Sha1), - |entry| entry.object_id(), - ) - }) - .unwrap_or(ObjectId::null(gix::hash::Kind::Sha1)), - None, - ) + id_and_root_in_head(&gix_repo, p) } else { - ( - gix_repo.index()?.entry_by_path(p.into()).map_or( - ObjectId::null(gix::hash::Kind::Sha1), - |entry| entry.id, - ), - None, - ) + id_and_root_in_index(&gix_repo, p) }; let (new_blob_id, new_root) = if stage { - ( - gix_repo.index()?.entry_by_path(p.into()).map_or( - ObjectId::null(gix::hash::Kind::Sha1), - |entry| entry.id, - ), - None, - ) + id_and_root_in_index(&gix_repo, p) } else { ( ObjectId::null(gix::hash::Kind::Sha1), From 21c7e37738de434a965b108994b79bf8d5053067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Fri, 8 Aug 2025 19:06:34 +0200 Subject: [PATCH 15/15] Include header in diff lines --- asyncgit/src/sync/diff.rs | 139 +++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 69 deletions(-) diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 1e3614c7b2..11f7cadd71 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -230,77 +230,78 @@ impl ConsumeHunk for FileDiff { header: &str, hunk: &[u8], ) -> std::io::Result<()> { - let lines = hunk - .lines() - .scan( - (before_hunk_start, after_hunk_start), - |(old_lineno, new_lineno), line| { - let (line_type, content, old_lineno, new_lineno) = - match line { - [b'+', rest @ ..] => { - let result = ( - DiffLineType::Add, - rest, - None, - Some(*new_lineno), - ); - *new_lineno += 1; - result - } - [b'-', rest @ ..] => { - let result = ( - DiffLineType::Delete, - rest, - Some(*old_lineno), - None, - ); - *old_lineno += 1; - result - } - [b' ', rest @ ..] => { - let result = ( - DiffLineType::None, - rest, - Some(*old_lineno), - Some(*new_lineno), - ); - *old_lineno += 1; - *new_lineno += 1; - result - } - [b'@', ..] => ( - DiffLineType::Header, - line, + let non_header_lines = hunk.lines().scan( + (before_hunk_start, after_hunk_start), + |(old_lineno, new_lineno), line| { + let (line_type, content, old_lineno, new_lineno) = + match line { + [b'+', rest @ ..] => { + let result = ( + DiffLineType::Add, + rest, None, + Some(*new_lineno), + ); + *new_lineno += 1; + result + } + [b'-', rest @ ..] => { + let result = ( + DiffLineType::Delete, + rest, + Some(*old_lineno), None, - ), - _ => { - // Empty lines or unknown prefixes are treated as context. - let result = ( - DiffLineType::None, - line, - Some(*old_lineno), - Some(*new_lineno), - ); - *old_lineno += 1; - *new_lineno += 1; - result - } - }; - - Some(DiffLine { - position: DiffLinePosition { - old_lineno, - new_lineno, - }, - content: String::from_utf8_lossy(content) - .trim_matches(is_newline) - .into(), - line_type, - }) - }, - ) - .collect(); + ); + *old_lineno += 1; + result + } + [b' ', rest @ ..] => { + let result = ( + DiffLineType::None, + rest, + Some(*old_lineno), + Some(*new_lineno), + ); + *old_lineno += 1; + *new_lineno += 1; + result + } + _ => { + // Empty lines or unknown prefixes are treated as context. + let result = ( + DiffLineType::None, + line, + Some(*old_lineno), + Some(*new_lineno), + ); + *old_lineno += 1; + *new_lineno += 1; + result + } + }; + + Some(DiffLine { + position: DiffLinePosition { + old_lineno, + new_lineno, + }, + content: String::from_utf8_lossy(content) + .trim_matches(is_newline) + .into(), + line_type, + }) + }, + ); + + let mut lines = vec![DiffLine { + content: header.to_string().into(), + line_type: DiffLineType::Header, + position: DiffLinePosition { + old_lineno: None, + new_lineno: None, + }, + }]; + lines.extend(non_header_lines); let hunk_header = HunkHeader { old_start: before_hunk_start,