Radish alpha
r
rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt
Git libraries for Radicle
Radicle
Git
Merge remote-tracking branch 'origin/diff-eof'
Fintan Halpenny committed 3 years ago
commit 5fdccdc8f032486b90963ba9dbb31b25836f9de3
parent aa6459c
3 files changed +113 -202
modified radicle-surf/src/diff.rs
@@ -90,16 +90,8 @@ impl Diff {
        &self.stats
    }

-
    fn insert_modified(
-
        &mut self,
-
        path: PathBuf,
-
        hunks: impl Into<Hunks<Modification>>,
-
        eof: Option<EofNewLine>,
-
    ) {
-
        let diff = DiffContent::Plain {
-
            hunks: hunks.into(),
-
        };
-
        let diff = FileDiff::Modified(Modified { path, eof, diff });
+
    fn insert_modified(&mut self, path: PathBuf, diff: DiffContent) {
+
        let diff = FileDiff::Modified(Modified { path, diff });
        self.files.push(diff)
    }

@@ -121,15 +113,6 @@ impl Diff {
        self.files.push(diff);
    }

-
    fn insert_modified_binary(&mut self, path: PathBuf) {
-
        let diff = FileDiff::Modified(Modified {
-
            path,
-
            eof: None,
-
            diff: DiffContent::Binary,
-
        });
-
        self.files.push(diff)
-
    }
-

    fn insert_added(&mut self, path: PathBuf, diff: DiffContent) {
        let diff = FileDiff::Added(Added { path, diff });
        self.files.push(diff);
@@ -201,17 +184,21 @@ pub enum EofNewLine {
    OldMissing,
    NewMissing,
    BothMissing,
+
    NoneMissing,
+
}
+

+
impl Default for EofNewLine {
+
    fn default() -> Self {
+
        Self::NoneMissing
+
    }
}

/// A file that was modified within a [`Diff`].
#[cfg_attr(feature = "serde", derive(Serialize), serde(rename_all = "camelCase"))]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Modified {
-
    /// The path to this file, relative to the repository root.
    pub path: PathBuf,
    pub diff: DiffContent,
-
    /// Was there an EOF newline present.
-
    pub eof: Option<EofNewLine>,
}

/// The set of changes for a given file.
@@ -228,10 +215,20 @@ pub enum DiffContent {
    #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
    Plain {
        hunks: Hunks<Modification>,
+
        eof: EofNewLine,
    },
    Empty,
}

+
impl DiffContent {
+
    pub fn eof(&self) -> Option<EofNewLine> {
+
        match self {
+
            Self::Plain { hunks: _, eof } => Some(eof.clone()),
+
            _ => None,
+
        }
+
    }
+
}
+

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum FileDiff {
    Added(Added),
@@ -260,7 +257,6 @@ impl Serialize for FileDiff {
            FileDiff::Modified(x) => {
                state.serialize_field("path", &x.path)?;
                state.serialize_field("diff", &x.diff)?;
-
                state.serialize_field("eof", &x.eof)?
            },
            FileDiff::Moved(x) => {
                state.serialize_field("oldPath", &x.old_path)?;
modified radicle-surf/src/diff/git.rs
@@ -17,7 +17,7 @@

use std::convert::TryFrom;

-
use crate::diff::{self, Addition, Deletion, Diff, EofNewLine, Hunk, Hunks, Line, Modification};
+
use super::{Diff, DiffContent, EofNewLine, Hunk, Hunks, Line, Modification, Stats};

pub mod error {
    use std::path::PathBuf;
@@ -87,11 +87,14 @@ pub mod error {
    }
}

-
impl TryFrom<git2::Patch<'_>> for Hunks<Modification> {
+
impl TryFrom<git2::Patch<'_>> for DiffContent {
    type Error = error::Hunk;

    fn try_from(patch: git2::Patch) -> Result<Self, Self::Error> {
        let mut hunks = Vec::new();
+
        let mut old_missing_eof = false;
+
        let mut new_missing_eof = false;
+

        for h in 0..patch.num_hunks() {
            let (hunk, hunk_lines) = patch.hunk(h)?;
            let header = Line(hunk.header().to_owned());
@@ -99,52 +102,37 @@ impl TryFrom<git2::Patch<'_>> for Hunks<Modification> {

            for l in 0..hunk_lines {
                let line = patch.line_in_hunk(h, l)?;
+
                match line.origin_value() {
+
                    git2::DiffLineType::ContextEOFNL => {
+
                        new_missing_eof = true;
+
                        old_missing_eof = true;
+
                        continue;
+
                    },
+
                    git2::DiffLineType::AddEOFNL => {
+
                        old_missing_eof = true;
+
                        continue;
+
                    },
+
                    git2::DiffLineType::DeleteEOFNL => {
+
                        new_missing_eof = true;
+
                        continue;
+
                    },
+
                    _ => {},
+
                }
                let line = Modification::try_from(line)?;
                lines.push(line);
            }
            hunks.push(Hunk { header, lines });
        }
-
        Ok(Hunks(hunks))
-
    }
-
}
-

-
impl TryFrom<git2::Patch<'_>> for Hunks<Addition> {
-
    type Error = error::Addition;
-

-
    fn try_from(patch: git2::Patch) -> Result<Self, Self::Error> {
-
        let mut hunks = Vec::with_capacity(patch.num_hunks());
-
        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<Addition> = Vec::with_capacity(hunk_lines);
-

-
            for l in 0..hunk_lines {
-
                let line = patch.line_in_hunk(h, l)?;
-
                lines.push(Addition::try_from(line)?);
-
            }
-
            hunks.push(Hunk { header, lines })
-
        }
-
        Ok(hunks.into())
-
    }
-
}
-

-
impl TryFrom<git2::Patch<'_>> for Hunks<Deletion> {
-
    type Error = error::Deletion;
-

-
    fn try_from(patch: git2::Patch) -> Result<Self, Self::Error> {
-
        let mut hunks = Vec::with_capacity(patch.num_hunks());
-
        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<Deletion> = Vec::with_capacity(hunk_lines);
-

-
            for l in 0..hunk_lines {
-
                let line = patch.line_in_hunk(h, l)?;
-
                lines.push(Deletion::try_from(line)?);
-
            }
-
            hunks.push(Hunk { header, lines })
-
        }
-
        Ok(hunks.into())
+
        let eof = match (old_missing_eof, new_missing_eof) {
+
            (true, true) => EofNewLine::BothMissing,
+
            (true, false) => EofNewLine::OldMissing,
+
            (false, true) => EofNewLine::NewMissing,
+
            (false, false) => EofNewLine::NoneMissing,
+
        };
+
        Ok(DiffContent::Plain {
+
            hunks: Hunks(hunks),
+
            eof,
+
        })
    }
}

