refactoring Revision and ToCommit traits

- Added a generic impl for &Revision
- Updated ToCommit for easier usage.

Signed-off-by: Han Xu <keepsimple@gmail.com>

radicle-surf/examples/browsing.rs

@@ -41,7 +41,7 @@ fn main() {
     let repo = Repository::discover(&repo_path).unwrap();
     let now = Instant::now();
     let head = repo.head().unwrap();
-    let root = repo.root_dir(&head).unwrap();
+    let root = repo.root_dir(head).unwrap();
     print_directory(&root, &repo, 0);
 
     let elapsed_millis = now.elapsed().as_millis();

radicle-surf/src/git.rs

@@ -127,14 +127,6 @@ impl Revision for RefString {
     }
 }
 
-impl Revision for &RefString {
-    type Error = git2::Error;
-
-    fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
-        repo.refname_to_id(self)
-    }
-}
-
 impl Revision for Qualified<'_> {
     type Error = git2::Error;
 
@@ -143,14 +135,6 @@ impl Revision for Qualified<'_> {
     }
 }
 
-impl Revision for &Qualified<'_> {
-    type Error = git2::Error;
-
-    fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
-        repo.refname_to_id(self)
-    }
-}
-
 impl Revision for Oid {
     type Error = Infallible;
 
@@ -170,14 +154,6 @@ impl Revision for &str {
 impl Revision for Branch {
     type Error = Error;
 
-    fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
-        (&self).object_id(repo)
-    }
-}
-
-impl Revision for &Branch {
-    type Error = Error;
-
     fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
         let refname = repo.namespaced_refname(&self.refname())?;
         Ok(repo.refname_to_id(&refname)?)
@@ -187,16 +163,24 @@ impl Revision for &Branch {
 impl Revision for Tag {
     type Error = Infallible;
 
-    fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
-        (&self).object_id(repo)
+    fn object_id(&self, _repo: &Repository) -> Result<Oid, Self::Error> {
+        Ok(self.id())
     }
 }
 
-impl Revision for &Tag {
-    type Error = Infallible;
+impl Revision for String {
+    type Error = git2::Error;
 
     fn object_id(&self, _repo: &Repository) -> Result<Oid, Self::Error> {
-        Ok(self.id())
+        Oid::from_str(self).map(Oid::from)
+    }
+}
+
+impl<R: Revision> Revision for &R {
+    type Error = R::Error;
+
+    fn object_id(&self, repo: &Repository) -> Result<Oid, Self::Error> {
+        (*self).object_id(repo)
     }
 }
 
@@ -213,22 +197,22 @@ pub trait ToCommit {
     type Error: std::error::Error + Send + Sync + 'static;
 
     /// Converts to a commit in `repo`.
-    fn to_commit(&self, repo: &Repository) -> Result<Commit, Self::Error>;
+    fn to_commit(self, repo: &Repository) -> Result<Commit, Self::Error>;
 }
 
 impl ToCommit for Commit {
     type Error = Infallible;
 
-    fn to_commit(&self, _repo: &Repository) -> Result<Commit, Self::Error> {
-        Ok(self.clone())
+    fn to_commit(self, _repo: &Repository) -> Result<Commit, Self::Error> {
+        Ok(self)
     }
 }
 
 impl<R: Revision> ToCommit for R {
     type Error = Error;
 
-    fn to_commit(&self, repo: &Repository) -> Result<Commit, Self::Error> {
-        let oid = repo.object_id(self)?;
+    fn to_commit(self, repo: &Repository) -> Result<Commit, Self::Error> {
+        let oid = repo.object_id(&self)?;
         let commit = repo.find_commit(oid)?;
         Ok(Commit::try_from(commit)?)
     }

radicle-surf/src/git/history.rs

@@ -38,7 +38,7 @@ enum FilterBy {
 
 impl<'a> History<'a> {
     /// Creates a new history starting from `head`, in `repo`.
-    pub fn new<C: ToCommit>(repo: &'a Repository, head: &C) -> Result<Self, Error> {
+    pub fn new<C: ToCommit>(repo: &'a Repository, head: C) -> Result<Self, Error> {
         let head = head
             .to_commit(repo)
             .map_err(|err| Error::ToCommit(err.into()))?;

radicle-surf/src/git/repo.rs

@@ -243,7 +243,7 @@ impl Repository {
     ///
     /// To visit inside any nested sub-directories, call `directory.get(&repo)`
     /// on the sub-directory.
-    pub fn root_dir<C: ToCommit>(&self, commit: &C) -> Result<Directory, Error> {
+    pub fn root_dir<C: ToCommit>(&self, commit: C) -> Result<Directory, Error> {
         let commit = commit
             .to_commit(self)
             .map_err(|err| Error::ToCommit(err.into()))?;
@@ -254,7 +254,7 @@ impl Repository {
 
     /// Returns the last commit, if exists, for a `path` in the history of
     /// `rev`.
-    pub fn last_commit<P, C>(&self, path: P, rev: &C) -> Result<Option<Commit>, Error>
+    pub fn last_commit<P, C>(&self, path: P, rev: C) -> Result<Option<Commit>, Error>
     where
         P: AsRef<Path>,
         C: ToCommit,
@@ -271,7 +271,7 @@ impl Repository {
     /// Gets the [`Stats`] of this repository.
     pub fn stats(&self) -> Result<Stats, Error> {
         let branches = self.branches(Glob::all_heads())?.count();
-        let mut history = self.history(&self.head()?)?;
+        let mut history = self.history(self.head()?)?;
         let (commits, contributors) = history.try_fold(
             (0, BTreeSet::new()),
             |(commits, mut contributors), commit| {
@@ -343,7 +343,7 @@ impl Repository {
     }
 
     /// Returns the history with the `head` commit.
-    pub fn history<C: ToCommit>(&self, head: &C) -> Result<History, Error> {
+    pub fn history<C: ToCommit>(&self, head: C) -> Result<History, Error> {
         History::new(self, head)
     }
 }

radicle-surf/t/src/file_system.rs

@@ -54,7 +54,7 @@ mod directory {
         let repo = Repository::open(GIT_PLATINUM).unwrap();
         // Get the snapshot of the directory for a given commit.
         let root = repo
-            .root_dir(&"80ded66281a4de2889cc07293a8f10947c6d57fe")
+            .root_dir("80ded66281a4de2889cc07293a8f10947c6d57fe")
             .unwrap();
 
         // Assert that we can find the memory.rs file!

radicle-surf/t/src/git/code_browsing.rs

@@ -66,3 +66,18 @@ fn test_file_history() {
         .unwrap();
     assert_eq!(file.size(), 67);
 }
+
+#[test]
+fn test_commit_history() {
+    let repo = Repository::open(GIT_PLATINUM).unwrap();
+    let head = "a0dd9122d33dff2a35f564d564db127152c88e02";
+
+    // verify `&str` works.
+    let h1 = repo.history(head).unwrap();
+
+    // verify `&String` works.
+    let head_string = head.to_string();
+    let h2 = repo.history(&head_string).unwrap();
+
+    assert_eq!(h1.head().id, h2.head().id);
+}

radicle-surf/t/src/git/last_commit.rs

@@ -18,7 +18,7 @@ fn readme_missing_and_memory() {
 
     // memory.rs is commited later so it should not exist here.
     let memory_last_commit_oid = repo
-        .last_commit(Path::new("src/memory.rs"), &oid)
+        .last_commit(Path::new("src/memory.rs"), oid)
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
 
@@ -26,7 +26,7 @@ fn readme_missing_and_memory() {
 
     // README.md exists in this commit.
     let readme_last_commit = repo
-        .last_commit(Path::new("README.md"), &oid)
+        .last_commit(Path::new("README.md"), oid)
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
 
@@ -44,7 +44,7 @@ fn folder_svelte() {
     let expected_commit_id = Oid::from_str("f3a089488f4cfd1a240a9c01b3fcc4c34a4e97b2").unwrap();
 
     let folder_svelte = repo
-        .last_commit(Path::new("examples/Folder.svelte"), &oid)
+        .last_commit(Path::new("examples/Folder.svelte"), oid)
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
 
@@ -64,7 +64,7 @@ fn nest_directory() {
     let nested_directory_tree_commit_id = repo
         .last_commit(
             Path::new("this/is/a/really/deeply/nested/directory/tree"),
-            &oid,
+            oid,
         )
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
@@ -85,13 +85,13 @@ fn can_get_last_commit_for_special_filenames() {
     let expected_commit_id = Oid::from_str("a0dd9122d33dff2a35f564d564db127152c88e02").unwrap();
 
     let backslash_commit_id = repo
-        .last_commit(Path::new(r"special/faux\\path"), &oid)
+        .last_commit(Path::new(r"special/faux\\path"), oid)
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
     assert_eq!(backslash_commit_id, Some(expected_commit_id));
 
     let ogre_commit_id = repo
-        .last_commit(Path::new("special/👹👹👹"), &oid)
+        .last_commit(Path::new("special/👹👹👹"), oid)
         .expect("Failed to get last commit")
         .map(|commit| commit.id);
     assert_eq!(ogre_commit_id, Some(expected_commit_id));

radicle-surf/t/src/git/rev.rs

@@ -44,7 +44,7 @@ fn _master() -> Result<(), Error> {
 fn commit() -> Result<(), Error> {
     let repo = Repository::open(GIT_PLATINUM)?;
     let rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
-    let mut history = repo.history(&rev)?;
+    let mut history = repo.history(rev)?;
 
     let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
     assert!(history.any(|commit| commit.unwrap().id == commit1));
@@ -56,7 +56,7 @@ fn commit() -> Result<(), Error> {
 fn commit_parents() -> Result<(), Error> {
     let repo = Repository::open(GIT_PLATINUM)?;
     let rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
-    let history = repo.history(&rev)?;
+    let history = repo.history(rev)?;
     let commit = history.head();
 
     assert_eq!(
@@ -71,7 +71,7 @@ fn commit_parents() -> Result<(), Error> {
 fn commit_short() -> Result<(), Error> {
     let repo = Repository::open(GIT_PLATINUM)?;
     let rev = repo.oid("3873745c8")?;
-    let mut history = repo.history(&rev)?;
+    let mut history = repo.history(rev)?;
 
     let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
     assert!(history.any(|commit| commit.unwrap().id == commit1));