More Git Dialog UI cleanup

backend/app/src/actors/git.rs

@@ -90,26 +90,20 @@ impl Handler<ListCommits> for GitActor {
     }
 }
 
-#[message(response = Result<Vec<git2_ox::Diff>, git2_ox::error::Error>)]
-pub struct ListDiffs {
+#[message(response = Result<git2_ox::Diff, git2_ox::error::Error>)]
+pub struct GetDiff {
     pub base_rev: Option<String>,
     pub head_rev: Option<String>,
 }
 
-impl Handler<ListDiffs> for GitActor {
+impl Handler<GetDiff> for GitActor {
     async fn handle(
         &mut self,
         _ctx: &mut Context<Self>,
-        msg: ListDiffs,
-    ) -> Result<Vec<git2_ox::Diff>, git2_ox::error::Error> {
-        let diffs_iter = self
-            .repository
-            .iter_diffs_between_revisions(msg.base_rev.as_deref(), msg.head_rev.as_deref())?;
-        let mut diffs = Vec::new();
-        for diff_result in diffs_iter {
-            diffs.push(diff_result?);
-        }
-        Ok(diffs)
+        msg: GetDiff,
+    ) -> Result<git2_ox::Diff, git2_ox::error::Error> {
+        self.repository
+            .diff(msg.base_rev.as_deref(), msg.head_rev.as_deref())
     }
 }
 

backend/app/src/web/api/v1/git.rs

@@ -13,14 +13,14 @@ pub fn router() -> routing::Router<web::AppState> {
             routing::get(get_revision).post(checkout_revision),
         )
         .route("/commits", routing::get(list_commits))
-        .route("/diffs", routing::get(list_diffs))
+        .route("/diff", routing::get(get_diff))
         .route("/tags", routing::get(list_tags).post(create_tag))
         .route("/branches", routing::get(list_branches).post(create_branch))
         .route("/repository/status", routing::get(get_repository_status))
 }
 
 #[derive(utoipa::OpenApi)]
-#[openapi(paths(get_revision, checkout_revision, list_commits,  list_tags, create_tag, list_branches, create_branch, get_repository_status, list_diffs), tags((name = "Git Repository", description="Git Repository related endpoints")) )]
+#[openapi(paths(get_revision, checkout_revision, list_commits,  list_tags, create_tag, list_branches, create_branch, get_repository_status, get_diff), tags((name = "Git Repository", description="Git Repository related endpoints")) )]
 pub(super) struct ApiDoc;
 
 #[utoipa::path(
@@ -149,12 +149,12 @@ struct CommitRangeQuery {
 #[serde(rename_all = "camelCase")]
 struct ListDiffsResponse {
     /// Array of diffs in this commit range
-    diffs: Vec<git2_ox::Diff>,
+    diff: git2_ox::Diff,
 }
 
 #[utoipa::path(
     get,
-    path = "/diffs",
+    path = "/diff",
     summary = "List diffs",
     description = "List the diffs in a commit range",
     params(CommitRangeQuery),
@@ -163,17 +163,17 @@ struct ListDiffsResponse {
         (status = http::StatusCode::INTERNAL_SERVER_ERROR, description = "Internal server error", body = api::ApiStatusDetailResponse),
     )
 )]
-async fn list_diffs(
+async fn get_diff(
     State(state): State<web::AppState>,
     Query(query): Query<CommitRangeQuery>,
 ) -> Result<Json<ListDiffsResponse>, api::AppError> {
     let actor = state.git_actor();
-    let msg = actors::git::ListDiffs {
+    let msg = actors::git::GetDiff {
         base_rev: query.base_rev,
         head_rev: query.head_rev,
     };
-    let diffs = actor.call(msg).await??;
-    Ok(Json(ListDiffsResponse { diffs }))
+    let diff = actor.call(msg).await??;
+    Ok(Json(ListDiffsResponse { diff }))
 }
 
 #[derive(ToSchema, Serialize, Deserialize, IntoParams)]

backend/git2-ox/src/diff.rs

@@ -1,106 +1,118 @@
-#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))]
-#[cfg_attr(
-    feature = "serde",
-    derive(serde::Serialize),
-    serde(rename_all = "lowercase")
-)]
-#[derive(PartialEq)]
-enum DiffKind {
-    Binary,
-    Text,
-}
+use std::collections::hash_map;
+
+use crate::{Result, error};
 
 #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))]
 #[cfg_attr(
     feature = "serde",
     derive(serde::Serialize),
     serde(rename_all = "camelCase")
 )]
