Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cli: Refactor git diff encoding
Slack Coder committed 2 years ago
commit 0141313b35ef40c5abd12e3b2728d91d052d1c8b
parent 6ed3ab914d9e028dbb54b84a6d66730955106a2a
5 files changed +354 -178
modified radicle-cli/src/commands/review.rs
@@ -1,7 +1,5 @@
#[path = "review/builder.rs"]
mod builder;
-
#[path = "review/diff.rs"]
-
mod diff;

use std::ffi::OsString;

modified radicle-cli/src/commands/review/builder.rs
@@ -22,10 +22,9 @@ use radicle::prelude::*;
use radicle::storage::git::Repository;
use radicle_surf::diff::*;

+
use crate::git::unified_diff;
use crate::terminal as term;

-
use super::diff::DiffWriter;
-

/// Help message shown to user.
const HELP: &str = "\
y - accept this hunk
@@ -222,7 +221,7 @@ impl<'a> ReviewBuilder<'a> {
            repo.find_commit(oid)?
        };

-
        let mut writer = DiffWriter::new(io::stdout()).styled(true);
+
        let mut writer = unified_diff::Writer::new(io::stdout()).styled(true);
        let mut queue = ReviewQueue::default(); // Queue of hunks to review.
        let mut current = None; // File of the current hunk.
        let mut stdin = io::stdin().lock();
