Merge remote-tracking branch 'han/new-design-3'

* han/new-design-3:
  radicle-surf: refactor the design: part 3

Signed-off-by: Fintan Halpenny <[email protected]>

Author: FintanH

radicle-surf/docs/refactor-design.md

@@ -68,6 +68,13 @@ use `DirectoryContents` for its items and not to use `Tree` or `Forest` at all.
 We also found the `list_directory()` method duplicates with `iter()` method.
 Hence `list_directory()` is removed, together with `SystemType` type.
 
+## Remove `Vcs` trait
+
+The `Vcs` trait was introduced to support different version control backends,
+for example both Git and Pijul, and potentially others. However, since this
+port is part of `radicle-git` repo, we are only supporting Git going forward.
+We no longer need another layer of indirection defined by `Vcs` trait.
+
 ## The new API
 
 With the changes proposed in the previous section, we describe what the new API

radicle-surf/src/commit.rs

@@ -30,10 +30,7 @@ use crate::{
     file_system,
     person::Person,
     revision::Revision,
-    vcs::{
-        git::{self, BranchName, RepositoryRef, Rev},
-        Vcs,
-    },
+    vcs::git::{self, BranchName, RepositoryRef, Rev},
 };
 
 use radicle_git_ext::Oid;
@@ -244,7 +241,7 @@ where
     };
 
     let stats = repo.get_stats(&rev)?;
-    let headers = repo.get_history(rev)?.iter().map(Header::from).collect();
+    let headers = repo.history(rev)?.iter().map(Header::from).collect();
 
     Ok(Commits { headers, stats })
 }

radicle-surf/src/object/tree.rs

@@ -32,10 +32,7 @@ use crate::{
     git::RepositoryRef,
     object::{Error, Info, ObjectType},
     revision::Revision,
-    vcs::{
-        git::{Branch, Rev},
-        Vcs,
-    },
+    vcs::git::{Branch, Rev},
 };
 
 /// Result of a directory listing, carries other trees and blobs.
@@ -158,7 +155,7 @@ where
     entries.sort_by(|a, b| a.info.object_type.cmp(&b.info.object_type));
 
     let last_commit = if path.is_root() {
-        Some(commit::Header::from(repo.get_history(rev).unwrap().first()))
+        Some(commit::Header::from(repo.history(rev).unwrap().first()))
     } else {
         None
     };

radicle-surf/src/vcs.rs

@@ -15,10 +15,8 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-//! A model of a general VCS. The components consist of a [`History`]
-//! and a [`Vcs`] trait.
+//! A model of a general VCS History.
 
-// use crate::file_system::directory::Directory;
 use nonempty::NonEmpty;
 
 pub mod git;
@@ -134,34 +132,3 @@ impl<A> History<A> {
             .collect()
     }
 }
-
-pub(crate) trait GetVcs<Error>
-where
-    Self: Sized,
-{
-    /// The way to identify a Repository.
-    type RepoId;
-
-    /// Find a Repository
-    fn get_repo(identifier: Self::RepoId) -> Result<Self, Error>;
-}
-
-/// The `VCS` trait encapsulates the minimal amount of information for
-/// interacting with some notion of `History` from a given
-/// Version-Control-System.
-pub trait Vcs<A, Error> {
-    /// The way to identify a History.
-    type HistoryId;
-
-    /// The way to identify an artefact.
-    type ArtefactId;
-
-    /// Find a History in a Repo given a way to identify it
-    fn get_history(&self, identifier: Self::HistoryId) -> Result<History<A>, Error>;
-
-    /// Find all histories in a Repo
-    fn get_histories(&self) -> Result<Vec<History<A>>, Error>;
-
-    /// Identify artefacts of a Repository
-    fn get_identifier(artefact: &A) -> Self::ArtefactId;
-}

radicle-surf/src/vcs/git/repo.rs

