Revert "More Git Dialog UI cleanup"

backend/app/src/actors/git.rs

@@ -90,20 +90,26 @@ impl Handler<ListCommits> for GitActor {
     }
 }
 
-#[message(response = Result<git2_ox::Diff, git2_ox::error::Error>)]
-pub struct GetDiff {
+#[message(response = Result<Vec<git2_ox::Diff>, git2_ox::error::Error>)]
+pub struct ListDiffs {
     pub base_rev: Option<String>,
     pub head_rev: Option<String>,
 }
 
-impl Handler<GetDiff> for GitActor {
+impl Handler<ListDiffs> for GitActor {
     async fn handle(
         &mut self,
         _ctx: &mut Context<Self>,
-        msg: GetDiff,
-    ) -> Result<git2_ox::Diff, git2_ox::error::Error> {
-        self.repository
-            .diff(msg.base_rev.as_deref(), msg.head_rev.as_deref())
+        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)
     }
 }
 

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("/diff", routing::get(get_diff))
+        .route("/diffs", routing::get(list_diffs))
         .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, get_diff), 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, list_diffs), 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
-    diff: git2_ox::Diff,
+    diffs: Vec<git2_ox::Diff>,
 }
 
 #[utoipa::path(
     get,
-    path = "/diff",
+    path = "/diffs",
     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 get_diff(
+async fn list_diffs(
     State(state): State<web::AppState>,
     Query(query): Query<CommitRangeQuery>,
 ) -> Result<Json<ListDiffsResponse>, api::AppError> {
     let actor = state.git_actor();
-    let msg = actors::git::GetDiff {
+    let msg = actors::git::ListDiffs {
         base_rev: query.base_rev,
         head_rev: query.head_rev,
     };
-    let diff = actor.call(msg).await??;
-    Ok(Json(ListDiffsResponse { diff }))
+    let diffs = actor.call(msg).await??;
+    Ok(Json(ListDiffsResponse { diffs }))
 }
 
 #[derive(ToSchema, Serialize, Deserialize, IntoParams)]

backend/git2-ox/src/diff.rs

@@ -1,118 +1,106 @@
-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 = "lowercase")
+)]
+#[derive(PartialEq)]
+enum DiffKind {
+    Binary,
+    Text,
+}
 
 #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))]
 #[cfg_attr(
     feature = "serde",
     derive(serde::Serialize),
     serde(rename_all = "camelCase")
 )]
-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,
+struct DiffFile {
+    /// Path to the diff file
+    path: Option<String>,
+    /// Content of the diff file
+    content: Option<String>,
 }
 
-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,
+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;
         }
-    }
-}
 
-type Path = String;
-type FileContent = String;
+        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,
+        })
+    }
+}
 #[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 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());
-
-                    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))?;
-
-        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);
+    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 {
-                patch_output.push_str("<invalid utf8>");
-            }
-            true
-        })
-        .map_err(|e| error::Error::from_ctx_and_error("Error creating patch", e))?;
+                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,
+            )
+        };
 
-        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,
-        })
+        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,
+        }
+    }
+
+    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(),
+        }
     }
 }

backend/git2-ox/src/repository.rs

@@ -92,11 +92,15 @@ impl Repository {
         Commit::try_for_revision(&self.repo, rev)
     }
 
-    fn git2_diff_for_revisions(
+    /// 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(
         &self,
         base_rev: Option<&str>,
         head_rev: Option<&str>,
-    ) -> Result<git2::Diff> {
+    ) -> Result<impl Iterator<Item = Result<Diff>>> {
         let head = head_rev.unwrap_or("HEAD");
         let tree = utils::get_tree_for_revision(&self.repo, head)?;
 
@@ -118,15 +122,28 @@ impl Repository {
         // Enable rename detection with DiffFindOptions
         let mut find_opts = git2::DiffFindOptions::new();
         find_opts.renames(true);
-        // Transform a diff marking file renames, copies, etc.
+        // Transform the 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)
+        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
+                    }
+                }
+            }
+        }))
     }
 
     /// 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/diff", {
+  const { data, error } = await client.GET("/api/v1/git/diffs", {
     params: { query: range },
   });
   if (error) {
     throw new Error(`Error fetching diffs: ${error.message}`);
   }
-  return data.diff;
+  return data.diffs;
 };
 
 export async function fetchCommitsMetadata(