Fix Git diff rendering

src/app.rs

@@ -425,6 +425,7 @@ pub(crate) enum Message {
     ToggleGitFile(PathBuf),
     OpenGitDiff(PathBuf),
     SetGitDiffMode(GitDiffMode),
+    GitDiffScrolled(scrollable::Viewport),
     GitCommitMessageChanged(String),
     GitStageSelected,
     GitUnstageSelected,
@@ -1755,6 +1756,16 @@ impl App {
             Message::ToggleGitFile(path) => self.toggle_git_file(path),
             Message::OpenGitDiff(path) => self.open_git_diff(&path),
             Message::SetGitDiffMode(mode) => self.git_diff_mode = mode,
+            Message::GitDiffScrolled(viewport) => {
+                let offset = scrollable::AbsoluteOffset {
+                    x: viewport.absolute_offset().x,
+                    y: 0.0,
+                };
+                return Task::batch([
+                    iced::widget::operation::scroll_to(git_diff_old_scroll_id(), offset),
+                    iced::widget::operation::scroll_to(git_diff_new_scroll_id(), offset),
+                ]);
+            }
             Message::GitCommitMessageChanged(message) => self.git_commit_message = message,
             Message::GitStageSelected => self.stage_selected_git_files(),
             Message::GitUnstageSelected => self.unstage_selected_git_files(),
@@ -2428,6 +2439,14 @@ pub(super) fn chat_scroll_id() -> iced::widget::Id {
     iced::widget::Id::new("chat-transcript")
 }
 
+pub(super) fn git_diff_old_scroll_id() -> iced::widget::Id {
+    iced::widget::Id::new("git-diff-old")
+}
+
+pub(super) fn git_diff_new_scroll_id() -> iced::widget::Id {
+    iced::widget::Id::new("git-diff-new")
+}
+
 pub(super) fn composer_id() -> iced::widget::Id {
     iced::widget::Id::new("chat-composer")
 }

src/app/git.rs

@@ -1,6 +1,5 @@
 use super::*;
 use std::collections::BTreeMap;
-use std::ffi::OsStr;
 use std::process::{Command, Output};
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -483,43 +482,65 @@ fn worktree_kind(summary: gix::status::index_worktree::iter::Summary) -> GitChan
 }
 
 fn load_diff(root: &Path, file: &GitFile) -> Result<GitDiff, String> {
-    let mut sections = Vec::new();
+    let repository = gix::discover(root).map_err(|error| error.to_string())?;
+    let index = repository
+        .index_or_empty()
+        .map_err(|error| error.to_string())?;
+    let index_path =
+        gix::path::to_unix_separators_on_windows(gix::path::into_bstr(file.path.as_path()));
+    let index_blob = index
+        .entry_by_path(index_path.as_ref())
+        .filter(|entry| {
+            entry
+                .mode
+                .to_tree_entry_mode()
+                .is_some_and(|mode| mode.is_blob_or_symlink())
+        })
+        .map(|entry| {
+            repository
+                .find_blob(entry.id)
+                .map(|mut blob| blob.take_data())
+                .map_err(|error| error.to_string())
+        })
+        .transpose()?;
+    let mut lines = Vec::new();
     if file.staged_kind.is_some() {
-        let output = run_git_path(
+        let tree = repository
-            root,
+            .find_tree(
-            &["diff", "--cached", "--no-ext-diff", "--no-color", "--"],
+                repository
-            &file.path,
+                    .head_tree_id_or_empty()
-        )?;
+                    .map_err(|error| error.to_string())?,
-        sections.push(("Staged changes", output));
+            )
+            .map_err(|error| error.to_string())?;
+        let head_blob = tree
+            .lookup_entry_by_path(&file.path)
+            .map_err(|error| error.to_string())?
+            .filter(|entry| entry.mode().is_blob_or_symlink())
+            .map(|entry| {
+                repository
+                    .find_blob(entry.object_id())
+                    .map(|mut blob| blob.take_data())
+                    .map_err(|error| error.to_string())
+            })
+            .transpose()?;
+        lines.push(diff_section("Staged changes"));
+        lines.extend(diff_blobs(
+            head_blob.as_deref(),
+            index_blob.as_deref(),
+            &file.display_path,
+        )?);
     }
     if file.worktree_kind.is_some() {
-        let mut output = run_git_path(
+        let worktree_blob = read_worktree_blob(&root.join(&file.path))?;
-            root,
+        lines.push(diff_section("Worktree changes"));
-            &["diff", "--no-ext-diff", "--no-color", "--"],
+        lines.extend(diff_blobs(
-            &file.path,
+            index_blob.as_deref(),
-        )?;
+            worktree_blob.as_deref(),
-        if output.is_empty() && file.worktree_kind == Some(GitChangeKind::Added) {
+            &file.display_path,
-            output = run_no_index_diff(root, &file.path)?;
+        )?);
-        }
-        sections.push(("Worktree changes", output));
-    }
-    let mut lines = Vec::new();
-    for (label, content) in sections {
-        lines.push(GitDiffLine {
-            kind: GitDiffLineKind::Section,
-            old_number: None,
-            new_number: None,
-            text: label.into(),
-        });
-        lines.extend(parse_unified_diff(&content));
     }
     if lines.is_empty() {
-        lines.push(GitDiffLine {
+        lines.push(diff_message("No textual diff is available."));
-            kind: GitDiffLineKind::Header,
-            old_number: None,
-            new_number: None,
-            text: "No textual diff is available.".into(),
-        });
     }
     Ok(GitDiff {
         path: file.display_path.clone(),
@@ -527,6 +548,76 @@ fn load_diff(root: &Path, file: &GitFile) -> Result<GitDiff, String> {
     })
 }
 
+fn diff_section(text: &str) -> GitDiffLine {
+    GitDiffLine {
+        kind: GitDiffLineKind::Section,
+        old_number: None,
+        new_number: None,
+        text: text.into(),
+    }
+}
+
+fn diff_message(text: &str) -> GitDiffLine {
+    GitDiffLine {
+        kind: GitDiffLineKind::Header,
+        old_number: None,
+        new_number: None,
+        text: text.into(),
+    }
+}
+
+fn read_worktree_blob(path: &Path) -> Result<Option<Vec<u8>>, String> {
+    match fs::symlink_metadata(path) {
+        Ok(metadata) if metadata.file_type().is_symlink() => fs::read_link(path)
+            .map(|target| Some(target.as_os_str().as_encoded_bytes().to_vec()))
+            .map_err(|error| error.to_string()),
+        Ok(metadata) if metadata.is_file() => {
+            fs::read(path).map(Some).map_err(|error| error.to_string())
+        }
+        Ok(_) => Ok(None),
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None),
+        Err(error) => Err(error.to_string()),
+    }
+}
+
+fn diff_blobs(
+    before: Option<&[u8]>,
+    after: Option<&[u8]>,
+    path: &str,
+) -> Result<Vec<GitDiffLine>, String> {
+    let before_label = before.map_or_else(|| "/dev/null".into(), |_| format!("a/{path}"));
+    let after_label = after.map_or_else(|| "/dev/null".into(), |_| format!("b/{path}"));
+    let before = before.unwrap_or_default();
+    let after = after.unwrap_or_default();
+    let mut lines = vec![
+        diff_message(&format!("--- {before_label}")),
+        diff_message(&format!("+++ {after_label}")),
+    ];
+    if before == after {
+        lines.push(diff_message("No textual diff is available."));
+        return Ok(lines);
+    }
+    if before.iter().take(8_000).any(|byte| *byte == 0)
+        || after.iter().take(8_000).any(|byte| *byte == 0)
+    {
+        lines.push(diff_message("Binary file changed."));
+        return Ok(lines);
+    }
+    let input = gix::diff::blob::InternedInput::new(before, after);
+    let diff =
+        gix::diff::blob::diff_with_slider_heuristics(gix::diff::blob::Algorithm::Histogram, &input);
+    let output = gix::diff::blob::UnifiedDiff::new(
+        &diff,
+        &input,
+        gix::diff::blob::unified_diff::ConsumeBinaryHunk::new(Vec::new(), "\n"),
+        Default::default(),
+    )
+    .consume()
+    .map_err(|error| error.to_string())?;
+    lines.extend(parse_unified_diff(&String::from_utf8_lossy(&output)));
+    Ok(lines)
+}
+
 fn parse_unified_diff(diff: &str) -> Vec<GitDiffLine> {
     let mut old_number = None;
     let mut new_number = None;
@@ -619,10 +710,6 @@ fn run_git_paths(root: &Path, arguments: &[&str], paths: &[PathBuf]) -> Result<S
     )
 }
 