@@ -20,21 +20,18 @@ use crate::{
     file_system,
     file_system::{directory, DirectoryContents, Label},
     vcs,
-    vcs::{
-        git::{
-            error::*,
-            reference::{glob::RefGlob, Ref, Rev},
-            Branch,
-            BranchName,
-            Commit,
-            Namespace,
-            RefScope,
-            Signature,
-            Stats,
-            Tag,
-            TagName,
-        },
-        Vcs,
+    vcs::git::{
+        error::*,
+        reference::{glob::RefGlob, Ref, Rev},
+        Branch,
+        BranchName,
+        Commit,
+        Namespace,
+        RefScope,
+        Signature,
+        Stats,
+        Tag,
+        TagName,
     },
 };
 use directory::Directory;
@@ -145,21 +142,15 @@ impl<'a> RepositoryRef<'a> {
         Ok(namespaces?.into_iter().collect())
     }
 
-    pub(super) fn reference<R, P>(&self, reference: R, check: P) -> Result<History, Error>
+    pub(super) fn ref_history<R>(&self, reference: R) -> Result<History, Error>
     where
         R: Into<Ref>,
-        P: FnOnce(&git2::Reference) -> Option<Error>,
     {
         let reference = match self.which_namespace()? {
             None => reference.into(),
             Some(namespace) => reference.into().namespaced(namespace),
         }
         .find_ref(self)?;
-
-        if let Some(err) = check(&reference) {
-            return Err(err);
-        }
-
         self.to_history(&reference)
     }
 
@@ -207,7 +198,7 @@ impl<'a> RepositoryRef<'a> {
     /// Gets stats of `rev`.
     pub fn get_stats(&self, rev: &Rev) -> Result<Stats, Error> {
         let branches = self.list_branches(RefScope::Local)?.len();
-        let history = self.get_history(rev.clone())?;
+        let history = self.history(rev.clone())?;
         let commits = history.len();
 
         let contributors = history
@@ -280,12 +271,6 @@ impl<'a> RepositoryRef<'a> {
         Ok(commit)
     }
 
-    /// Build a [`History`] using the `head` reference.
-    pub fn head_history(&self) -> Result<History, Error> {
-        let head = self.repo_ref.head()?;
-        self.to_history(&head)
-    }
-
     /// Turn a [`git2::Reference`] into a [`History`] by completing
     /// a revwalk over the first commit in the reference.
     pub(super) fn to_history(&self, history: &git2::Reference<'a>) -> Result<History, Error> {
@@ -439,10 +424,15 @@ impl<'a> RepositoryRef<'a> {
             opts.skip_binary_check(true);
         }
 
-        let diff =
+        let mut diff =
             self.repo_ref
                 .diff_tree_to_tree(old_tree.as_ref(), Some(&new_tree), Some(&mut opts))?;
 
+        // Detect renames by default.
+        let mut find_opts = git2::DiffFindOptions::new();
+        find_opts.renames(true);
+        diff.find_similar(Some(&mut find_opts))?;
+
         Ok(diff)
     }
 
@@ -539,40 +529,17 @@ impl<'a> RepositoryRef<'a> {
             rev: commit.id().to_string(),
         })
     }
-}
-
-impl<'a> Vcs<Commit, Error> for RepositoryRef<'a> {
-    type HistoryId = Rev;
-    type ArtefactId = Oid;
 
-    fn get_history(&self, history_id: Self::HistoryId) -> Result<History, Error> {
-        match history_id {
-            Rev::Ref(reference) => self.reference(reference, |_| None),
+    /// Returns the history of `rev`.
+    pub fn history(&self, rev: Rev) -> Result<History, Error> {
+        match rev {
+            Rev::Ref(reference) => self.ref_history(reference),
             Rev::Oid(oid) => {
                 let commit = self.get_git2_commit(oid)?;
                 self.commit_to_history(commit)
             },
         }
     }
-
-    fn get_histories(&self) -> Result<Vec<History>, Error> {
-        self.repo_ref
-            .references()
-            .map_err(Error::from)
-            .and_then(|mut references| {
-                references.try_fold(vec![], |mut acc, reference| {
-                    reference.map_err(Error::from).and_then(|r| {
-                        let history = self.to_history(&r)?;
-                        acc.push(history);
-                        Ok(acc)
-                    })
-                })
-            })
-    }
-
-    fn get_identifier(artifact: &Commit) -> Self::ArtefactId {
-        artifact.id
-    }
 }
 
 impl<'a> std::fmt::Debug for RepositoryRef<'a> {
@@ -608,16 +575,6 @@ impl<'a> From<&'a Repository> for RepositoryRef<'a> {
     }
 }
 
-impl vcs::GetVcs<Error> for Repository {
-    type RepoId = String;
-
-    fn get_repo(repo_id: Self::RepoId) -> Result<Self, Error> {
-        git2::Repository::open(&repo_id)
-            .map(Repository)
-            .map_err(Error::from)
-    }
-}
-
 impl From<git2::Repository> for Repository {
     fn from(repo: git2::Repository) -> Self {
         Repository(repo)