cli: Improve pretty diff printing

Refactor, cleanup, improve tests etc.

radicle-cli/examples/rad-diff.md

@@ -81,3 +81,79 @@ $ rad diff --staged
 ╰──────────────────────────────────────────────╯
 
 ```
+
+```
+$ git rm -f -q main.c
+$ rad diff --staged
+╭────────────────────────────────────────────╮
+│ main.c -6 ❲deleted❳                        │
+├────────────────────────────────────────────┤
+│ @@ -1,6 +0,0 @@                            │
+│ 1          - #include <stdio.h>            │
+│ 2          -                               │
+│ 3          - int main(void) {              │
+│ 4          -     printf("Hello World!/n"); │
+│ 5          -     return 0;                 │
+│ 6          - }                             │
+╰────────────────────────────────────────────╯
+
+```
+
+For now, copies are not detected.
+
+```
+$ git reset --hard master -q
+$ mkdir docs
+$ cp README.md docs/README.md
+$ git add docs
+$ rad diff --staged
+╭─────────────────────────────╮
+│ docs/README.md +1 ❲created❳ │
+├─────────────────────────────┤
+│ @@ -0,0 +1,1 @@             │
+│      1     + Hello World!   │
+╰─────────────────────────────╯
+
+$ git reset
+$ git checkout .
+```
+
+Empty file.
+
+```
+$ touch EMPTY
+$ git add EMPTY
+$ rad diff --staged
+╭─────────────────╮
+│ EMPTY ❲created❳ │
+╰─────────────────╯
+
+$ git reset
+$ git checkout .
+```
+
+File mode change.
+
+```
+$ chmod +x README.md
+$ rad diff
+╭───────────────────────────────────────────╮
+│ README.md 100644 -> 100755 ❲mode changed❳ │
+╰───────────────────────────────────────────╯
+
+$ git reset -q
+$ git checkout .
+```
+
+Binary file.
+
+```
+$ touch file.bin
+$ truncate -s 8 file.bin
+$ git add file.bin
+$ rad diff --staged
+╭─────────────────────────────╮
+│ file.bin ❲binary❳ ❲created❳ │
+╰─────────────────────────────╯
+
+```

radicle-cli/src/git/ddiff.rs

@@ -245,6 +245,7 @@ impl From<&FileDDiff> for unified_diff::FileHeader {
             path: value.path.clone(),
             old: value.old.clone(),
             new: value.new.clone(),
+            binary: false,
         }
     }
 }

radicle-cli/src/git/pretty_diff.rs

@@ -1,11 +1,14 @@
 use std::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use radicle::git;
+use radicle_git_ext::Oid;
 use radicle_surf::diff;
+use radicle_surf::diff::{Added, Copied, Deleted, FileStats, Hunks, Modified, Moved};
 use radicle_surf::diff::{Diff, DiffContent, FileDiff, Hunk, Modification};
 use radicle_term as term;
 use term::cell::Cell;
+use term::VStack;
 
 use crate::git::unified_diff::FileHeader;
 use crate::terminal::highlight::{Highlighter, Theme};
@@ -62,10 +65,59 @@ impl Repo for git::raw::Repository {
 }
 
 /// Blobs passed down to the hunk renderer.
-#[derive(Debug, Default)]
+#[derive(Debug)]
-pub struct Blobs {
+pub struct Blobs<T> {
-    old: Option<Vec<term::Line>>,
+    pub old: Option<T>,
-    new: Option<Vec<term::Line>>,
+    pub new: Option<T>,
+}
+
+impl<T> Blobs<T> {
+    pub fn new(old: Option<T>, new: Option<T>) -> Self {
+        Self { old, new }
+    }
+}
+
+impl Blobs<(PathBuf, Blob)> {
+    pub fn highlight(&self, hi: &mut Highlighter) -> Blobs<Vec<term::Line>> {
+        let mut blobs = Blobs::default();
+        if let Some((path, Blob::Plain(content))) = &self.old {
+            blobs.old = hi.highlight(path, content).ok();
+        }
+        if let Some((path, Blob::Plain(content))) = &self.new {
+            blobs.new = hi.highlight(path, content).ok();
+        }
+        blobs
+    }
+
+    pub fn from_paths<R: Repo>(
+        old: Option<(&Path, Oid)>,
+        new: Option<(&Path, Oid)>,
+        repo: &R,
+    ) -> Blobs<(PathBuf, Blob)> {
+        Blobs::new(
+            old.and_then(|(path, oid)| {
+                repo.blob(oid)
+                    .ok()
+                    .or_else(|| repo.file(path))
+                    .map(|blob| (path.to_path_buf(), blob))
+            }),
+            new.and_then(|(path, oid)| {
+                repo.blob(oid)
+                    .ok()
+                    .or_else(|| repo.file(path))
+                    .map(|blob| (path.to_path_buf(), blob))
+            }),
+        )
+    }
+}
+
+impl<T> Default for Blobs<T> {
+    fn default() -> Self {
+        Self {
+            old: None,
+            new: None,
+        }
+    }
 }
 
 /// Types that can be rendered as pretty diffs.
@@ -96,39 +148,108 @@ impl ToPretty for Diff {
     ) -> Self::Output {
         term::VStack::default()
             .padding(0)
-            .children(self.files().map(|f| f.pretty(hi, context, repo).boxed()))
+            .children(self.files().flat_map(|f| {
+                [
+                    f.pretty(hi, context, repo).boxed(),
+                    term::Line::blank().boxed(), // Blank line between files.
+                ]
+            }))
     }
 }
 
 impl ToPretty for FileHeader {
     type Output = term::Line;
-    type Context = ();
+    type Context = Option<FileStats>;
 
     fn pretty<R: Repo>(
         &self,
         _hi: &mut Highlighter,
-        _context: &Self::Context,
+        stats: &Self::Context,
         _repo: &R,
     ) -> Self::Output {
-        match self {
+        let theme = Theme::default();
-            FileHeader::Added { path, .. } => term::Line::new(path.display().to_string()),
+        let (mut header, badge, binary) = match self {
+            FileHeader::Added { path, binary, .. } => (
+                term::Line::new(path.display().to_string()),
+                Some(term::format::badge_positive("created")),
+                *binary,
+            ),
             FileHeader::Moved {
                 old_path, new_path, ..
-            } => term::Line::spaced([
+            } => (
+                term::Line::spaced([
                     term::label(old_path.display().to_string()),
                     term::label("->".to_string()),
                     term::label(new_path.display().to_string()),
                 ]),
-            FileHeader::Deleted { path, .. } => term::Line::new(path.display().to_string()),
+                Some(term::format::badge_secondary("moved")),
-            FileHeader::Modified { path, .. } => term::Line::new(path.display().to_string()),
+                false,
+            ),
+            FileHeader::Deleted { path, binary, .. } => (
+                term::Line::new(path.display().to_string()),
+                Some(term::format::badge_negative("deleted")),
+                *binary,
+            ),
+            FileHeader::Modified {
+                path,
+                old,
+                new,
+                binary,
+                ..
+            } => {
+                if old.mode != new.mode {
+                    (
+                        term::Line::spaced([
+                            term::label(path.display().to_string()),
+                            term::label(format!("{:o}", u32::from(old.mode.clone())))
+                                .fg(term::Color::Blue),
+                            term::label("->".to_string()),
+                            term::label(format!("{:o}", u32::from(new.mode.clone())))
+                                .fg(term::Color::Blue),
+                        ]),
+                        Some(term::format::badge_secondary("mode changed")),
+                        *binary,
+                    )
+                } else {
+                    (term::Line::new(path.display().to_string()), None, *binary)
+                }
+            }
             FileHeader::Copied {
                 old_path, new_path, ..
-            } => term::Line::spaced([
+            } => (
+                term::Line::spaced([
                     term::label(old_path.display().to_string()),
                     term::label("->".to_string()),
                     term::label(new_path.display().to_string()),
                 ]),
+                Some(term::format::badge_secondary("copied")),
+                false,
+            ),
+        };
+
+        if binary {
+            header.push(term::Label::space());
+            header.push(term::label(term::format::badge_yellow("binary")));
         }
+
+        let (additions, deletions) = if let Some(stats) = stats {
+            (stats.additions, stats.deletions)
+        } else {
+            (0, 0)
+        };
+        if deletions > 0 {
+            header.push(term::Label::space());
+            header.push(term::label(format!("-{deletions}")).fg(theme.color("negative.light")));
+        }
+        if additions > 0 {
+            header.push(term::Label::space());
+            header.push(term::label(format!("+{additions}")).fg(theme.color("positive.light")));
+        }
+        if let Some(badge) = badge {
+            header.push(term::Label::space());
+            header.push(badge);
+        }
+        header
     }
 }
 
@@ -142,131 +263,144 @@ impl ToPretty for FileDiff {
         _context: &Self::Context,
         repo: &R,
     ) -> Self::Output {
-        let content = match self {
+        let header = FileHeader::from(self);
-            FileDiff::Added(f) => f.diff.pretty(hi, self, repo),
+
-            FileDiff::Moved(f) => f.diff.pretty(hi, self, repo),
+        match self {
-            FileDiff::Deleted(f) => f.diff.pretty(hi, self, repo),
+            FileDiff::Added(f) => f.pretty(hi, &header, repo),
-            FileDiff::Modified(f) => f.diff.pretty(hi, self, repo),
+            FileDiff::Deleted(f) => f.pretty(hi, &header, repo),
-            FileDiff::Copied(f) => f.diff.pretty(hi, self, repo),
+            FileDiff::Modified(f) => f.pretty(hi, &header, repo),
-        };
+            FileDiff::Moved(f) => f.pretty(hi, &header, repo),
-        term::VStack::default()
+            FileDiff::Copied(f) => f.pretty(hi, &header, repo),
-            .padding(0)
+        }
-            .child(content)
-            .child(term::Line::blank())
     }
 }
 
 impl ToPretty for DiffContent {
     type Output = term::VStack<'static>;
-    type Context = FileDiff;
+    type Context = Blobs<(PathBuf, Blob)>;
 
     fn pretty<R: Repo>(
         &self,
         hi: &mut Highlighter,
-        context: &Self::Context,
+        blobs: &Self::Context,
         repo: &R,
     ) -> Self::Output {
-        let header = FileHeader::from(context);
+        let mut vstack = term::VStack::default().padding(0);
-        let theme = Theme::default();
-
-        let (old, new, badge) = match context {
-            FileDiff::Added(f) => (
-                None,
-                Some((f.new.oid, f.path.clone())),
-                Some(term::format::badge_positive("created")),
-            ),
-            FileDiff::Moved(f) => (
-                Some((f.old.oid, f.old_path.clone())),
-                Some((f.new.oid, f.new_path.clone())),
-                Some(term::format::badge_secondary("moved")),
-            ),
-            FileDiff::Deleted(f) => (
-                Some((f.old.oid, f.path.clone())),
-                None,
-                Some(term::format::badge_negative("deleted")),
-            ),
-            FileDiff::Modified(f) => (
-                Some((f.old.oid, f.path.clone())),
-                Some((f.new.oid, f.path.clone())),
-                None,
-            ),
-            FileDiff::Copied(f) => (
-                Some((f.old.oid, f.old_path.clone())),
-                Some((f.old.oid, f.new_path.clone())),
-                Some(term::format::badge_secondary("copied")),
-            ),
-        };
-        let mut header = header.pretty(hi, &(), repo);
-
-        let (additions, deletions) = if let Some(stats) = self.stats() {
-            (stats.additions, stats.deletions)
-        } else {
-            (0, 0)
-        };
-
-        if deletions > 0 {
-            header.push(term::Label::space());
-            header.push(term::label(format!("-{deletions}")).fg(theme.color("negative.light")));
-        }
-        if additions > 0 {
-            header.push(term::Label::space());
-            header.push(term::label(format!("+{additions}")).fg(theme.color("positive.light")));
-        }
-        if let Some(badge) = badge {
-            header.push(term::Label::space());
-            header.push(badge);
-        }
-
-        let old = old.and_then(|(oid, path)| repo.blob(oid).ok().or_else(|| repo.file(&path)));
-        let new = new.and_then(|(oid, path)| repo.blob(oid).ok().or_else(|| repo.file(&path)));
-        let mut blobs = Blobs::default();
-
-        if let Some(Blob::Plain(content)) = old {
-            blobs.old = hi.highlight(context.path(), &content).ok();
-        }
-        if let Some(Blob::Plain(content)) = new {
-            blobs.new = hi.highlight(context.path(), &content).ok();
-        }
-        let mut vstack = term::VStack::default()
-            .border(Some(term::colors::FAINT))
-            .padding(1)
-            .child(term::Line::default().extend(header));
-
-        match context {
-            FileDiff::Moved(_) | FileDiff::Copied(_) => {}
-            FileDiff::Added(_) if blobs.new.is_none() => {
-                vstack = vstack.divider();
-                vstack.push(term::Line::new(term::format::italic("Empty file")));
-            }
-            FileDiff::Deleted(_) if blobs.old.is_none() => {
-                vstack = vstack.divider();
-                vstack.push(term::Line::new(term::format::italic("Empty file")));
-            }
-            FileDiff::Added(_) | FileDiff::Deleted(_) | FileDiff::Modified(_) => {
-                vstack = vstack.divider();
 
         match self {
-                    DiffContent::Plain { hunks, .. } => {
+            DiffContent::Plain {
+                hunks: Hunks(hunks),
+                ..
+            } => {
+                let blobs = blobs.highlight(hi);
+
                 for (i, h) in hunks.iter().enumerate() {
                     vstack.push(h.pretty(hi, &blobs, repo));
-                            if i != hunks.0.len() - 1 {
+                    if i != hunks.len() - 1 {
                         vstack = vstack.divider();
                     }
                 }
             }
-                    DiffContent::Empty => {
+            DiffContent::Empty => {}
-                        vstack.push(term::Line::new(term::format::italic("Empty file")));
+            DiffContent::Binary => {}
-                    }
-                    DiffContent::Binary => {
-                        vstack.push(term::Line::new(term::format::italic("Binary file")));
-                    }
-                }
-            }
         }
         vstack
     }
 }
 
+impl ToPretty for Moved {
+    type Output = term::VStack<'static>;
+    type Context = FileHeader;
+
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        header: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
+        let header = header.pretty(hi, &self.diff.stats().copied(), repo);
+
+        term::VStack::default()
+            .border(Some(term::colors::FAINT))
+            .padding(1)
+            .child(term::Line::default().extend(header))
+    }
+}
+
+impl ToPretty for Added {
+    type Output = term::VStack<'static>;
+    type Context = FileHeader;
+
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        header: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
+        let old = None;
+        let new = Some((self.path.as_path(), self.new.oid));
+
+        pretty_modification(header, &self.diff, old, new, repo, hi)
+    }
+}
+
+impl ToPretty for Deleted {
+    type Output = term::VStack<'static>;
+    type Context = FileHeader;
+
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        header: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
+        let old = Some((self.path.as_path(), self.old.oid));
+        let new = None;
+
+        pretty_modification(header, &self.diff, old, new, repo, hi)
+    }
+}
+
+impl ToPretty for Modified {
+    type Output = term::VStack<'static>;
+    type Context = FileHeader;
+
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        header: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
+        let old = Some((self.path.as_path(), self.old.oid));
+        let new = Some((self.path.as_path(), self.new.oid));
+
+        pretty_modification(header, &self.diff, old, new, repo, hi)
+    }
+}
+
+impl ToPretty for Copied {
+    type Output = term::VStack<'static>;
+    type Context = FileHeader;
+
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        _context: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
+        let header = FileHeader::Copied {
+            old_path: self.old_path.clone(),
+            new_path: self.old_path.clone(),
+        }
+        .pretty(hi, &self.diff.stats().copied(), repo);
+
+        term::VStack::default()
+            .border(Some(term::colors::FAINT))
+            .padding(1)
+            .child(header)
+    }
+}
+
 impl ToPretty for HunkHeader {
     type Output = term::Line;
     type Context = ();
@@ -291,9 +425,14 @@ impl ToPretty for HunkHeader {
 
 impl ToPretty for Hunk<Modification> {
     type Output = term::VStack<'static>;
-    type Context = Blobs;
+    type Context = Blobs<Vec<term::Line>>;
 
-    fn pretty<R: Repo>(&self, hi: &mut Highlighter, blobs: &Blobs, repo: &R) -> Self::Output {
+    fn pretty<R: Repo>(
+        &self,
+        hi: &mut Highlighter,
+        blobs: &Self::Context,
+        repo: &R,
+    ) -> Self::Output {
         let mut vstack = term::VStack::default().padding(0);
         let mut table = term::Table::<5, term::Filled<term::Line>>::new(term::TableOptions {
             overflow: false,
@@ -379,9 +518,14 @@ impl ToPretty for Hunk<Modification> {
 
 impl ToPretty for Modification {
     type Output = term::Line;
-    type Context = Blobs;
+    type Context = Blobs<Vec<term::Line>>;
 
-    fn pretty<R: Repo>(&self, _hi: &mut Highlighter, blobs: &Blobs, _repo: &R) -> Self::Output {
+    fn pretty<R: Repo>(
+        &self,
+        _hi: &mut Highlighter,
+        blobs: &Blobs<Vec<term::Line>>,
+        _repo: &R,
+    ) -> Self::Output {
         match self {
             Modification::Deletion(diff::Deletion { line, line_no }) => {
                 if let Some(lines) = &blobs.old.as_ref() {
@@ -411,6 +555,30 @@ impl ToPretty for Modification {
     }
 }
 
+/// Render a file added, deleted or modified.
+fn pretty_modification<R: Repo>(
+    header: &FileHeader,
+    diff: &DiffContent,
+    old: Option<(&Path, Oid)>,
+    new: Option<(&Path, Oid)>,
+    repo: &R,
+    hi: &mut Highlighter,
+) -> VStack<'static> {
+    let blobs = Blobs::from_paths(old, new, repo);
+    let header = header.pretty(hi, &diff.stats().copied(), repo);
+    let vstack = term::VStack::default()
+        .border(Some(term::colors::FAINT))
+        .padding(1)
+        .child(header);
+
+    let body = diff.pretty(hi, &blobs, repo);
+    if body.is_empty() {
+        vstack
+    } else {
+        vstack.divider().merge(body)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use std::ffi::OsStr;

radicle-cli/src/git/unified_diff.rs

@@ -50,6 +50,7 @@ pub enum FileHeader {
     Added {
         path: PathBuf,
         new: DiffFile,
+        binary: bool,
     },
     Copied {
         old_path: PathBuf,
@@ -58,11 +59,13 @@ pub enum FileHeader {
     Deleted {
         path: PathBuf,
         old: DiffFile,
+        binary: bool,
     },
     Modified {
         path: PathBuf,
         old: DiffFile,
         new: DiffFile,
+        binary: bool,
     },
     Moved {
         old_path: PathBuf,
@@ -78,15 +81,21 @@ impl std::convert::From<&FileDiff> for FileHeader {
                 path: v.path.clone(),
                 old: v.old.clone(),
                 new: v.new.clone(),
+                binary: matches!(v.diff, DiffContent::Binary),
             },
             FileDiff::Added(v) => FileHeader::Added {
                 path: v.path.clone(),
                 new: v.new.clone(),
+                binary: matches!(v.diff, DiffContent::Binary),
+            },
+            FileDiff::Copied(c) => FileHeader::Copied {
+                old_path: c.old_path.clone(),
+                new_path: c.new_path.clone(),
             },
-            FileDiff::Copied(_) => todo!(),
             FileDiff::Deleted(v) => FileHeader::Deleted {
                 path: v.path.clone(),
                 old: v.old.clone(),
+                binary: matches!(v.diff, DiffContent::Binary),
             },
             FileDiff::Moved(v) => FileHeader::Moved {
                 old_path: v.old_path.clone(),
@@ -288,7 +297,7 @@ impl Encode for FileDiff {
 impl Encode for FileHeader {
     fn encode(&self, w: &mut Writer) -> Result<(), Error> {
         match self {
-            FileHeader::Modified { path, old, new } => {
+            FileHeader::Modified { path, old, new, .. } => {
                 w.meta(format!(
                     "diff --git a/{} b/{}",
                     path.display(),
@@ -315,7 +324,7 @@ impl Encode for FileHeader {
                 w.meta(format!("--- a/{}", path.display()))?;
                 w.meta(format!("+++ b/{}", path.display()))?;
             }
-            FileHeader::Added { path, new } => {
+            FileHeader::Added { path, new, .. } => {
                 w.meta(format!(
                     "diff --git a/{} b/{}",
                     path.display(),
@@ -333,7 +342,7 @@ impl Encode for FileHeader {
                 w.meta(format!("+++ b/{}", path.display()))?;
             }
             FileHeader::Copied { .. } => todo!(),
-            FileHeader::Deleted { path, old } => {
+            FileHeader::Deleted { path, old, .. } => {
                 w.meta(format!(
                     "diff --git a/{} b/{}",
                     path.display(),

radicle-term/src/format.rs

@@ -52,6 +52,14 @@ pub fn badge_primary<D: std::fmt::Display>(input: D) -> Paint<String> {
     }
 }
 
+pub fn badge_yellow<D: std::fmt::Display>(input: D) -> Paint<String> {
+    if Paint::is_enabled() {
+        Paint::yellow(format!(" {input} ")).invert()
+    } else {
+        Paint::new(format!("❲{input}❳"))
+    }
+}
+
 pub fn badge_positive<D: std::fmt::Display>(input: D) -> Paint<String> {
     if Paint::is_enabled() {
         Paint::green(format!(" {input} ")).invert()

radicle-term/src/vstack.rs

@@ -66,6 +66,11 @@ impl<'a> VStack<'a> {
         self
     }
 
+    /// Check if this stack is empty.
+    pub fn is_empty(&self) -> bool {
+        self.rows.is_empty()
+    }
+
     /// Add multiple elements to the stack.
     pub fn children<I>(self, children: I) -> Self
     where
@@ -79,6 +84,14 @@ impl<'a> VStack<'a> {
         vstack
     }
 
+    /// Merge with another `VStack`.
+    pub fn merge(mut self, other: Self) -> Self {
+        for row in other.rows {
+            self.rows.push(row);
+        }
+        self
+    }
+
     /// Set or unset the outer border.
     pub fn border(mut self, color: Option<Color>) -> Self {
         self.opts.border = color;