@@ -161,37 +149,7 @@ impl<'a> TryFrom<git2::DiffLine<'a>> for Modification {
    }
}

-
impl TryFrom<git2::DiffLine<'_>> for Addition {
-
    type Error = error::Addition;
-

-
    fn try_from(line: git2::DiffLine) -> Result<Self, Self::Error> {
-
        debug_assert!(
-
            line.old_lineno().is_none(),
-
            "trying to build Addition with a modified line"
-
        );
-
        Ok(Self {
-
            line: line.content().to_owned().into(),
-
            line_no: line.new_lineno().ok_or(error::Addition::MissingNewLineNo)?,
-
        })
-
    }
-
}
-

-
impl TryFrom<git2::DiffLine<'_>> for Deletion {
-
    type Error = error::Deletion;
-

-
    fn try_from(line: git2::DiffLine) -> Result<Self, Self::Error> {
-
        debug_assert!(
-
            line.new_lineno().is_none(),
-
            "trying to build Deletion with a modified line"
-
        );
-
        Ok(Self {
-
            line: line.content().to_owned().into(),
-
            line_no: line.old_lineno().ok_or(error::Deletion::MissingOldLineNo)?,
-
        })
-
    }
-
}
-

-
impl From<git2::DiffStats> for diff::Stats {
+
impl From<git2::DiffStats> for Stats {
    fn from(stats: git2::DiffStats) -> Self {
        Self {
            files_changed: stats.files_changed(),
@@ -241,19 +199,11 @@ fn created(

    let patch = git2::Patch::from_diff(git_diff, idx)?;
    if let Some(patch) = patch {
-
        diff.insert_added(
-
            path,
-
            diff::DiffContent::Plain {
-
                hunks: Hunks::try_from(patch)?,
-
            },
-
        );
+
        diff.insert_added(path, DiffContent::try_from(patch)?);
+
    } else if diff_file.is_binary() {
+
        diff.insert_added(path, DiffContent::Binary);
    } else {
-
        diff.insert_added(
-
            path,
-
            diff::DiffContent::Plain {
-
                hunks: Hunks::default(),
-
            },
-
        );
+
        return Err(error::Diff::PatchUnavailable(path));
    }
    Ok(())
}
@@ -271,19 +221,11 @@ fn deleted(
        .to_path_buf();
    let patch = git2::Patch::from_diff(git_diff, idx)?;
    if let Some(patch) = patch {
-
        diff.insert_deleted(
-
            path,
-
            diff::DiffContent::Plain {
-
                hunks: Hunks::try_from(patch)?,
-
            },
-
        );
+
        diff.insert_deleted(path, DiffContent::try_from(patch)?);
+
    } else if diff_file.is_binary() {
+
        diff.insert_deleted(path, DiffContent::Binary);
    } else {
-
        diff.insert_deleted(
-
            path,
-
            diff::DiffContent::Plain {
-
                hunks: Hunks::default(),
-
            },
-
        );
+
        return Err(error::Diff::PatchUnavailable(path));
    }
    Ok(())
}
@@ -302,48 +244,10 @@ fn modified(
    let patch = git2::Patch::from_diff(git_diff, idx)?;

    if let Some(patch) = patch {
-
        let mut hunks: Vec<Hunk<Modification>> = Vec::new();
-
        let mut old_missing_eof = false;
-
        let mut new_missing_eof = false;
-

-
        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<Modification> = Vec::new();
-

-
            for l in 0..hunk_lines {
-
                let line = patch.line_in_hunk(h, l)?;
-
                match line.origin_value() {
-
                    git2::DiffLineType::ContextEOFNL => {
-
                        new_missing_eof = true;
-
                        old_missing_eof = true;
-
                        continue;
-
                    },
-
                    git2::DiffLineType::AddEOFNL => {
-
                        old_missing_eof = true;
-
                        continue;
-
                    },
-
                    git2::DiffLineType::DeleteEOFNL => {
-
                        new_missing_eof = true;
-
                        continue;
-
                    },
-
                    _ => {},
-
                }
-
                let line = Modification::try_from(line)?;
-
                lines.push(line);
-
            }
-
            hunks.push(Hunk { header, lines });
-
        }
-
        let eof = match (old_missing_eof, new_missing_eof) {
-
            (true, true) => Some(EofNewLine::BothMissing),
-
            (true, false) => Some(EofNewLine::OldMissing),
-
            (false, true) => Some(EofNewLine::NewMissing),
-
            (false, false) => None,
-
        };
-
        diff.insert_modified(path, hunks, eof);
+
        diff.insert_modified(path, DiffContent::try_from(patch)?);
        Ok(())
    } else if diff_file.is_binary() {
-
        diff.insert_modified_binary(path);
+
        diff.insert_modified(path, DiffContent::Binary);
        Ok(())
    } else {
        Err(error::Diff::PatchUnavailable(path))
modified radicle-surf/t/src/diff.rs
@@ -45,6 +45,7 @@ fn test_initial_diff() -> Result<(), Error> {
                )],
            }]
            .into(),
+
            eof: EofNewLine::default(),
        },
    })];