@@ -249,35 +248,21 @@ impl<'a> ReviewBuilder<'a> {
        for file in diff.files() {
            match file {
                FileDiff::Modified(f) => match &f.diff {
-
                    DiffContent::Plain { hunks, .. } => {
-
                        queue.push(file, Some(hunks));
-
                    }
-
                    DiffContent::Binary => {
-
                        queue.push(file, None);
-
                    }
+
                    DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
+
                    DiffContent::Binary => queue.push(file, None),
                    DiffContent::Empty => {}
                },
                FileDiff::Added(f) => match &f.diff {
-
                    DiffContent::Plain { hunks, .. } => {
-
                        queue.push(file, Some(hunks));
-
                    }
-
                    DiffContent::Binary => {
-
                        queue.push(file, None);
-
                    }
+
                    DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
+
                    DiffContent::Binary => queue.push(file, None),
                    DiffContent::Empty => {}
                },
                FileDiff::Deleted(f) => match &f.diff {
-
                    DiffContent::Plain { hunks, .. } => {
-
                        queue.push(file, Some(hunks));
-
                    }
-
                    DiffContent::Binary => {
-
                        queue.push(file, None);
-
                    }
+
                    DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
+
                    DiffContent::Binary => queue.push(file, None),
                    DiffContent::Empty => {}
                },
-
                FileDiff::Moved(_) => {
-
                    queue.push(file, None);
-
                }
+
                FileDiff::Moved(_) => queue.push(file, None),
                FileDiff::Copied(_) => {
                    // Copies are not supported and should never be generated due to the diff
                    // options we pass.
@@ -297,23 +282,25 @@ impl<'a> ReviewBuilder<'a> {
            let ReviewItem { file, hunk } = item;

            if current.map_or(true, |c| c != file) {
-
                writer.file_header(file)?;
+
                writer.encode(&unified_diff::FileHeader::from(file))?;
                current = Some(file);
            }
            if let Some(h) = hunk {
-
                writer.hunk(h)?;
+
                writer.encode(h)?;
            }

            match self.prompt(&mut stdin, &mut stderr, progress) {
                Some(ReviewAction::Accept) => {
-
                    let mut writer = DiffWriter::new(Vec::new());
-

-
                    writer.file_header(file)?;
-
                    if let Some(h) = hunk {
-
                        writer.hunk(h)?;
+
                    let mut buf = Vec::new();
+
                    {
+
                        let mut writer = unified_diff::Writer::new(&mut buf);
+

+
                        writer.encode(&unified_diff::FileHeader::from(file))?;
+
                        if let Some(h) = hunk {
+
                            writer.encode(h)?;
+
                        }
                    }
-
                    let diff = writer.into_inner();
-
                    let diff = git::raw::Diff::from_buffer(&diff)?;
+
                    let diff = git::raw::Diff::from_buffer(&buf)?;

                    let mut index = repo.apply_to_tree(&brain, &diff, None)?;
                    let brain = index.write_tree_to(repo)?;
deleted radicle-cli/src/commands/review/diff.rs
@@ -1,143 +0,0 @@
-
use std::path::Path;
-
use std::{fmt, io};
-

-
use radicle::git;
-
use radicle_surf::diff::{FileDiff, Hunk, Modification};
-
use radicle_term::ansi::Style;
-

-
use crate::terminal as term;
-

-
/// Writes git-compatible diffs to the given stream.
-
#[derive(Default)]
-
pub struct DiffWriter<W: io::Write> {
-
    styled: bool,
-
    stream: W,
-
}
-

-
impl<W: io::Write> DiffWriter<W> {
-
    /// Create a new writer from a writable stream.
-
    pub fn new(stream: W) -> Self {
-
        Self {
-
            stream,
-
            styled: false,
-
        }
-
    }
-

-
    /// Consume writer and return the underlying stream.
-
    pub fn into_inner(self) -> W {
-
        self.stream
-
    }
-

-
    /// Use styled output or not.
-
    pub fn styled(mut self, choice: bool) -> Self {
-
        self.styled = choice;
-
        self
-
    }
-

-
    /// Write a diff file header.
-
    pub fn file_header(&mut self, file: &FileDiff) -> io::Result<()> {
-
        fn diff(old: &Path, new: &Path) -> String {
-
            format!("diff --git a/{} b/{}", old.display(), new.display())
-
        }
-

-
        match file {
-
            FileDiff::Modified(f) => {
-
                self.meta(diff(&f.path, &f.path))?;
-
                self.meta(format!(
-
                    "index {}..{} {:o}",
-
                    term::format::oid(f.old.oid),
-
                    term::format::oid(f.new.oid),
-
                    u32::from(f.new.mode.clone())
-
                ))?;
-
                self.meta(format!("--- a/{}", f.path.display()))?;
-
                self.meta(format!("+++ b/{}", f.path.display()))?;
-
            }
-
            FileDiff::Added(f) => {
-
                self.meta(diff(&f.path, &f.path))?;
-
                self.meta(format!("new file mode {:o}", u32::from(f.new.mode.clone())))?;
-
                self.meta(format!(
-
                    "index {}..{}",
-
                    term::format::oid(git::raw::Oid::zero()),
-
                    term::format::oid(*f.new.oid),
-
                ))?;
-
                self.meta("--- /dev/null")?;
-
                self.meta(format!("+++ b/{}", f.path.display()))?;
-
            }
-
            FileDiff::Copied(_) => todo!(),
-
            FileDiff::Deleted(f) => {
-
                self.meta(diff(&f.path, &f.path))?;
-
                self.meta(format!(
-
                    "deleted file mode {:o}",
-
                    u32::from(f.old.mode.clone())
-
                ))?;
-
                self.meta(format!(
-
                    "index {}..{}",
-
                    term::format::oid(*f.old.oid),
-
                    term::format::oid(git::raw::Oid::zero()),
-
                ))?;
-
                self.meta(format!("--- a/{}", f.path.display()))?;
-
                self.meta("+++ /dev/null")?;
-
            }
-
            FileDiff::Moved(f) => {
-
                self.meta(diff(&f.old_path, &f.new_path))?;
-
                // Nb. We only display diffs as moves when the file was not changed.
-
                self.meta("similarity index 100%")?;
-
                self.meta(format!("rename from {}", f.old_path.display()))?;
-
                self.meta(format!("rename to {}", f.new_path.display()))?;
-
            }
-
        };
-

-
        Ok(())
-
    }
-

-
    /// Write a diff hunk.
-
    pub fn hunk(&mut self, hunk: &Hunk<Modification>) -> io::Result<()> {
-
        self.magenta(hunk.header.from_utf8_lossy().trim_end())?;
-

-
        for modification in &hunk.lines {
-
            match modification {
-
                Modification::Deletion(radicle_surf::diff::Deletion { line, .. }) => {
-
                    self.deleted(format!(
-
                        "-{}",
-
                        String::from_utf8_lossy(line.as_bytes()).trim_end()
-
                    ))?;
-
                }
-
                Modification::Addition(radicle_surf::diff::Addition { line, .. }) => {
-
                    self.added(format!("+{}", line.from_utf8_lossy()).trim_end())?;
-
                }
-
                Modification::Context { line, .. } => {
-
                    self.context(format!(" {}", line.from_utf8_lossy().trim_end()))?;
-
                }
-
            }
-
        }
-
        Ok(())
-
    }
-

-
    fn write(&mut self, s: impl fmt::Display, style: Style) -> io::Result<()> {
-
        if self.styled {
-
            writeln!(self.stream, "{}", term::Paint::new(s).with_style(style))
-
        } else {
-
            writeln!(self.stream, "{s}")
-
        }
-
    }
-

-
    fn meta(&mut self, s: impl fmt::Display) -> io::Result<()> {
-
        self.write(s, term::Style::new(term::Color::Yellow))
-
    }
-

-
    fn magenta(&mut self, s: impl fmt::Display) -> io::Result<()> {
-
        self.write(s, term::Style::new(term::Color::Magenta))
-
    }
-

-
    fn deleted(&mut self, s: impl fmt::Display) -> io::Result<()> {
-
        self.write(s, term::Style::new(term::Color::Red))
-
    }
-

-
    fn added(&mut self, s: impl fmt::Display) -> io::Result<()> {
-
        self.write(s, term::Style::new(term::Color::Green))
-
    }
-

-
    fn context(&mut self, s: impl fmt::Display) -> io::Result<()> {
-
        self.write(s, term::Style::default().dim())
-
    }
-
}
modified radicle-cli/src/git.rs
@@ -1,4 +1,8 @@
//! Git-related functions and types.
+

+
#[path = "git/unified_diff.rs"]
+
pub mod unified_diff;
+

use std::collections::HashSet;
use std::fmt::Display;
use std::fs::{File, OpenOptions};
added radicle-cli/src/git/unified_diff.rs
@@ -0,0 +1,330 @@
+
//! Formatting support for Git's [diff format](https://git-scm.com/docs/diff-format).
+
use std::fmt;
+
use std::io;
+
use std::path::PathBuf;
+

+
use crate::terminal as term;
+

+
use radicle::git::raw::Oid;
+
use radicle_surf::diff::{Diff, DiffContent, DiffFile, FileDiff, Hunk, Modification};
+

+
/// The kind of FileDiff Header which can be used to print the FileDiff information which precedes
+
/// `Hunks`.
+
#[derive(Debug, Clone, PartialEq)]
+
pub enum FileHeader {
+
    Added {
+
        path: PathBuf,
+
        new: DiffFile,
+
    },
+
    Copied {
+
        old_path: PathBuf,
+
        new_path: PathBuf,
+
    },
+
    Deleted {
+
        path: PathBuf,
+
        old: DiffFile,
+
    },
+
    Modified {
+
        path: PathBuf,
+
        old: DiffFile,
+
        new: DiffFile,
+
    },
+
    Moved {
+
        old_path: PathBuf,
+
        new_path: PathBuf,
+
    },
+
}
+

+
impl std::convert::From<&FileDiff> for FileHeader {
+
    // TODO: Pathnames with 'unusual names' need to be quoted.
+
    fn from(value: &FileDiff) -> Self {
+
        match value {
+
            FileDiff::Modified(v) => FileHeader::Modified {
+
                path: v.path.clone(),
+
                old: v.old.clone(),
+
                new: v.new.clone(),
+
            },
+
            FileDiff::Added(v) => FileHeader::Added {
+
                path: v.path.clone(),
+
                new: v.new.clone(),
+
            },
+
            FileDiff::Copied(_) => todo!(),
+
            FileDiff::Deleted(v) => FileHeader::Deleted {
+
                path: v.path.clone(),
+
                old: v.old.clone(),
+
            },
+
            FileDiff::Moved(v) => FileHeader::Moved {
+
                old_path: v.old_path.clone(),
+
                new_path: v.new_path.clone(),
+
            },
+
        }
+
    }
+
}
+

+
/// Meta data which precedes a `Hunk`s content.
+
///
+
/// For example:
+
/// @@ -24,8 +24,6 @@ use radicle_surf::diff::*;
+
#[derive(Clone, Debug, Default, PartialEq)]
+
pub struct HunkHeader {
+
    /// Line the hunk started in the old file.
+
    pub old_line_no: usize,
+
    /// Number of removed and context lines.
+
    pub old_size: usize,
+
    /// Line the hunk started in the new file.
+
    pub new_line_no: usize,
+
    /// Number of added and context lines.
+
    pub new_size: usize,
+
    /// Trailing text for the Hunk Header.
+
    ///
+
    /// From Git's documentation "Hunk headers mention the name of the function to which the hunk
+
    /// applies. See "Defining a custom hunk-header" in gitattributes for details of how to tailor
+
    /// to this to specific languages.".  It is likely best to leave this empty when generating
+
    /// diffs.
+
    pub text: Vec<u8>,
+
}
+

+
/// A Trait for converting a value to its UnifiedDiff format.
+
pub trait UnifiedDiff: Clone {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()>;
+

+
    fn to_unified_string(self) -> String {
+
        let mut buf = Vec::new();
+

+
        {
+
            let mut w = Writer::new(&mut buf);
+
            w.encode(self).unwrap();
+
        }
+

+
        String::from_utf8(buf).unwrap()
+
    }
+
}
+

+
impl UnifiedDiff for &Diff {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        for fdiff in self.files() {
+
            fdiff.encode(w)?;
+
        }
+

+
        Ok(())
+
    }
+
}
+

+
impl UnifiedDiff for &DiffContent {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        match self {
+
            DiffContent::Plain { hunks, .. } => {
+
                for h in hunks.iter() {
+
                    h.encode(w)?;
+
                }
+
                Ok(())
+
            }
+
            DiffContent::Empty => Ok(()),
+
            DiffContent::Binary => unimplemented!(),
+
        }
+
    }
+
}
+

+
impl UnifiedDiff for FileDiff {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        w.encode(&FileHeader::from(self))?;
+
        match self {
+
            FileDiff::Modified(f) => {
+
                w.encode(&f.diff)?;
+
            }
+
            FileDiff::Added(f) => {
+
                w.encode(&f.diff)?;
+
            }
+
            FileDiff::Copied(f) => {
+
                w.encode(&f.diff)?;
+
            }
+
            FileDiff::Deleted(f) => {
+
                w.encode(&f.diff)?;
+
            }
+
            FileDiff::Moved(f) => {
+
                // Nb. We only display diffs as moves when the file was not changed.
+
                w.encode(&f.diff)?;
+
            }
+
        }
+

+
        Ok(())
+
    }
+
}
+

+
impl UnifiedDiff for &FileHeader {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        match self {
+
            FileHeader::Modified { path, old, new } => {
+
                w.meta(format!(
+
                    "diff --git a/{} b/{}",
+
                    path.display(),
+
                    path.display()
+
                ))?;
+

+
                if old.mode == new.mode {
+
                    w.meta(format!(
+
                        "index {}..{} {:o}",
+
                        term::format::oid(old.oid),
+
                        term::format::oid(new.oid),
+
                        u32::from(old.mode.clone()),
+
                    ))?;
+
                } else {
+
                    w.meta(format!("old mode {:o}", u32::from(old.mode.clone())))?;
+
                    w.meta(format!("new mode {:o}", u32::from(new.mode.clone())))?;
+
                    w.meta(format!(
+
                        "index {}..{}",
+
                        term::format::oid(old.oid),
+
                        term::format::oid(new.oid)
+
                    ))?;
+
                }
+

+
                w.meta(format!("--- a/{}", path.display()))?;
+
                w.meta(format!("+++ b/{}", path.display()))?;
+
            }
+
            FileHeader::Added { path, new } => {
+
                w.meta(format!(
+
                    "diff --git a/{} b/{}",
+
                    path.display(),
+
                    path.display()
+
                ))?;
+

+
                w.meta(format!("new file mode {:o}", u32::from(new.mode.clone())))?;
+
                w.meta(format!(
+
                    "index {}..{}",
+
                    term::format::oid(Oid::zero()),
+
                    term::format::oid(new.oid),
+
                ))?;
+

+
                w.meta("--- /dev/null")?;
+
                w.meta(format!("+++ b/{}", path.display()))?;
+
            }
+
            FileHeader::Copied { .. } => todo!(),
+
            FileHeader::Deleted { path, old } => {
+
                w.meta(format!(
+
                    "diff --git a/{} b/{}",
+
                    path.display(),
+
                    path.display()
+
                ))?;
+

+
                w.meta(format!(
+
                    "deleted file mode {:o}",
+
                    u32::from(old.mode.clone())
+
                ))?;
+
                w.meta(format!(
+
                    "index {}..{}",
+
                    term::format::oid(old.oid),
+
                    term::format::oid(Oid::zero())
+
                ))?;
+

+
                w.meta(format!("--- a/{}", path.display()))?;
+
                w.meta("+++ /dev/null".to_string())?;
+
            }
+
            FileHeader::Moved { old_path, new_path } => {
+
                w.meta(format!(
+
                    "diff --git a/{} b/{}",
+
                    old_path.display(),
+
                    new_path.display()
+
                ))?;
+
                w.meta("similarity index 100%")?;
+
                w.meta(format!("rename from {}", old_path.display()))?;
+
                w.meta(format!("rename to {}", new_path.display()))?;
+
            }
+
        };
+
        Ok(())
+
    }
+
}
+

+
impl UnifiedDiff for &HunkHeader {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        let old = if self.old_size == 1 {
+
            format!("{}", self.old_line_no)
+
        } else {
+
            format!("{},{}", self.old_line_no, self.old_size)
+
        };
+
        let new = if self.new_size == 1 {
+
            format!("{}", self.new_line_no)
+
        } else {
+
            format!("{},{}", self.new_line_no, self.new_size)
+
        };
+
        let text = if self.text.is_empty() {
+
            "".to_string()
+
        } else {
+
            format!(" {}", String::from_utf8_lossy(&self.text))
+
        };
+

+
        w.meta(format!("@@ -{old} +{new} @@{text}"))
+
    }
+
}
+

+
impl UnifiedDiff for &Hunk<Modification> {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        // TODO: Remove trailing newlines accurately.
+
        //   trim_end() will destroy diff information if the diff has a trailing whitespace on
+
        //   purpose.
+
        w.magenta(self.header.from_utf8_lossy().trim_end())?;
+
        for l in &self.lines {
+
            l.encode(w)?;
+
        }
+

+
        Ok(())
+
    }
+
}
+

+
impl UnifiedDiff for &Modification {
+
    fn encode(&self, w: &mut Writer) -> io::Result<()> {
+
        match self {
+
            Modification::Deletion(radicle_surf::diff::Deletion { line, .. }) => {
+
                let s = format!("-{}", String::from_utf8_lossy(line.as_bytes()).trim_end());
+
                w.write(s, term::Style::new(term::Color::Red))
+
            }
+
            Modification::Addition(radicle_surf::diff::Addition { line, .. }) => {
+
                let s = format!("+{}", String::from_utf8_lossy(line.as_bytes()).trim_end());
+
                w.write(s, term::Style::new(term::Color::Green))
+
            }
+
            Modification::Context { line, .. } => {
+
                let s = format!(" {}", String::from_utf8_lossy(line.as_bytes()).trim_end());
+
                w.write(s, term::Style::default().dim())
+
            }
+
        }
+
    }
+
}
+

+
/// An IO Writer with color printing to the terminal.
+
pub struct Writer<'a> {
+
    styled: bool,
+
    stream: Box<dyn io::Write + 'a>,
+
}
+

+
impl<'a> Writer<'a> {
+
    pub fn new(w: impl io::Write + 'a) -> Self {
+
        Self {
+
            styled: false,
+
            stream: Box::new(w),
+
        }
+
    }
+

+
    pub fn encode(&mut self, arg: impl UnifiedDiff) -> io::Result<()> {
+
        arg.encode(self)
+
    }
+

+
    pub fn styled(mut self, value: bool) -> Self {
+
        self.styled = value;
+
        self
+
    }
+

+
    fn write(&mut self, s: impl fmt::Display, style: term::Style) -> io::Result<()> {
+
        if self.styled {
+
            writeln!(self.stream, "{}", term::Paint::new(s).with_style(style))
+
        } else {
+
            writeln!(self.stream, "{s}")
+
        }
+
    }
+

+
    fn meta(&mut self, s: impl fmt::Display) -> io::Result<()> {
+
        self.write(s, term::Style::new(term::Color::Yellow))
+
    }
+

+
    fn magenta(&mut self, s: impl fmt::Display) -> io::Result<()> {
+
        self.write(s, term::Style::new(term::Color::Magenta))
+
    }
+
}