-fn run_git_path(root: &Path, arguments: &[&str], path: &Path) -> Result<String, String> {
-    run_git_paths(root, arguments, &[path.to_path_buf()])
-}
-
 fn run_git_args(root: &Path, arguments: &[&str]) -> Result<String, String> {
     let mut command = git_command(root);
     command.args(arguments);
@@ -661,21 +748,6 @@ fn run_git_remote(root: &Path, action: &str, branch: Option<&str>) -> Result<(),
     finish_git(command.output(), action).map(|_| ())
 }
 
-fn run_no_index_diff(root: &Path, path: &Path) -> Result<String, String> {
-    let mut command = git_command(root);
-    command.args(["diff", "--no-index", "--no-color", "--"]);
-    command.arg(OsStr::new("/dev/null"));
-    command.arg(root.join(path));
-    let output = command
-        .output()
-        .map_err(|error| format!("Could not run Git diff: {error}"))?;
-    if output.status.success() || output.status.code() == Some(1) {
-        Ok(String::from_utf8_lossy(&output.stdout).into_owned())
-    } else {
-        finish_git(Ok(output), "diff")
-    }
-}
-
 fn git_command(root: &Path) -> Command {
     let mut command = Command::new("git");
     command.arg("-C").arg(root).env("GIT_TERMINAL_PROMPT", "0");

src/app/view/git.rs

@@ -1,5 +1,6 @@
 use super::*;
 use crate::app::git::{GitChangeKind, GitDiffLine, GitDiffLineKind, GitDiffMode, GitDiffRow};
+use crate::app::{git_diff_new_scroll_id, git_diff_old_scroll_id};
 use iced::widget::column;
 
 impl App {
@@ -259,13 +260,7 @@ impl App {
         } else {
             split_diff(diff.split_rows())
         };
-        let diff_scroll = scrollable(content)
+        let diff_scroll = scrollable(content).height(Length::Fill).width(Length::Fill);
-            .direction(scrollable::Direction::Both {
-                vertical: scrollable::Scrollbar::new(),
-                horizontal: scrollable::Scrollbar::new(),
-            })
-            .height(Length::Fill)
-            .width(Length::Fill);
         let dialog = container(
             column![
                 row![
@@ -342,17 +337,59 @@ fn unified_diff(lines: &[GitDiffLine]) -> Element<'_, Message> {
 }
 
 fn split_diff(rows: Vec<GitDiffRow>) -> Element<'static, Message> {
-    let mut content = column![].spacing(0).width(Length::Fill);
+    let width = split_diff_width(&rows);
+    let old = scrollable(split_diff_half(&rows, true, width))
+        .id(git_diff_old_scroll_id())
+        .direction(scrollable::Direction::Horizontal(
+            scrollable::Scrollbar::new(),
+        ))
+        .on_scroll(Message::GitDiffScrolled)
+        .width(Length::Fill);
+    let new = scrollable(split_diff_half(&rows, false, width))
+        .id(git_diff_new_scroll_id())
+        .direction(scrollable::Direction::Horizontal(
+            scrollable::Scrollbar::new(),
+        ))
+        .on_scroll(Message::GitDiffScrolled)
+        .width(Length::Fill);
+    row![old, rule::vertical(1), new].into()
+}
+
+fn split_diff_width(rows: &[GitDiffRow]) -> f32 {
+    rows.iter()
+        .flat_map(|row| match row {
+            GitDiffRow::Header(line) => [Some(line), None],
+            GitDiffRow::Pair { old, new } => [old.as_ref(), new.as_ref()],
+        })
+        .flatten()
+        .map(|line| {
+            line.text
+                .chars()
+                .map(|character| if character == '\t' { 4 } else { 1 })
+                .sum::<usize>()
+        })
+        .max()
+        .unwrap_or_default() as f32
+        * 8.0
+        + 66.0
+}
+
+fn split_diff_half(rows: &[GitDiffRow], old: bool, width: f32) -> Element<'static, Message> {
+    let mut content = column![].spacing(0).width(width);
     for row in rows {
         content = content.push(match row {
-            GitDiffRow::Header(line) => diff_line(&line, false),
+            GitDiffRow::Header(line) => diff_line(line, old),
-            GitDiffRow::Pair { old, new } => row![
+            GitDiffRow::Pair {
-                diff_half(old.as_ref(), true),
+                old: old_line,
-                rule::vertical(1),
+                new: new_line,
-                diff_half(new.as_ref(), false),
+            } => diff_half(
-            ]
+                if old {
-            .height(24)
+                    old_line.as_ref()
-            .into(),
+                } else {
+                    new_line.as_ref()
+                },
+                old,
+            ),
         });
     }
     content.into()
@@ -427,3 +464,28 @@ fn staged_color() -> Color {
 fn modal_backdrop_style(_: &Theme) -> container::Style {
     container::Style::default().background(Color::from_rgba8(0, 0, 0, 0.72))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn split_diff_width_covers_the_longest_side_and_tabs() {
+        let rows = [GitDiffRow::Pair {
+            old: Some(GitDiffLine {
+                kind: GitDiffLineKind::Deletion,
+                old_number: Some(1),
+                new_number: None,
+                text: "short".into(),
+            }),
+            new: Some(GitDiffLine {
+                kind: GitDiffLineKind::Addition,
+                old_number: None,
+                new_number: Some(1),
+                text: "\tlong".into(),
+            }),
+        }];
+
+        assert_eq!(split_diff_width(&rows), 130.0);
+    }
+}