-struct DiffFile {
-    /// Path to the diff file
-    path: Option<String>,
-    /// Content of the diff file
-    content: Option<String>,
+pub(crate) struct DiffStats {
+    /// Number of files changed
+    files_changed: usize,
+    /// Number of insertions
+    insertions: usize,
+    /// Number of deletions
+    deletions: usize,
+    /// Number of lines in the old versions of all affected files
+    total_old_num_lines: usize,
 }
 
-impl DiffFile {
-    pub fn try_from_repo_and_diff_file(
-        repo: &git2::Repository,
-        diff_file: &git2::DiffFile,
-    ) -> Option<DiffFile> {
-        let oid = diff_file.id();
-        if oid.is_zero() {
-            return None;
+impl DiffStats {
+    fn from_stats_and_total_old_num_lines(
+        stats: &git2::DiffStats,
+        total_old_num_lines: usize,
+    ) -> Self {
+        Self {
+            files_changed: stats.files_changed(),
+            insertions: stats.insertions(),
+            deletions: stats.deletions(),
+            total_old_num_lines,
         }
-
-        let content = match repo.find_blob(oid) {
-            Ok(blob) if diff_file.is_not_binary() => {
-                Some(String::from_utf8_lossy(blob.content()).into_owned())
-            }
-            _ => {
-                log::warn!("Did not find blob for {:?}", diff_file.path());
-                None
-            }
-        };
-
-        Some(DiffFile {
-            path: diff_file
-                .path()
-                .map(|p| p.to_str().unwrap_or("unknown").to_owned()),
-            content,
-        })
     }
 }
+
+type Path = String;
+type FileContent = String;
+
 #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))]
 #[cfg_attr(
     feature = "serde",
     derive(serde::Serialize),
     serde(rename_all = "camelCase")
 )]
 pub struct Diff {
-    /// Old file path and content
-    old: Option<DiffFile>,
-    /// New file path and content
-    new: Option<DiffFile>,
-    /// Kind of the diff
-    kind: DiffKind,
     /// Patch between old and new
     patch: String,
+    /// Stats of the diff
+    stats: DiffStats,
+    /// Map of old source paths to the old content
+    old_sources: hash_map::HashMap<Path, FileContent>,
 }
+
 impl Diff {
-    pub fn from_repo_and_patch(repo: &git2::Repository, patch: &mut git2::Patch) -> Diff {
-        let (old, new, diff_type) = {
-            let delta = patch.delta();
-            let diff_type = if delta.flags().is_binary() {
-                DiffKind::Binary
-            } else {
-                DiffKind::Text
-            };
-            (
-                DiffFile::try_from_repo_and_diff_file(repo, &delta.old_file()),
-                DiffFile::try_from_repo_and_diff_file(repo, &delta.new_file()),
-                diff_type,
-            )
-        };
+    pub fn try_from_repo_and_diff(repo: &git2::Repository, diff: &git2::Diff) -> Result<Self> {
+        let mut patch_output = String::new();
+        let mut total_num_lines: usize = 0;
+        let mut old_files: hash_map::HashMap<Path, FileContent> = hash_map::HashMap::new();
+        // Collect old file contents from each delta
+        diff.foreach(
+            &mut |delta, _| {
+                let oid = delta.old_file().id();
+                if !oid.is_zero() {
+                    let path = delta
+                        .old_file()
+                        .path()
+                        .map(|p| p.to_string_lossy().to_string())
+                        .unwrap_or_else(|| "<unknown>".to_string());
 
-        let patch_buf = patch.to_buf().expect("failed unwrapping patch buffer");
-        let patch_text = patch_buf.as_str().unwrap_or("").to_owned();
-        Self {
-            old,
-            new,
-            kind: diff_type,
-            patch: patch_text,
-        }
-    }
+                    if let Ok(blob) = repo.find_blob(oid) {
+                        if let Ok(content) = std::str::from_utf8(blob.content()) {
+                            let text = content.to_string();
+                            total_num_lines += text.lines().count();
+                            old_files.insert(path, text);
+                        } else {
+                            old_files.insert(path, "<binary or invalid utf8>".to_string());
+                        }
+                    }
+                }
+                true
+            },
+            None,
+            None,
+            None,
+        )
+        .map_err(|e| error::Error::from_ctx_and_error("Error getting old file contents", e))?;
 
-    pub fn binary_from_repo_and_delta(repo: &git2::Repository, delta: &git2::DiffDelta) -> Diff {
-        let old = DiffFile::try_from_repo_and_diff_file(repo, &delta.old_file());
-        let new = DiffFile::try_from_repo_and_diff_file(repo, &delta.new_file());
-        Self {
-            old,
-            new,
-            kind: DiffKind::Binary,
-            patch: "".to_string(),
-        }
+        diff.print(git2::DiffFormat::Patch, |_delta, _hunk, line| {
+            match line.origin() {
+                // For Addition, Deletion, and Context lines, the prefix needs to be prepended
+                '+' | '-' | ' ' | '@' => {
+                    patch_output.push(line.origin());
+                }
+                // For any other line type (e.g., file headers, hunk headers),
+                // the content is already fully formatted and should not be prefixed.
+                _ => {}
+            }
+            if let Ok(text) = str::from_utf8(line.content()) {
+                patch_output.push_str(text);
+            } else {
+                patch_output.push_str("<invalid utf8>");
+            }
+            true
+        })
+        .map_err(|e| error::Error::from_ctx_and_error("Error creating patch", e))?;
+
+        Ok(Self {
+            patch: patch_output,
+            stats: DiffStats::from_stats_and_total_old_num_lines(
+                &diff
+                    .stats()
+                    .map_err(|e| error::Error::from_ctx_and_error("Error getting diff stats", e))?,
+                total_num_lines,
+            ),
+            old_sources: old_files,
+        })
     }
 }

backend/git2-ox/src/repository.rs

@@ -92,15 +92,11 @@ impl Repository {
         Commit::try_for_revision(&self.repo, rev)
     }
 
-    /// Returns an iterator over diffs between two revisions `base_rev` and `head_rev`
-    ///
-    /// * `base_rev` - Base revision to use as a tree, uses initial commit if set to `None`
-    /// * `head_rev` - Head revision until which to diff. Using current `HEAD` if set to `None`
-    pub fn iter_diffs_between_revisions(
+    fn git2_diff_for_revisions(
         &self,
         base_rev: Option<&str>,
         head_rev: Option<&str>,
-    ) -> Result<impl Iterator<Item = Result<Diff>>> {
+    ) -> Result<git2::Diff> {
         let head = head_rev.unwrap_or("HEAD");
         let tree = utils::get_tree_for_revision(&self.repo, head)?;
 
@@ -112,31 +108,25 @@ impl Repository {
             None => None,
         };
 
-        let diff = self
+        let mut diff = self
             .repo
             .diff_tree_to_tree(base_tree.as_ref(), Some(&tree), None)
             .map_err(|e| {
                 Error::from_ctx_and_error(format!("Failed to diff tree {base_rev:?} to {head}"), e)
             })?;
 
-        let num_deltas = diff.deltas().len();
-
-        Ok((0..num_deltas).filter_map(move |delta_idx| {
-            // `diff` is owned by this function and must not be moved into the closure.
-            // Using `move` only for delta_idx, not for diff.
-            match git2::Patch::from_diff(&diff, delta_idx) {
-                Err(e) => Some(Err(Error::from_ctx_and_error("Failed to create patch", e))),
-                Ok(Some(mut patch)) => Some(Ok(Diff::from_repo_and_patch(&self.repo, &mut patch))),
-                Ok(None) => {
-                    if let Some(delta) = diff.get_delta(delta_idx) {
-                        Some(Ok(Diff::binary_from_repo_and_delta(&self.repo, &delta)))
-                    } else {
-                        log::error!("Failed to get delta for idx {delta_idx}");
-                        None
-                    }
-                }
-            }
-        }))
+        // Enable rename detection with DiffFindOptions
+        let mut find_opts = git2::DiffFindOptions::new();
+        find_opts.renames(true);
+        // Transform a diff marking file renames, copies, etc.
+        diff.find_similar(Some(&mut find_opts))
+            .map_err(|e| Error::from_ctx_and_error("Failed to find similar files in diff", e))?;
+        Ok(diff)
+    }
+
+    pub fn diff(&self, base_rev: Option<&str>, head_rev: Option<&str>) -> Result<Diff> {
+        let diff = self.git2_diff_for_revisions(base_rev, head_rev)?;
+        Diff::try_from_repo_and_diff(self.repo(), &diff)
     }
 
     /// Returns an iterator over tags in the repository which names contain `filter`

frontend/src/client.ts

@@ -144,13 +144,13 @@ export const fetchDiffs = async (range?: {
   baseRev?: string;
   headRev?: string;
 }) => {
-  const { data, error } = await client.GET("/api/v1/git/diffs", {
+  const { data, error } = await client.GET("/api/v1/git/diff", {
     params: { query: range },
   });
   if (error) {
     throw new Error(`Error fetching diffs: ${error.message}`);
   }
-  return data.diffs;
+  return data.diff;
 };
 
 export async function fetchCommitsMetadata(