diff --git a/surf/src/diff.rs b/surf/src/diff.rs index 4821cf0..5ef20a4 100644 --- a/surf/src/diff.rs +++ b/surf/src/diff.rs @@ -17,15 +17,14 @@ #![allow(dead_code, unused_variables, missing_docs)] -use crate::file_system::{Directory, DirectoryContents, Path}; -use std::{cell::RefCell, cmp::Ordering, ops::Deref, rc::Rc}; -use thiserror::Error; +use std::{cell::RefCell, cmp::Ordering, convert::TryFrom, ops::Deref, rc::Rc, slice}; #[cfg(feature = "serialize")] use serde::{ser, Serialize, Serializer}; -pub mod git; +use crate::file_system::{Directory, DirectoryContents, Path}; +pub mod git; #[cfg_attr( feature = "serialize", @@ -118,7 +117,7 @@ pub enum FileDiff { Binary, #[cfg_attr(feature = "serialize", serde(rename_all = "camelCase"))] Plain { - hunks: Vec, + hunks: Hunks, }, } @@ -134,6 +133,58 @@ pub struct Hunk { pub lines: Vec, } +/// A set of [`Hunk`]s. +#[cfg_attr(feature = "serialize", derive(Serialize))] +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct Hunks(pub Vec); + +pub struct IterHunks<'a> { + inner: slice::Iter<'a, Hunk>, +} + +impl Hunks { + pub fn iter(&self) -> IterHunks<'_> { + IterHunks { + inner: self.0.iter(), + } + } +} + +impl From> for Hunks { + fn from(hunks: Vec) -> Self { + Self(hunks) + } +} + +impl<'a> Iterator for IterHunks<'a> { + type Item = &'a Hunk; + + fn next(&mut self) -> Option { + self.inner.next() + } +} + +impl TryFrom> for Hunks { + type Error = git::error::Hunk; + + fn try_from(patch: git2::Patch) -> Result { + let mut hunks = Vec::new(); + for h in 0..patch.num_hunks() { + let (hunk, hunk_lines) = patch.hunk(h)?; + let header = Line(hunk.header().to_owned()); + let mut lines: Vec = Vec::new(); + + for l in 0..hunk_lines { + let line = patch.line_in_hunk(h, l)?; + let line = LineDiff::try_from(line)?; + lines.push(line); + } + hunks.push(Hunk { header, lines }); + } + Ok(Hunks(hunks)) + } +} + /// The content of a single line. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Line(pub(crate) Vec); @@ -297,7 +348,12 @@ impl Diff { let mut path = parent_path.borrow().clone(); path.push(new_file_name.clone()); - diff.add_created_file(path, FileDiff::Plain { hunks: vec![] }); + diff.add_created_file( + path, + FileDiff::Plain { + hunks: Hunks::default(), + }, + ); diff.add_deleted_files(old_entry, parent_path); old_entry_opt = old_iter.next(); @@ -314,7 +370,12 @@ impl Diff { path.push(old_file_name.clone()); diff.add_created_files(new_entry, parent_path); - diff.add_deleted_file(path, FileDiff::Plain { hunks: vec![] }); + diff.add_deleted_file( + path, + FileDiff::Plain { + hunks: Hunks::default(), + }, + ); old_entry_opt = old_iter.next(); new_entry_opt = new_iter.next(); @@ -407,7 +468,7 @@ impl Diff { pub(crate) fn add_modified_file( &mut self, path: Path, - hunks: Vec, + hunks: impl Into, eof: Option, ) { // TODO: file diff can be calculated at this point @@ -415,7 +476,9 @@ impl Diff { // https://nest.pijul.com/pijul_org/pijul:master/1468b7281a6f3785e9#anesp4Qdq3V self.modified.push(ModifiedFile { path, - diff: FileDiff::Plain { hunks }, + diff: FileDiff::Plain { + hunks: hunks.into(), + }, eof, }); } @@ -444,7 +507,9 @@ impl Diff { let mut new_files: Vec = Diff::collect_files_from_entry(dc, parent_path, |path| CreateFile { path, - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }); self.created.append(&mut new_files); } @@ -457,7 +522,9 @@ impl Diff { let mut new_files: Vec = Diff::collect_files_from_entry(dc, parent_path, |path| DeleteFile { path, - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }); self.deleted.append(&mut new_files); } @@ -483,7 +550,9 @@ mod tests { let expected_diff = Diff { created: vec![CreateFile { path: Path::with_root(&[unsound::label::new("banana.rs")]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }], deleted: vec![], copied: vec![], @@ -507,7 +576,9 @@ mod tests { created: vec![], deleted: vec![DeleteFile { path: Path::with_root(&[unsound::label::new("banana.rs")]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }], moved: vec![], copied: vec![], @@ -549,7 +620,9 @@ mod tests { copied: vec![], modified: vec![ModifiedFile { path: Path::with_root(&[unsound::label::new("banana.rs")]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, eof: None, }], }; @@ -575,7 +648,9 @@ mod tests { unsound::label::new("src"), unsound::label::new("banana.rs"), ]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }], deleted: vec![], moved: vec![], @@ -605,7 +680,9 @@ mod tests { unsound::label::new("src"), unsound::label::new("banana.rs"), ]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, }], moved: vec![], copied: vec![], @@ -641,7 +718,9 @@ mod tests { unsound::label::new("src"), unsound::label::new("banana.rs"), ]), - diff: FileDiff::Plain { hunks: vec![] }, + diff: FileDiff::Plain { + hunks: Hunks::default(), + }, eof: None, }], }; diff --git a/surf/src/diff/git.rs b/surf/src/diff/git.rs index 8a47294..7c1d91c 100644 --- a/surf/src/diff/git.rs +++ b/surf/src/diff/git.rs @@ -17,47 +17,74 @@ use std::convert::TryFrom; -use thiserror::Error; - use crate::{ - diff::{self, Diff, EofNewLine, Hunk, Line, LineDiff}, + diff::{self, Diff, EofNewLine, Hunk, Hunks, Line, LineDiff}, file_system::Path, - vcs, }; -/// A Git diff error. -#[derive(Debug, PartialEq, Error)] -#[non_exhaustive] -pub enum Error { - /// A The path of a file isn't available. - #[error("couldn't retrieve file path")] - PathUnavailable, - /// A patch is unavailable. - #[error("couldn't retrieve patch for {0}")] - PatchUnavailable(Path), - /// A Git delta type isn't currently handled. - #[error("git delta type is not handled")] - DeltaUnhandled(git2::Delta), - /// A Git `DiffLine` is invalid. - #[error("invalid `git2::DiffLine`")] - InvalidLineDiff, +pub mod error { + use thiserror::Error; + + use crate::file_system::{self, Path}; + + #[derive(Debug, Error, PartialEq)] + #[non_exhaustive] + pub enum LineDiff { + /// A Git `DiffLine` is invalid. + #[error( + "invalid `git2::DiffLine` which contains no line numbers for either side of the diff" + )] + Invalid, + } + + #[derive(Debug, Error, PartialEq)] + #[non_exhaustive] + pub enum Hunk { + #[error(transparent)] + Git(#[from] git2::Error), + #[error(transparent)] + Line(#[from] LineDiff), + } + + /// A Git diff error. + #[derive(Debug, PartialEq, Error)] + #[non_exhaustive] + pub enum Diff { + /// A Git delta type isn't currently handled. + #[error("git delta type is not handled")] + DeltaUnhandled(git2::Delta), + #[error(transparent)] + FileSystem(#[from] file_system::Error), + #[error(transparent)] + Git(#[from] git2::Error), + #[error(transparent)] + Hunk(#[from] Hunk), + #[error(transparent)] + Line(#[from] LineDiff), + /// A patch is unavailable. + #[error("couldn't retrieve patch for {0}")] + PatchUnavailable(Path), + /// A The path of a file isn't available. + #[error("couldn't retrieve file path")] + PathUnavailable, + } } impl<'a> TryFrom> for LineDiff { - type Error = Error; + type Error = error::LineDiff; fn try_from(line: git2::DiffLine) -> Result { match (line.old_lineno(), line.new_lineno()) { (None, Some(n)) => Ok(Self::addition(line.content().to_owned(), n)), (Some(n), None) => Ok(Self::deletion(line.content().to_owned(), n)), (Some(l), Some(r)) => Ok(Self::context(line.content().to_owned(), l, r)), - (None, None) => Err(Error::InvalidLineDiff), + (None, None) => Err(error::LineDiff::Invalid), } } } impl<'a> TryFrom> for Diff { - type Error = vcs::git::error::Error; + type Error = error::Diff; fn try_from(git_diff: git2::Diff) -> Result { use git2::{Delta, Patch}; @@ -68,59 +95,51 @@ impl<'a> TryFrom> for Diff { match delta.status() { Delta::Added => { let diff_file = delta.new_file(); - let path = diff_file.path().ok_or(diff::git::Error::PathUnavailable)?; + let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; let path = Path::try_from(path.to_path_buf())?; let patch = Patch::from_diff(&git_diff, idx)?; if let Some(patch) = patch { - let mut hunks: Vec = Vec::new(); - - for h in 0..patch.num_hunks() { - let (hunk, hunk_lines) = patch.hunk(h)?; - let header = Line(hunk.header().to_owned()); - let mut lines: Vec = Vec::new(); - - for l in 0..hunk_lines { - let line = patch.line_in_hunk(h, l)?; - let line = LineDiff::try_from(line)?; - lines.push(line); - } - hunks.push(Hunk { header, lines }); - } - diff.add_created_file(path, diff::FileDiff::Plain { hunks }); + diff.add_created_file( + path, + diff::FileDiff::Plain { + hunks: Hunks::try_from(patch)?, + }, + ); } else { - diff.add_created_file(path, diff::FileDiff::Plain { hunks: vec![] }); + diff.add_created_file( + path, + diff::FileDiff::Plain { + hunks: Hunks::default(), + }, + ); } }, Delta::Deleted => { let diff_file = delta.old_file(); - let path = diff_file.path().ok_or(diff::git::Error::PathUnavailable)?; + let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; let path = Path::try_from(path.to_path_buf())?; let patch = Patch::from_diff(&git_diff, idx)?; if let Some(patch) = patch { - let mut hunks: Vec = Vec::new(); - - for h in 0..patch.num_hunks() { - let (hunk, hunk_lines) = patch.hunk(h)?; - let header = Line(hunk.header().to_owned()); - let mut lines: Vec = Vec::new(); - - for l in 0..hunk_lines { - let line = patch.line_in_hunk(h, l)?; - let line = LineDiff::try_from(line)?; - lines.push(line); - } - hunks.push(Hunk { header, lines }); - } - diff.add_deleted_file(path, diff::FileDiff::Plain { hunks }); + diff.add_deleted_file( + path, + diff::FileDiff::Plain { + hunks: Hunks::try_from(patch)?, + }, + ); } else { - diff.add_deleted_file(path, diff::FileDiff::Plain { hunks: vec![] }); + diff.add_deleted_file( + path, + diff::FileDiff::Plain { + hunks: Hunks::default(), + }, + ); } }, Delta::Modified => { let diff_file = delta.new_file(); - let path = diff_file.path().ok_or(diff::git::Error::PathUnavailable)?; + let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; let path = Path::try_from(path.to_path_buf())?; let patch = Patch::from_diff(&git_diff, idx)?; @@ -168,18 +187,18 @@ impl<'a> TryFrom> for Diff { } else if diff_file.is_binary() { diff.add_modified_binary_file(path); } else { - return Err(diff::git::Error::PatchUnavailable(path).into()); + return Err(error::Diff::PatchUnavailable(path)); } }, Delta::Renamed => { let old = delta .old_file() .path() - .ok_or(diff::git::Error::PathUnavailable)?; + .ok_or(error::Diff::PathUnavailable)?; let new = delta .new_file() .path() - .ok_or(diff::git::Error::PathUnavailable)?; + .ok_or(error::Diff::PathUnavailable)?; let old_path = Path::try_from(old.to_path_buf())?; let new_path = Path::try_from(new.to_path_buf())?; @@ -190,11 +209,11 @@ impl<'a> TryFrom> for Diff { let old = delta .old_file() .path() - .ok_or(diff::git::Error::PathUnavailable)?; + .ok_or(error::Diff::PathUnavailable)?; let new = delta .new_file() .path() - .ok_or(diff::git::Error::PathUnavailable)?; + .ok_or(error::Diff::PathUnavailable)?; let old_path = Path::try_from(old.to_path_buf())?; let new_path = Path::try_from(new.to_path_buf())?; @@ -202,7 +221,7 @@ impl<'a> TryFrom> for Diff { diff.add_copied_file(old_path, new_path); }, status => { - return Err(diff::git::Error::DeltaUnhandled(status).into()); + return Err(error::Diff::DeltaUnhandled(status)); }, } } diff --git a/surf/src/vcs/git.rs b/surf/src/vcs/git.rs index d4a1afa..4b80fad 100644 --- a/surf/src/vcs/git.rs +++ b/surf/src/vcs/git.rs @@ -1561,7 +1561,7 @@ mod tests { lines: vec![ LineDiff::addition(b"This repository is a data source for the Upstream front-end tests.\n".to_vec(), 1), ] - }] + }].into() }, }], deleted: vec![], @@ -1605,7 +1605,7 @@ mod tests { LineDiff::addition(b"This repository is a data source for the Upstream front-end tests and the\n".to_vec(), 1), LineDiff::addition(b"[`radicle-surf`](https://github.com/radicle-dev/git-platinum) unit tests.\n".to_vec(), 2), ] - }] + }].into() }, eof: None, }] @@ -1621,7 +1621,7 @@ mod tests { use file_system::*; let diff = Diff { - created: vec![CreateFile{path: unsound::path::new("LICENSE"), diff: FileDiff::Plain { hunks: vec![] }}], + created: vec![CreateFile{path: unsound::path::new("LICENSE"), diff: FileDiff::Plain { hunks: Hunks::default() }}], deleted: vec![], moved: vec![ MoveFile { @@ -1641,7 +1641,7 @@ mod tests { LineDiff::addition(b"[`radicle-surf`](https://github.com/radicle-dev/git-platinum) unit tests.\n".to_vec(), 2), LineDiff::context(b"\n".to_vec(), 3, 4), ] - }] + }].into() }, eof: None, }] diff --git a/surf/src/vcs/git/error.rs b/surf/src/vcs/git/error.rs index 580dfa5..5e4c23d 100644 --- a/surf/src/vcs/git/error.rs +++ b/surf/src/vcs/git/error.rs @@ -84,7 +84,7 @@ pub enum Error { PathNotFound(file_system::Path), /// An error that comes from performing a *diff* operations. #[error(transparent)] - Diff(#[from] diff::git::Error), + Diff(#[from] diff::git::error::Diff), /// A wrapper around the generic [`git2::Error`]. #[error(transparent)] Git(#[from] git2::Error), diff --git a/surf/src/vcs/git/repo.rs b/surf/src/vcs/git/repo.rs index 1a86736..f3e38f5 100644 --- a/surf/src/vcs/git/repo.rs +++ b/surf/src/vcs/git/repo.rs @@ -157,12 +157,13 @@ impl<'a> RepositoryRef<'a> { /// Get the [`Diff`] between two commits. pub fn diff(&self, from: Oid, to: Oid) -> Result { self.diff_commits(None, Some(from), to) - .and_then(Diff::try_from) + .and_then(|diff| Diff::try_from(diff).map_err(Error::from)) } /// Get the [`Diff`] of a commit with no parents. pub fn initial_diff(&self, oid: Oid) -> Result { - self.diff_commits(None, None, oid).and_then(Diff::try_from) + self.diff_commits(None, None, oid) + .and_then(|diff| Diff::try_from(diff).map_err(Error::from)) } /// Parse an [`Oid`] from the given string.