diff --git a/radicle-cli/src/commands/review.rs b/radicle-cli/src/commands/review.rs index 5783acae..ff822e0a 100644 --- a/radicle-cli/src/commands/review.rs +++ b/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; diff --git a/radicle-cli/src/commands/review/builder.rs b/radicle-cli/src/commands/review/builder.rs index bf212afa..9e600da1 100644 --- a/radicle-cli/src/commands/review/builder.rs +++ b/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()); + let mut buf = Vec::new(); + { + let mut writer = unified_diff::Writer::new(&mut buf); - writer.file_header(file)?; - if let Some(h) = hunk { - writer.hunk(h)?; + 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)?; diff --git a/radicle-cli/src/commands/review/diff.rs b/radicle-cli/src/commands/review/diff.rs deleted file mode 100644 index 613d15cb..00000000 --- a/radicle-cli/src/commands/review/diff.rs +++ /dev/null @@ -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 { - styled: bool, - stream: W, -} - -impl DiffWriter { - /// 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) -> 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()) - } -} diff --git a/radicle-cli/src/git.rs b/radicle-cli/src/git.rs index 5cc28da3..fe85d422 100644 --- a/radicle-cli/src/git.rs +++ b/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}; diff --git a/radicle-cli/src/git/unified_diff.rs b/radicle-cli/src/git/unified_diff.rs new file mode 100644 index 00000000..36f5b045 --- /dev/null +++ b/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, +} + +/// 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 { + 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, +} + +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)) + } +}