@@ -79,7 +80,6 @@ fn test_diff_file() -> Result<(), Error> {
    )?;
    let expected_diff = FileDiff::Modified(Modified {
            path: path_buf,
-
            eof: None,
        diff: DiffContent::Plain {
            hunks: vec![Hunk {
                header: Line::from(b"@@ -1 +1,2 @@\n".to_vec()),
@@ -88,7 +88,8 @@ fn test_diff_file() -> Result<(), Error> {
                    Modification::addition(b"This repository is a data source for the Upstream front-end tests and the\n".to_vec(), 1),
                    Modification::addition(b"[`radicle-surf`](https://github.com/radicle-dev/git-platinum) unit tests.\n".to_vec(), 2),
                ]
-
            }].into()
+
            }].into(),
+
            eof: EofNewLine::default(),
        },
    });
    assert_eq!(expected_diff, diff);
@@ -106,7 +107,6 @@ fn test_diff() -> Result<(), Error> {

    let expected_files = vec![FileDiff::Modified(Modified {
            path: Path::new("README.md").to_path_buf(),
-
            eof: None,
            diff: DiffContent::Plain {
                hunks: vec![Hunk {
                    header: Line::from(b"@@ -1 +1,2 @@\n".to_vec()),
@@ -115,7 +115,8 @@ fn test_diff() -> Result<(), Error> {
                        Modification::addition(b"This repository is a data source for the Upstream front-end tests and the\n".to_vec(), 1),
                        Modification::addition(b"[`radicle-surf`](https://github.com/radicle-dev/git-platinum) unit tests.\n".to_vec(), 2),
                    ]
-
                }].into()
+
                }].into(),
+
                eof: EofNewLine::default(),
            },
        })];
    let expected_stats = Stats {
@@ -188,7 +189,6 @@ fn test_diff_serde() -> Result<(), Error> {
    let rev_to = Branch::local(refname!("diff-test"));
    let diff = repo.diff(rev_from, rev_to)?;

-
    let eof: Option<u8> = None;
    let json = serde_json::json!({
        "added": [{
            "path": "LICENSE",
@@ -206,7 +206,8 @@ fn test_diff_serde() -> Result<(), Error> {
                        "lineNo": 2,
                        "type": "addition",
                    }]
-
                }]
+
                }],
+
                "eof": "noneMissing",
            },
        }],
        "deleted": [{
@@ -252,7 +253,8 @@ fn test_diff_serde() -> Result<(), Error> {
                        "type": "deletion",
                    },
                    ]
-
                }]
+
                }],
+
                "eof": "noneMissing",
            },
        }],
        "moved": [{
@@ -284,9 +286,9 @@ fn test_diff_serde() -> Result<(), Error> {
                          "type": "addition"
                        },
                    ]
-
                }]
+
                }],
+
                "eof": "noneMissing",
            },
-
            "eof": eof,
        }],
        "stats": {
            "deletions": 9,
@@ -315,7 +317,7 @@ index f89e4c0..7c56eb7 100644
    let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
    let diff = Diff::try_from(diff).unwrap();
    assert_eq!(
-
        diff.modified().next().unwrap().eof,
+
        diff.modified().next().unwrap().diff.eof(),
        Some(EofNewLine::BothMissing)
    );
}
@@ -333,41 +335,50 @@ index f89e4c0..7c56eb7 100644
"#;
    let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
    let diff = Diff::try_from(diff).unwrap();
-
    assert_eq!(diff.modified().next().unwrap().eof, None);
+
    assert_eq!(
+
        diff.modified().next().unwrap().diff.eof(),
+
        Some(EofNewLine::NoneMissing)
+
    );
}

// TODO(xphoniex): uncomment once libgit2 has fixed the bug
//#[test]
-
//     fn test_old_missing_eof_newline() {
-
//         let buf = r#"
-
// diff --git a/.env b/.env
-
// index f89e4c0..7c56eb7 100644
-
// --- a/.env
-
// +++ b/.env
-
// @@ -1 +1 @@
-
// -hello=123
-
// \ No newline at end of file
-
// +hello=1234
-
// "#;
-
//         let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
-
//         let diff = Diff::try_from(diff).unwrap();
-
//         assert_eq!(diff.modified[0].eof, Some(EofNewLine::OldMissing));
-
//     }
+
//fn test_old_missing_eof_newline() {
+
//    let buf = r#"
+
//diff --git a/.env b/.env
+
//index f89e4c0..7c56eb7 100644
+
//--- a/.env
+
//+++ b/.env
+
//@@ -1 +1 @@
+
//-hello=123
+
//\ No newline at end of file
+
//+hello=1234
+
//"#;
+
//    let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
+
//    let diff = Diff::try_from(diff).unwrap();
+
//    assert_eq!(
+
//        diff.modified().next().unwrap().diff.eof(),
+
//        Some(EofNewLine::OldMissing)
+
//    );
+
//}

// TODO(xphoniex): uncomment once libgit2 has fixed the bug
//#[test]
-
//     fn test_new_missing_eof_newline() {
-
//         let buf = r#"
-
// diff --git a/.env b/.env
-
// index f89e4c0..7c56eb7 100644
-
// --- a/.env
-
// +++ b/.env
-
// @@ -1 +1 @@
-
// -hello=123
-
// +hello=1234
-
// \ No newline at end of file
-
// "#;
-
//         let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
-
//         let diff = Diff::try_from(diff).unwrap();
-
//         assert_eq!(diff.modified[0].eof, Some(EofNewLine::NewMissing));
-
//     }
+
//fn test_new_missing_eof_newline() {
+
//    let buf = r#"
+
//diff --git a/.env b/.env
+
//index f89e4c0..7c56eb7 100644
+
//--- a/.env
+
//+++ b/.env
+
//@@ -1 +1 @@
+
//-hello=123
+
//+hello=1234
+
//\ No newline at end of file
+
//"#;
+
//    let diff = git2::Diff::from_buffer(buf.as_bytes()).unwrap();
+
//    let diff = Diff::try_from(diff).unwrap();
+
//    assert_eq!(
+
//        diff.modified().next().unwrap().diff.eof(),
+
//        Some(EofNewLine::NewMissing)
+
//    );
+
//}