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

* han/new-design-4:
  radicle-surf: refactor design: part 4

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>

radicle-surf/docs/refactor-design.md

@@ -9,10 +9,9 @@ can use it to create a GitHub-like UI for a git repo:
 
 1. Code browsing: given a specific commit/ref, browse files and directories.
 2. Diff between two revisions that resolve into two commits.
-3. Retrieve the history of the commits.
-4. Retrieve a specific object: all its metadata.
-5. Retrieve the refs: Branches, Tags, Remotes, Notes and user-defined
-"categories", where a category is: refs/<category>/<...>.
+3. Retrieve the history of commits with a given head, and optionally a file.
+4. List refs and retrieve their metadata: Branches, Tags, Remotes,
+Notes and user-defined "categories", where a category is: refs/<category>/<...>.
 
 ## Motivation
 
@@ -20,18 +19,17 @@ The `radicle-surf` crate aims to provide a safe and easy-to-use API that
 supports the features listed in [Introduction]. Based on the existing API,
 the main goals of the refactoring are:
 
-- API review: make API simpler whenever possible.
-- Address open issues in the original `radicle-surf` repo as much as possible.
-- Not to shy away from being `git` specific. (i.e. not to consider supporting
-other VCS systems)
-- Hide away `git2` from the API. The use of `git2` should be an implementation
-detail.
+- API review: identify the issues with the current API.
+- New API: propose a new API that could reuse parts of the existing API.
+- Address open issues in the original `radicle-surf` repo.
+- Be `git` specific. (i.e. no need to support other VCS systems)
+- Remove `git2` from the public API. The use of `git2` should be an
+implementation detail.
 
 ## API review
 
-The current API has quite a bit accidental complexity that is not inherent with
-the requirements, especially when we can be git specific and don't care about
-other VCS systems.
+In this section, we review some core types in the current API and propose
+changes to them. The main theme is to make the API simpler and easier to use.
 
 ### Remove the `Browser`
 
@@ -80,39 +78,121 @@ We no longer need another layer of indirection defined by `Vcs` trait.
 With the changes proposed in the previous section, we describe what the new API
 would look like and how they meet the requirements.
 
-### Common principles
+### Basic types
 
-#### How to identify things that resolve into a commit
+#### Revision and Commit
 
-In our API, it will be good to have a single way to identify all refs and
-objects that resolve into commits. In other words, we try to avoid using
-different ways at different places. Currently there are multiple types in the
-API for this purpose:
+In Git, `Revision` commonly resolves into a `Commit` but could refers to other
+objects for example a `Blob`. Hence we need to keep both concepts in the API.
+Currently we have multiple types to identify a `Commit` or `Revision`.
 
 - Commit
 - Oid
 - Rev
 
-Because `Rev` is the most high level among those and supports refs already,
-I think we should use `Rev` in our API as much as possible.
+The relations between them are: all `Rev` and `Commit` can resolve into `Oid`,
+and most `Rev`s can resolve into `Commit`.
 
-#### How to identify History
+On one hand, `Oid` is the ultimate unique identifer but it is more machine-
+friendly than human-friendly. On the other hand, `Revision` is most human-
+friendly and better suited in the API interface. A conversion from `Revision`
+to `Oid` will be useful.
 
-TBD
+For the places where `Commit` is required, we should explicitly ask for
+`Commit` instead of `Revision`.
+
+In conclusion, we define two new traits to support the use of `Revision` and
+`Commit`:
+
+```Rust
+pub trait Revision {
+    /// Resolves a revision into an object id in `repo`.
+    fn object_id(&self, repo: &RepositoryRef) -> Result<Oid, Error>;
+}
+
+pub trait ToCommit {
+    /// Converts to a commit in `repo`.
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error>;
+}
+```
+
+These two traits will be implemented for most common representations of
+`Revision` and `Commit`, for example `&str`, refs like `Branch`, `Tag`, etc.
+Our API will use these traits where we expect a `Revision` or a `Commit`.
+
+#### History
+
+The current `History` is generic over VCS types and also retrieves the full list
+of commits when the history is created. The VCS part can be removed and the
+history can lazy-load the list of commits by implmenting `Iterator` to support
+ potentially very long histories.
+
+We can also store the head commit with the history so that it's easy to get
+the start point and it helps to identify the history.
+
+To support getting the history of a file, we provide methods to modify a
+`History` to filter by a file path.
+
+The new `History` type would look like this:
+
+```Rust
+pub struct History<'a> {
+    repo: RepositoryRef<'a>,
+    head: Commit,
+    revwalk: git2::Revwalk<'a>,
+    filter_by: Option<FilterBy>,
+}
+
+enum FilterBy {
+    File { path: file_system::Path },
+}
+```
+
+For the methods provided by `History`, please see section [Retrieve the history]
+(#retrieve-the-history) below.
+
+#### Commit
+
+`Commit` is a central concept in Git. In `radicle-surf` we define `Commit` type
+to represent its metadata:
+
+```Rust
+pub struct Commit {
+    /// Object Id
+    pub id: Oid,
+    /// The author of the commit.
+    pub author: Author,
+    /// The actor who committed this commit.
+    pub committer: Author,
+    /// The long form message of the commit.
+    pub message: String,
+    /// The summary message of the commit.
+    pub summary: String,
+    /// The parents of this commit.
+    pub parents: Vec<Oid>,
+}
+```
+
+To get the content (i.e. the tree object) of the commit, the user should use
+`snapshot` method described in [Code browsing](#code-browsing) section.
+
+To get the diff of the commit, the user should use `diff_from_parent` method
+described in [Diffs](#diffs) section. Note that we might move that method to
+`Commit` type itself.
 
 ### Code browsing
 
 The user should be able to browse the files and directories for any given
-commits or references. The core API is:
+commit. The core API is:
 
 - Create a root Directory:
 ```Rust
-imp RepositoryRef {
-    pub fn snapshot(&self, rev: &Rev) -> Result<Directory, Error>;
+impl RepositoryRef {
+    pub fn snapshot<C: ToCommit>(&self, commit: C) -> Result<Directory, Error>;
 }
 ```
 
-- Browse a Directory:
+- Browse a Directory's contents:
 ```Rust
 impl Directory {
     pub fn contents(&self) -> impl Iterator<Item = &DirectoryContents>;
@@ -135,26 +215,142 @@ pub enum DirectoryContents {
 
 ### Diffs
 
-The user would be able to create a diff between any two revisions that resolve
-into two commits.
+The user would be able to create a diff between any two revisions. In the first
+implementation, these revisions have to resolve into commits. But in future,
+the revisions could refer to other objects, e.g. files (blobs).
 
-The main change is to use `Rev` instead of `Oid` to identify `from` and `to`.
 The core API is:
 
 ```Rust
-imp RepositoryRef {
-    pub fn diff(&self, from: &Rev, to: &Rev) -> Result<Diff, Error>;
+impl RepositoryRef {
+    /// Returns the diff between two revisions.
+    pub fn diff<R: Revision>(&self, from: R, to: R) -> Result<Diff, Error>;
 }
 ```
 
-To help convert from `Oid` to `Rev`, we provide a helper method:
+We used to have the following method:
 ```Rust
-imp RepositoryRef {
-    /// Returns the Oid of `rev`.
-    pub fn rev_oid(&self, rev: &Rev) -> Result<Oid, Error>;
+    /// Returns the diff between a revision and the initial state of the repo.
+    pub fn initial_diff<R: Revision>(&self, rev: R) -> Result<Diff, Error>;
+```
+
+However, it is not comparing with any other commit so the output is basically
+the snapshot of `R`. I am not sure if it is necessary. My take is that we can
+remove this method.
+
+We also have the following method:
+```Rust
+    /// Returns the diff of a specific commit.
+    pub fn diff_from_parent<C: ToCommit>(&self, commit: C) -> Result<Diff, Error>;
+```
+
+I think it is probably better to instead define the above method as
+`Commit::diff()` as its output is associated with a `Commit`.
+
+### Retrieve the history
+
+The user would be able to get the list of previous commits reachable from a
+particular commit.
+
+To create a `History` from a repo with a given head:
+```Rust
+impl RepositoryRef {
+    pub fn history<C: ToCommit>(&self, head: C) -> Result<History, Error>;
 }
 ```
 
+`History` implements `Iterator` that produces `Result<Commit, Error>`, and
+also provides these methods:
+
+```Rust
+impl<'a> History<'a> {
+    pub fn new<C: ToCommit>(repo: RepositoryRef<'a>, head: C) -> Result<Self, Error>;
+
+    pub fn head(&self) -> &Commit;
+
+    // Modifies a history with a filter by `path`.
+    // This is to support getting the history of a file.
+    pub fn by_path(mut self, path: file_system::Path) -> Self;
+```
+
+- Alternative design:
+
+One potential downside of define `History` as an iterator is that:
+`history.next()` takes a mutable history object. A different design is to use
+`History` as immutable object that produces an iterator on-demand:
+
+```Rust
+pub struct History<'a> {
+    repo: RepositoryRef<'a>,
+    head: Commit,
+}
+
+impl<'a> History<'a> {
+    /// This method creats a new `RevWalk` internally and return an
+    /// iterator for all commits in a history.
+    pub fn iter(&self) -> impl Iterator<Item = Commit>;
+}
+```
+
+In this design, `History` does not keep `RevWalk` in its state. It will create
+a new one when `iter()` is called. I like the immutable interface of this design
+but did not implement it in the current code mainly because the libgit2 doc says
+[creating a new `RevWalk` is relatively expensive](https://libgit2.org/libgit2/#HEAD/group/revwalk/git_revwalk_new).
+
+### List refs and retrieve their metadata
+
+Git refs are simple names that point to objects using object IDs. `radicle-surf`
+support refs by its `Ref` type.
+
+```Rust
+pub enum Ref {
+    /// A git tag, which can be found under `.git/refs/tags/`.
+    Tag {
+        /// The name of the tag, e.g. `v1.0.0`.
+        name: TagName,
+    },
+    /// A git branch, which can be found under `.git/refs/heads/`.
+    LocalBranch {
+        /// The name of the branch, e.g. `master`.
+        name: BranchName,
+    },
+    /// A git branch, which can be found under `.git/refs/remotes/`.
+    RemoteBranch {
+        /// The remote name, e.g. `origin`.
+        remote: String,
+        /// The name of the branch, e.g. `master`.
+        name: BranchName,
+    },
+    /// A git namespace, which can be found under `.git/refs/namespaces/`.
+    ///
+    /// Note that namespaces can be nested.
+    Namespace {
+        /// The name value of the namespace.
+        namespace: String,
+        /// The reference under that namespace, e.g. The
+        /// `refs/remotes/origin/master/ portion of `refs/namespaces/
+        /// moi/refs/remotes/origin/master`.
+        reference: Box<Ref>,
+    },
+    /// A git notes, which can be found under `.git/refs/notes`
+    Notes {
+        /// The default name is "commits".
+        name: String,
+    }
+}
+```
+
+### Git Objects
+
+Git has four kinds of objects: Blob, Tree, Commit and Tag. We have already
+discussed `Commit` (a struct) and `Tag` (`Ref::Tag`) types. For `blob`, we
+use `File` type to represent, and for `tree`, we use `Directory` type to
+represent. The motivation is to let the user "surf" a repo as a file system
+as much as possible, and to avoid Git internal concepts in our API if possible.
+
+Open question: there could be some cases where the names `blob` and `tree`
+shall be used. We need to define such cases clearly if they exist.
+
 ## Error handling
 
 TBD

radicle-surf/examples/diff.rs

@@ -32,10 +32,7 @@ fn main() {
     let base_oid = Oid::from_str(&options.base_revision).unwrap();
     let now = Instant::now();
     let elapsed_nanos = now.elapsed().as_nanos();
-    let diff = repo
-        .as_ref()
-        .diff(&base_oid.into(), &head_oid.into())
-        .unwrap();
+    let diff = repo.as_ref().diff(base_oid, head_oid).unwrap();
     print_diff_summary(&diff, elapsed_nanos);
 }
 

radicle-surf/src/commit.rs

@@ -109,6 +109,12 @@ impl From<&git::Commit> for Header {
     }
 }
 
+impl From<git::Commit> for Header {
+    fn from(commit: git::Commit) -> Self {
+        Self::from(&commit)
+    }
+}
+
 #[cfg(feature = "serialize")]
 impl Serialize for Header {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -142,14 +148,10 @@ pub struct Commits {
 /// Will return [`Error`] if the project doesn't exist or the surf interaction
 /// fails.
 pub fn commit(repo: &RepositoryRef, rev: &Rev) -> Result<Commit, Error> {
-    let sha1 = repo.rev_oid(rev)?;
-    let commit = repo.get_commit(sha1)?;
-    let diff = if let Some(parent) = commit.parents.first() {
-        let parent_rev = (*parent).into();
-        repo.diff(&parent_rev, rev)?
-    } else {
-        repo.initial_diff(rev)?
-    };
+    let commit = repo.commit(rev)?;
+    let sha1 = commit.id;
+    let header = Header::from(&commit);
+    let diff = repo.diff_from_parent(commit)?;
 
     let mut deletions = 0;
     let mut additions = 0;
@@ -199,7 +201,7 @@ pub fn commit(repo: &RepositoryRef, rev: &Rev) -> Result<Commit, Error> {
         .collect();
 
     Ok(Commit {
-        header: Header::from(&commit),
+        header,
         stats: Stats {
             additions,
             deletions,
@@ -216,7 +218,7 @@ pub fn commit(repo: &RepositoryRef, rev: &Rev) -> Result<Commit, Error> {
 /// Will return [`Error`] if the project doesn't exist or the surf interaction
 /// fails.
 pub fn header(repo: &RepositoryRef, sha1: Oid) -> Result<Header, Error> {
-    let commit = repo.get_commit(sha1)?;
+    let commit = repo.commit(sha1)?;
     Ok(Header::from(&commit))
 }
 
@@ -240,8 +242,9 @@ where
         None => repo.head_oid()?.into(),
     };
 
-    let stats = repo.get_stats(&rev)?;
-    let headers = repo.history(rev)?.iter().map(Header::from).collect();
+    let stats = repo.get_commit_stats(&rev)?;
+    let commits: Result<Vec<git::Commit>, git::Error> = repo.history(&rev)?.collect();
+    let headers = commits?.iter().map(Header::from).collect();
 
     Ok(Commits { headers, stats })
 }

radicle-surf/src/file_system/path.rs

@@ -169,6 +169,14 @@ impl From<Path> for Vec<Label> {
     }
 }
 
+impl From<&Path> for path::PathBuf {
+    fn from(path: &Path) -> Self {
+        path.iter()
+            .map(|label| label.as_str())
+            .collect::<path::PathBuf>()
+    }
+}
+
 impl git2::IntoCString for Path {
     fn into_c_string(self) -> Result<CString, git2::Error> {
         if self.is_root() {

radicle-surf/src/object/blob.rs

@@ -141,7 +141,6 @@ where
 {
     let maybe_revision = maybe_revision.map(Rev::try_from).transpose()?;
     let revision = maybe_revision.unwrap();
-
     let root = repo.snapshot(&revision)?;
     let p = file_system::Path::from_str(path)?;
 

radicle-surf/src/object/tree.rs

@@ -155,7 +155,8 @@ 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.history(rev).unwrap().first()))
+        let history = repo.history(&rev)?;
+        Some(commit::Header::from(history.head()))
     } else {
         None
     };

radicle-surf/src/vcs.rs

@@ -15,120 +15,6 @@
 // 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 History.
-
-use nonempty::NonEmpty;
+//! A model of a general VCS.
 
 pub mod git;
-
-/// A non-empty bag of artifacts which are used to
-/// derive a [`crate::file_system::Directory`] view. Examples of artifacts
-/// would be commits in Git or patches in Pijul.
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
-pub struct History<A>(pub NonEmpty<A>);
-
-impl<A> History<A> {
-    /// Create a new `History` consisting of one artifact.
-    pub fn new(a: A) -> Self {
-        History(NonEmpty::new(a))
-    }
-
-    /// Push an artifact to the end of the `History`.
-    pub fn push(&mut self, a: A) {
-        self.0.push(a)
-    }
-
-    /// Iterator over the artifacts.
-    pub fn iter(&self) -> impl Iterator<Item = &A> {
-        self.0.iter()
-    }
-
-    /// Get the firest artifact in the `History`.
-    pub fn first(&self) -> &A {
-        self.0.first()
-    }
-
-    /// Get the length of `History` (aka the artefacts count)
-    pub fn len(&self) -> usize {
-        self.0.len()
-    }
-
-    /// Check if `History` is empty
-    pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
-    }
-
-    /// Given that the `History` is topological order from most
-    /// recent artifact to least recent, `find_suffix` gets returns
-    /// the history up until the point of the given artifact.
-    ///
-    /// This operation may fail if the artifact does not exist in
-    /// the given `History`.
-    pub fn find_suffix(&self, artifact: &A) -> Option<Self>
-    where
-        A: Clone + PartialEq,
-    {
-        let new_history: Option<NonEmpty<A>> = NonEmpty::from_slice(
-            &self
-                .iter()
-                .cloned()
-                .skip_while(|current| *current != *artifact)
-                .collect::<Vec<_>>(),
-        );
-
-        new_history.map(History)
-    }
-
-    /// Apply a function from `A` to `B` over the `History`
-    pub fn map<F, B>(self, f: F) -> History<B>
-    where
-        F: FnMut(A) -> B,
-    {
-        History(self.0.map(f))
-    }
-
-    /// Find an artifact in the `History`.
-    ///
-    /// The function provided should return `Some` if the item is the desired
-    /// output and `None` otherwise.
-    pub fn find<F, B>(&self, f: F) -> Option<B>
-    where
-        F: Fn(&A) -> Option<B>,
-    {
-        self.iter().find_map(f)
-    }
-
-    /// Find an atrifact in the given `History` using the artifacts ID.
-    ///
-    /// This operation may fail if the artifact does not exist in the history.
-    pub fn find_in_history<Identifier, F>(&self, identifier: &Identifier, id_of: F) -> Option<A>
-    where
-        A: Clone,
-        F: Fn(&A) -> Identifier,
-        Identifier: PartialEq,
-    {
-        self.iter()
-            .find(|artifact| {
-                let current_id = id_of(artifact);
-                *identifier == current_id
-            })
-            .cloned()
-    }
-
-    /// Find all occurences of an artifact in a bag of `History`s.
-    pub fn find_in_histories<Identifier, F>(
-        histories: Vec<Self>,
-        identifier: &Identifier,
-        id_of: F,
-    ) -> Vec<Self>
-    where
-        A: Clone,
-        F: Fn(&A) -> Identifier + Copy,
-        Identifier: PartialEq,
-    {
-        histories
-            .into_iter()
-            .filter(|history| history.find_in_history(identifier, id_of).is_some())
-            .collect()
-    }
-}

radicle-surf/src/vcs/git.rs

@@ -63,6 +63,8 @@
 //! # }
 //! ```
 
+use std::str::FromStr;
+
 // Re-export git2 as sub-module
 pub use git2::{self, Error as Git2Error, Time};
 pub use radicle_git_ext::Oid;
@@ -72,9 +74,13 @@ mod reference;
 pub use reference::{ParseError, Ref, Rev};
 
 mod repo;
-pub use repo::{History, Repository, RepositoryRef};
+pub use repo::{Repository, RepositoryRef};
+
+mod history;
+pub use history::History;
 
 pub mod error;
+pub use error::Error;
 
 pub mod ext;
 
@@ -138,3 +144,54 @@ where
         })
     }
 }
+
+/// Supports various ways to specify a revision used in Git.
+pub trait Revision {
+    /// Returns the object id of this revision in `repo`.
+    fn object_id(&self, repo: &RepositoryRef) -> Result<Oid, Error>;
+}
+
+impl Revision for Oid {
+    fn object_id(&self, _repo: &RepositoryRef) -> Result<Oid, Error> {
+        Ok(*self)
+    }
+}
+
+impl Revision for &str {
+    fn object_id(&self, _repo: &RepositoryRef) -> Result<Oid, Error> {
+        Oid::from_str(*self).map_err(Error::Git)
+    }
+}
+
+impl Revision for &Branch {
+    fn object_id(&self, repo: &RepositoryRef) -> Result<Oid, Error> {
+        let refname = repo.namespaced_refname(&self.refname())?;
+        Ok(repo.repo_ref.refname_to_id(&refname).map(Oid::from)?)
+    }
+}
+
+impl Revision for &Tag {
+    fn object_id(&self, repo: &RepositoryRef) -> Result<Oid, Error> {
+        let refname = repo.namespaced_refname(&self.refname())?;
+        Ok(repo.repo_ref.refname_to_id(&refname).map(Oid::from)?)
+    }
+}
+
+impl Revision for &Rev {
+    fn object_id(&self, repo: &RepositoryRef) -> Result<Oid, Error> {
+        match *self {
+            Rev::Oid(oid) => Ok(*oid),
+            Rev::Ref(r) => {
+                let r = match repo.which_namespace()? {
+                    None => r.clone(),
+                    Some(namespace) => match r {
+                        Ref::Namespace { .. } => r.clone(),
+                        _ => r.clone().namespaced(namespace),
+                    },
+                };
+                let refname = format!("{}", r);
+                Ok(repo.repo_ref.refname_to_id(&refname).map(Oid::from)?)
+            },
+        }
+    }
+}

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

@@ -16,10 +16,9 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 use crate::vcs::git::{self, error::Error, ext, reference::Ref};
-use std::{cmp::Ordering, convert::TryFrom, fmt, str};
-
 #[cfg(feature = "serialize")]
 use serde::{Deserialize, Serialize};
+use std::{cmp::Ordering, convert::TryFrom, fmt, str};
 
 /// The branch type we want to filter on.
 #[cfg_attr(feature = "serialize", derive(Serialize, Deserialize))]
@@ -147,13 +146,13 @@ impl Branch {
     }
 
     /// Get the name of the `Branch`.
-    pub fn name(&self) -> String {
+    pub fn refname(&self) -> String {
         let branch_name = self.name.0.clone();
         match self.locality {
-            BranchType::Local => branch_name,
+            BranchType::Local => format!("refs/heads/{}", branch_name),
             BranchType::Remote { ref name } => match name {
                 None => branch_name,
-                Some(remote_name) => format!("{}/{}", remote_name, branch_name),
+                Some(remote_name) => format!("refs/remotes/{}/{}", remote_name, branch_name),
             },
         }
     }

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

@@ -15,7 +15,10 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-use crate::vcs::git::error::Error;
+use crate::{
+    file_system::{self, directory},
+    vcs::git::{error::Error, Branch, RepositoryRef, Rev, Tag},
+};
 use radicle_git_ext::Oid;
 use std::{convert::TryFrom, str};
 
@@ -116,6 +119,16 @@ impl Commit {
             .unwrap_or(&self.message)
             .trim()
     }
+
+    /// Retrieves the file with `path` in this commit.
+    pub fn get_file(
+        &self,
+        repo: &RepositoryRef,
+        path: file_system::Path,
+    ) -> Result<directory::File, Error> {
+        let git2_commit = repo.get_git2_commit(self.id)?;
+        repo.get_commit_file(&git2_commit, path)
+    }
 }
 
 #[cfg(feature = "serialize")]
@@ -166,3 +179,45 @@ impl<'repo> TryFrom<git2::Commit<'repo>> for Commit {
         })
     }
 }
+
+/// A common trait for anything that can convert to a `Commit`.
+pub trait ToCommit {
+    /// Converts to a commit in `repo`.
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error>;
+}
+
+impl ToCommit for Commit {
+    fn to_commit(self, _repo: &RepositoryRef) -> Result<Commit, Error> {
+        Ok(self)
+    }
+}
+
+impl ToCommit for &str {
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error> {
+        repo.commit(self)
+    }
+}
+
+impl ToCommit for Oid {
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error> {
+        repo.commit(self)
+    }
+}
+
+impl ToCommit for &Branch {
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error> {
+        repo.commit(self)
+    }
+}
+
+impl ToCommit for &Tag {
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error> {
+        repo.commit(self)
+    }
+}
+
+impl ToCommit for &Rev {
+    fn to_commit(self, repo: &RepositoryRef) -> Result<Commit, Error> {
+        repo.commit(self)
+    }
+}

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

@@ -0,0 +1,103 @@
+// This file is part of radicle-surf
+// <https://github.com/radicle-dev/radicle-git>
+//
+// Copyright (C) 2022 The Radicle Team <dev@radicle.xyz>
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 or
+// later as published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+use crate::{
+    file_system,
+    vcs::git::{commit::ToCommit, Commit, Error, RepositoryRef},
+};
+use std::convert::TryFrom;
+
+/// An iterator that produces the history of commits for a given `head`,
+/// in the `repo`.
+pub struct History<'a> {
+    repo: RepositoryRef<'a>,
+    head: Commit,
+    revwalk: git2::Revwalk<'a>,
+    filter_by: Option<FilterBy>,
+}
+
+/// Internal implementation, subject to refactoring.
+enum FilterBy {
+    File { path: file_system::Path },
+}
+
+impl<'a> History<'a> {
+    /// Creates a new history starting from `head`, in `repo`.
+    pub fn new<C: ToCommit>(repo: RepositoryRef<'a>, head: C) -> Result<Self, Error> {
+        let head = head.to_commit(&repo)?;
+        let mut revwalk = repo.repo_ref.revwalk()?;
+        revwalk.push(head.id.into())?;
+        let history = Self {
+            repo,
+            head,
+            revwalk,
+            filter_by: None,
+        };
+        Ok(history)
+    }
+
+    /// Returns the first commit (i.e. the head) in the history.
+    pub fn head(&self) -> &Commit {
+        &self.head
+    }
+
+    /// Returns a modified `History` filtered by `path`.
+    ///
+    /// Note that it is possible that a filtered History becomes empty,
+    /// even though calling `.head()` still returns the original head.
+    pub fn by_path(mut self, path: file_system::Path) -> Self {
+        self.filter_by = Some(FilterBy::File { path });
+        self
+    }
+}
+
+impl<'a> Iterator for History<'a> {
+    type Item = Result<Commit, Error>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        // Loop through the commits with the optional filtering.
+        while let Some(oid) = self.revwalk.next() {
+            let found = oid
+                .map_err(Error::Git)
+                .and_then(|oid| {
+                    let git2_commit = self.repo.repo_ref.find_commit(oid)?;
+
+                    // Handles the optional filter_by.
+                    if let Some(FilterBy::File { path }) = &self.filter_by {
+                        let path_opt = self.repo.diff_commit_and_parents(path, &git2_commit)?;
+                        if path_opt.is_none() {
+                            return Ok(None); // Filter out this commit.
+                        }
+                    }
+
+                    let commit = Commit::try_from(git2_commit)?;
+                    Ok(Some(commit))
+                })
+                .transpose();
+            if found.is_some() {
+                return found;
+            }
+        }
+        None
+    }
+}
+
+impl<'a> std::fmt::Debug for History<'a> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "History of {}", self.head.id)
+    }
+}

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

@@ -30,6 +30,18 @@ pub struct Namespace {
     pub(super) values: NonEmpty<String>,
 }
 
+impl Namespace {
+    /// Appends a `refname` to this namespace, and returns
+    /// the full reference path.
+    pub fn append_refname(&self, refname: &str) -> String {
+        let mut prefix = String::new();
+        for value in self.values.iter() {
+            prefix = format!("{}refs/namespaces/{}/", &prefix, value);
+        }
+        format!("{}{}", &prefix, refname)
+    }
+}
+
 impl fmt::Display for Namespace {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let values: Vec<_> = self.values.clone().into();

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

@@ -19,7 +19,7 @@ use std::{fmt, str};
 
 use thiserror::Error;
 
-use crate::vcs::git::{repo::RepositoryRef, BranchName, Namespace, TagName};
+use crate::vcs::git::{BranchName, Namespace, TagName};
 use radicle_git_ext::Oid;
 pub(super) mod glob;
 
@@ -93,15 +93,6 @@ impl Ref {
 
         ref_namespace
     }
-
-    /// We try to find a [`git2::Reference`] based off of a `Ref` by turning the
-    /// ref into a fully qualified ref (e.g. refs/remotes/**/master).
-    pub fn find_ref<'a>(
-        &self,
-        repo: &RepositoryRef<'a>,
-    ) -> Result<git2::Reference<'a>, git2::Error> {
-        repo.repo_ref.find_reference(&self.to_string())
-    }
 }
 
 impl fmt::Display for Ref {

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

@@ -19,15 +19,16 @@ use crate::{
     diff::*,
     file_system,
     file_system::{directory, DirectoryContents, Label},
-    vcs,
     vcs::git::{
         error::*,
-        reference::{glob::RefGlob, Ref, Rev},
+        reference::{glob::RefGlob, Rev},
         Branch,
         BranchName,
         Commit,
+        History,
         Namespace,
         RefScope,
+        Revision,
         Signature,
         Stats,
         Tag,
@@ -35,23 +36,15 @@ use crate::{
     },
 };
 use directory::Directory;
-use nonempty::NonEmpty;
 use radicle_git_ext::Oid;
 use std::{
     collections::{BTreeSet, HashSet},
     convert::TryFrom,
+    path::PathBuf,
     str,
 };
 
-/// This is for flagging to the `file_history` function that it should
-/// stop at the first (i.e. Last) commit it finds for a file.
-pub(super) enum CommitHistory {
-    _Full,
-    Last,
-}
-
-/// A `History` that uses `git2::Commit` as the underlying artifact.
-pub type History = vcs::History<Commit>;
+use super::commit::ToCommit;
 
 /// Wrapper around the `git2`'s `git2::Repository` type.
 /// This is to to limit the functionality that we can do
@@ -66,6 +59,7 @@ pub struct Repository(pub(super) git2::Repository);
 ///
 /// Use the `From<&'a git2::Repository>` implementation to construct a
 /// `RepositoryRef`.
+#[derive(Clone, Copy)]
 pub struct RepositoryRef<'a> {
     pub(super) repo_ref: &'a git2::Repository,
 }
@@ -142,69 +136,72 @@ impl<'a> RepositoryRef<'a> {
         Ok(namespaces?.into_iter().collect())
     }
 
-    pub(super) fn ref_history<R>(&self, reference: R) -> Result<History, Error>
-    where
-        R: Into<Ref>,
-    {
-        let reference = match self.which_namespace()? {
-            None => reference.into(),
-            Some(namespace) => reference.into().namespaced(namespace),
-        }
-        .find_ref(self)?;
-        self.to_history(&reference)
-    }
-
     /// Get the [`Diff`] between two commits.
-    pub fn diff(&self, from: &Rev, to: &Rev) -> Result<Diff, Error> {
-        let from_commit = self.rev_to_commit(from)?;
-        let to_commit = self.rev_to_commit(to)?;
+    pub fn diff(&self, from: impl Revision, to: impl Revision) -> Result<Diff, Error> {
+        let from_commit = self.get_git2_commit(from.object_id(self)?)?;
+        let to_commit = self.get_git2_commit(to.object_id(self)?)?;
         self.diff_commits(None, Some(&from_commit), &to_commit)
             .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, rev: &Rev) -> Result<Diff, Error> {
-        let commit = self.rev_to_commit(rev)?;
+    pub fn initial_diff<R: Revision>(&self, rev: R) -> Result<Diff, Error> {
+        let commit = self.get_git2_commit(rev.object_id(self)?)?;
         self.diff_commits(None, None, &commit)
             .and_then(|diff| Diff::try_from(diff).map_err(Error::from))
     }
 
+    /// Get the diff introduced by a particlar rev.
+    pub fn diff_from_parent<C: ToCommit>(&self, commit: C) -> Result<Diff, Error> {
+        let commit = commit.to_commit(self)?;
+        match commit.parents.first() {
+            Some(parent) => self.diff(*parent, commit.id),
+            None => self.initial_diff(commit.id),
+        }
+    }
+
     /// Parse an [`Oid`] from the given string.
     pub fn oid(&self, oid: &str) -> Result<Oid, Error> {
         Ok(self.repo_ref.revparse_single(oid)?.id().into())
     }
 
     /// Gets a snapshot of the repo as a Directory.
-    pub fn snapshot(&self, rev: &Rev) -> Result<Directory, Error> {
-        let commit = self.rev_to_commit(rev)?;
-        self.directory_of_commit(&commit)
+    pub fn snapshot<C: ToCommit>(&self, commit: C) -> Result<Directory, Error> {
+        let commit = commit.to_commit(self)?;
+        let git2_commit = self.repo_ref.find_commit((commit.id).into())?;
+        self.directory_of_commit(&git2_commit)
     }
 
     /// Returns the last commit, if exists, for a `path` in the history of
     /// `rev`.
     pub fn last_commit(&self, path: file_system::Path, rev: &Rev) -> Result<Option<Commit>, Error> {
-        let git2_commit = self.rev_to_commit(rev)?;
-        let commit = Commit::try_from(git2_commit)?;
-        let file_history = self.file_history(&path, CommitHistory::Last, commit)?;
-        Ok(file_history.first().cloned())
+        let history = self.history(rev)?;
+        history.by_path(path).next().transpose()
     }
 
-    /// Retrieves `Commit` identified by `oid`.
-    pub fn get_commit(&self, oid: Oid) -> Result<Commit, Error> {
-        let git2_commit = self.get_git2_commit(oid)?;
-        Commit::try_from(git2_commit)
+    /// Returns a commit for `rev` if exists.
+    pub fn commit<R: Revision>(&self, rev: R) -> Result<Commit, Error> {
+        let oid = rev.object_id(self)?;
+        match self.repo_ref.find_commit(oid.into()) {
+            Ok(commit) => Commit::try_from(commit),
+            Err(e) => Err(Error::Git(e)),
+        }
     }
 
-    /// Gets stats of `rev`.
-    pub fn get_stats(&self, rev: &Rev) -> Result<Stats, Error> {
+    /// Gets stats of `commit`.
+    pub fn get_commit_stats<C: ToCommit>(&self, commit: C) -> Result<Stats, Error> {
         let branches = self.list_branches(RefScope::Local)?.len();
-        let history = self.history(rev.clone())?;
-        let commits = history.len();
+        let history = self.history(commit)?;
+        let mut commits = 0;
 
         let contributors = history
-            .iter()
-            .cloned()
-            .map(|commit| (commit.author.name, commit.author.email))
+            .filter_map(|commit| match commit {
+                Ok(commit) => {
+                    commits += 1;
+                    Some((commit.author.name, commit.author.email))
+                },
+                Err(_) => None,
+            })
             .collect::<BTreeSet<_>>();
 
         Ok(Stats {
@@ -247,62 +244,23 @@ impl<'a> RepositoryRef<'a> {
         Ok(head_commit.id().into())
     }
 
-    /// Returns the Oid of `rev`.
-    pub fn rev_oid(&self, rev: &Rev) -> Result<Oid, Error> {
-        let commit = self.rev_to_commit(rev)?;
-        Ok(commit.id().into())
-    }
-
-    pub(super) fn rev_to_commit(&self, rev: &Rev) -> Result<git2::Commit, Error> {
-        match rev {
-            Rev::Oid(oid) => Ok(self.repo_ref.find_commit((*oid).into())?),
-            Rev::Ref(reference) => Ok(reference.find_ref(self)?.peel_to_commit()?),
-        }
-    }
-
     /// Switch to a `namespace`
     pub fn switch_namespace(&self, namespace: &str) -> Result<(), Error> {
         Ok(self.repo_ref.set_namespace(namespace)?)
     }
 
+    /// Returns a full reference name with namespace(s) included.
+    pub(crate) fn namespaced_refname(&self, refname: &str) -> Result<String, Error> {
+        let fullname = match self.which_namespace()? {
+            Some(namespace) => namespace.append_refname(refname),
+            None => refname.to_string(),
+        };
+        Ok(fullname)
+    }
+
     /// Get a particular `git2::Commit` of `oid`.
-    pub fn get_git2_commit(&self, oid: Oid) -> Result<git2::Commit<'a>, Error> {
-        let commit = self.repo_ref.find_commit(oid.into())?;
-        Ok(commit)
-    }
-
-    /// 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> {
-        let head = history.peel_to_commit()?;
-        self.commit_to_history(head)
-    }
-
-    /// Turn a [`git2::Reference`] into a [`History`] by completing
-    /// a revwalk over the first commit in the reference.
-    pub(super) fn commit_to_history(&self, head: git2::Commit) -> Result<History, Error> {
-        let head_id = head.id();
-        let mut commits = NonEmpty::new(Commit::try_from(head)?);
-        let mut revwalk = self.repo_ref.revwalk()?;
-
-        // Set the revwalk to the head commit
-        revwalk.push(head_id)?;
-
-        for commit_result_id in revwalk {
-            // The revwalk iter returns results so
-            // we unpack these and push them to the history
-            let commit_id = commit_result_id?;
-
-            // Skip the head commit since we have processed it
-            if commit_id == head_id {
-                continue;
-            }
-
-            let commit = Commit::try_from(self.repo_ref.find_commit(commit_id)?)?;
-            commits.push(commit);
-        }
-
-        Ok(vcs::History(commits))
+    pub(crate) fn get_git2_commit(&self, oid: Oid) -> Result<git2::Commit, Error> {
+        self.repo_ref.find_commit(oid.into()).map_err(Error::Git)
     }
 
     /// Extract the signature from a commit
@@ -363,37 +321,19 @@ impl<'a> RepositoryRef<'a> {
         Ok(other == git2_oid || is_descendant)
     }
 
-    /// Get the history of the file system where the head of the [`NonEmpty`] is
-    /// the latest commit.
-    pub(super) fn file_history(
+    pub(crate) fn get_commit_file(
         &self,
-        path: &file_system::Path,
-        commit_history: CommitHistory,
-        commit: Commit,
-    ) -> Result<Vec<Commit>, Error> {
-        let mut revwalk = self.repo_ref.revwalk()?;
-        let mut commits = vec![];
-
-        // Set the revwalk to the head commit
-        revwalk.push(commit.id.into())?;
-
-        for commit in revwalk {
-            let parent_id = commit?;
-            let parent = self.repo_ref.find_commit(parent_id)?;
-            let paths = self.diff_commit_and_parents(path, &parent)?;
-            if let Some(_path) = paths {
-                commits.push(Commit::try_from(parent)?);
-                match &commit_history {
-                    CommitHistory::Last => break,
-                    CommitHistory::_Full => {},
-                }
-            }
-        }
-
-        Ok(commits)
+        git2_commit: &git2::Commit,
+        path: file_system::Path,
+    ) -> Result<directory::File, Error> {
+        let git2_tree = git2_commit.tree()?;
+        let entry = git2_tree.get_path(PathBuf::from(&path).as_ref())?;
+        let object = entry.to_object(self.repo_ref)?;
+        let blob = object.as_blob().ok_or(Error::PathNotFound(path))?;
+        Ok(directory::File::new(blob.content()))
     }
 
-    fn diff_commit_and_parents(
+    pub(crate) fn diff_commit_and_parents(
         &self,
         path: &file_system::Path,
         commit: &git2::Commit,
@@ -530,15 +470,9 @@ impl<'a> RepositoryRef<'a> {
         })
     }
 
-    /// 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)
-            },
-        }
+    /// Returns the history with the `head` commit.
+    pub fn history<C: ToCommit>(&self, head: C) -> Result<History, Error> {
+        History::new(*self, head)
     }
 }
 
@@ -562,7 +496,7 @@ impl Repository {
 
     /// Since our operations are read-only when it comes to surfing a repository
     /// we have a separate struct called [`RepositoryRef`]. This turns an owned
-    /// [`Repository`], the one returend by [`Repository::new`], into a
+    /// [`Repository`], the one returned by [`Repository::new`], into a
     /// [`RepositoryRef`].
     pub fn as_ref(&'_ self) -> RepositoryRef<'_> {
         RepositoryRef { repo_ref: &self.0 }

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

@@ -106,6 +106,11 @@ impl Tag {
             Self::Annotated { name, .. } => name.clone(),
         }
     }
+
+    /// Returns the full ref name of the tag.
+    pub fn refname(&self) -> String {
+        format!("refs/tags/{}", self.name().name())
+    }
 }
 
 impl<'repo> TryFrom<git2::Tag<'repo>> for Tag {

radicle-surf/t/src/git.rs

@@ -18,9 +18,7 @@ const GIT_PLATINUM: &str = "../data/git-platinum";
 // An issue with submodules, see: https://github.com/radicle-dev/radicle-surf/issues/54
 fn test_submodule_failure() {
     let repo = Repository::new("../..").unwrap();
-    repo.as_ref()
-        .snapshot(&Branch::local("main").into())
-        .unwrap();
+    repo.as_ref().snapshot(&Branch::local("main")).unwrap();
 }
 
 #[cfg(test)]
@@ -32,11 +30,11 @@ mod namespace {
     fn switch_to_banana() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let history_master = repo.history(Branch::local("master").into())?;
+        let history_master = repo.history(&Branch::local("master"))?;
         repo.switch_namespace("golden")?;
-        let history_banana = repo.history(Branch::local("banana").into())?;
+        let history_banana = repo.history(&Branch::local("banana"))?;
 
-        assert_ne!(history_master, history_banana);
+        assert_ne!(history_master.head(), history_banana.head());
 
         Ok(())
     }
@@ -45,15 +43,15 @@ mod namespace {
     fn me_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let history = repo.history(Branch::local("master").into())?;
+        let history = repo.history(&Branch::local("master"))?;
 
         assert_eq!(repo.which_namespace(), Ok(None));
 
         repo.switch_namespace("me")?;
         assert_eq!(repo.which_namespace(), Ok(Some(Namespace::try_from("me")?)));
 
-        let history_feature = repo.history(Branch::local("feature/#1194").into())?;
-        assert_eq!(history, history_feature);
+        let history_feature = repo.history(&Branch::local("feature/#1194"))?;
+        assert_eq!(history.head(), history_feature.head());
 
         let expected_branches: Vec<Branch> = vec![Branch::local("feature/#1194")];
         let mut branches = repo.list_branches(RefScope::Local)?;
@@ -76,7 +74,7 @@ mod namespace {
     fn golden_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let history = repo.history(Branch::local("master").into())?;
+        let history = repo.history(&Branch::local("master"))?;
 
         assert_eq!(repo.which_namespace(), Ok(None));
 
@@ -87,8 +85,8 @@ mod namespace {
             Ok(Some(Namespace::try_from("golden")?))
         );
 
-        let golden_history = repo.history(Branch::local("master").into())?;
-        assert_eq!(history, golden_history);
+        let golden_history = repo.history(&Branch::local("master"))?;
+        assert_eq!(history.head(), golden_history.head());
 
         let expected_branches: Vec<Branch> = vec![Branch::local("banana"), Branch::local("master")];
         let mut branches = repo.list_branches(RefScope::Local)?;
@@ -115,7 +113,7 @@ mod namespace {
     fn silver_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let history = repo.history(Branch::local("master").into())?;
+        let history = repo.history(&Branch::local("master"))?;
 
         assert_eq!(repo.which_namespace(), Ok(None));
 
@@ -124,8 +122,8 @@ mod namespace {
             repo.which_namespace(),
             Ok(Some(Namespace::try_from("golden/silver")?))
         );
-        let silver_history = repo.history(Branch::local("master").into())?;
-        assert_ne!(history, silver_history);
+        let silver_history = repo.history(&Branch::local("master"))?;
+        assert_ne!(history.head(), silver_history.head());
 
         let expected_branches: Vec<Branch> = vec![Branch::local("master")];
         let mut branches = repo.list_branches(RefScope::All)?;
@@ -157,17 +155,11 @@ mod rev {
     fn _master() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let history = repo.history(Branch::remote("master", "origin").into())?;
+        let mut history = repo.history(&Branch::remote("master", "origin"))?;
 
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
         assert!(
-            history
-                .find(|commit| if commit.id == commit1 {
-                    Some(commit.clone())
-                } else {
-                    None
-                })
-                .is_some(),
+            history.any(|commit| commit.unwrap().id == commit1),
             "commit_id={}, history =\n{:#?}",
             commit1,
             &history
@@ -175,13 +167,7 @@ mod rev {
 
         let commit2 = Oid::from_str("d6880352fc7fda8f521ae9b7357668b17bb5bad5")?;
         assert!(
-            history
-                .find(|commit| if commit.id == commit2 {
-                    Some(commit.clone())
-                } else {
-                    None
-                })
-                .is_some(),
+            history.any(|commit| commit.unwrap().id == commit2),
             "commit_id={}, history =\n{:#?}",
             commit2,
             &history
@@ -193,17 +179,12 @@ mod rev {
     #[test]
     fn commit() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
+        let repo = repo.as_ref();
         let rev: Rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?.into();
-        let history = repo.as_ref().history(rev)?;
+        let mut history = repo.history(&rev)?;
 
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
-        assert!(history
-            .find(|commit| if commit.id == commit1 {
-                Some(commit.clone())
-            } else {
-                None
-            })
-            .is_some());
+        assert!(history.any(|commit| commit.unwrap().id == commit1));
 
         Ok(())
     }
@@ -211,9 +192,10 @@ mod rev {
     #[test]
     fn commit_parents() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
+        let repo = repo.as_ref();
         let rev: Rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?.into();
-        let history = repo.as_ref().history(rev)?;
-        let commit = history.first();
+        let history = repo.history(&rev)?;
+        let commit = history.head();
 
         assert_eq!(
             commit.parents,
@@ -226,17 +208,12 @@ mod rev {
     #[test]
     fn commit_short() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
-        let rev: Rev = repo.as_ref().oid("3873745c8")?.into();
-        let history = repo.as_ref().history(rev)?;
+        let repo = repo.as_ref();
+        let rev: Rev = repo.oid("3873745c8")?.into();
+        let mut history = repo.history(&rev)?;
 
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
-        assert!(history
-            .find(|commit| if commit.id == commit1 {
-                Some(commit.clone())
-            } else {
-                None
-            })
-            .is_some());
+        assert!(history.any(|commit| commit.unwrap().id == commit1));
 
         Ok(())
     }
@@ -244,11 +221,12 @@ mod rev {
     #[test]
     fn tag() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
+        let repo = repo.as_ref();
         let rev: Rev = TagName::new("v0.2.0").into();
-        let history = repo.as_ref().history(rev)?;
+        let history = repo.history(&rev)?;
 
         let commit1 = Oid::from_str("2429f097664f9af0c5b7b389ab998b2199ffa977")?;
-        assert_eq!(history.first().id, commit1);
+        assert_eq!(history.head().id, commit1);
 
         Ok(())
     }
@@ -373,12 +351,23 @@ mod last_commit {
 
         let expected_oid = repo
             .as_ref()
-            .history(Branch::local("master").into())
+            .history(&Branch::local("master"))
             .unwrap()
-            .first()
+            .head()
             .id;
         assert_eq!(root_last_commit_id, Some(expected_oid));
     }
+
+    #[test]
+    fn binary_file() {
+        let repo = Repository::new(GIT_PLATINUM)
+            .expect("Could not retrieve ./data/git-platinum as git repository");
+        let repo = repo.as_ref();
+        let history = repo.history(&Branch::local("dev")).unwrap();
+        let file_commit = history.by_path(unsound::path::new("~/bin/cat")).next();
+        assert!(file_commit.is_some());
+        println!("file commit: {:?}", &file_commit);
+    }
 }
 
 #[cfg(test)]
@@ -389,15 +378,13 @@ mod diff {
 
     #[test]
     fn test_initial_diff() -> Result<(), Error> {
-        let oid = Oid::from_str("d3464e33d75c75c99bfb90fa2e9d16efc0b7d0e3")?;
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let commit = repo.get_git2_commit(oid).unwrap();
+        let oid = Oid::from_str("d3464e33d75c75c99bfb90fa2e9d16efc0b7d0e3")?;
+        let commit = repo.commit(oid).unwrap();
+        assert!(commit.parents.is_empty());
 
-        assert!(commit.parents().count() == 0);
-        assert!(commit.parent(0).is_err());
-
-        let diff = repo.initial_diff(&oid.into())?;
+        let diff = repo.initial_diff(oid)?;
 
         let expected_diff = Diff {
             created: vec![CreateFile {
@@ -424,15 +411,26 @@ mod diff {
         Ok(())
     }
 
+    #[test]
+    fn test_diff_of_rev() -> Result<(), Error> {
+        let repo = Repository::new(GIT_PLATINUM)?;
+        let repo = repo.as_ref();
+        let diff = repo.diff_from_parent("80bacafba303bf0cdf6142921f430ff265f25095")?;
+        assert_eq!(diff.created.len(), 0);
+        assert_eq!(diff.deleted.len(), 0);
+        assert_eq!(diff.moved.len(), 0);
+        assert_eq!(diff.modified.len(), 1);
+        Ok(())
+    }
+
     #[test]
     fn test_diff() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let oid = Oid::from_str("80bacafba303bf0cdf6142921f430ff265f25095")?;
-        let commit = repo.get_git2_commit(oid).unwrap();
-        let parent = commit.parent(0)?;
-        let parent_oid: Oid = parent.id().into();
-        let diff = repo.diff(&parent_oid.into(), &oid.into())?;
+        let oid = "80bacafba303bf0cdf6142921f430ff265f25095";
+        let commit = repo.commit(oid).unwrap();
+        let parent_oid = commit.parents.get(0).unwrap();
+        let diff = repo.diff(*parent_oid, oid)?;
 
         let expected_diff = Diff {
                 created: vec![],
@@ -463,10 +461,7 @@ mod diff {
     fn test_branch_diff() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
-        let diff = repo.diff(
-            &Branch::local("master").into(),
-            &Branch::local("dev").into(),
-        )?;
+        let diff = repo.diff(&Branch::local("master"), &Branch::local("dev"))?;
 
         println!("Diff two branches: master -> dev");
         println!(
@@ -824,7 +819,7 @@ mod code_browsing {
     fn iterate_root_dir_recursive() {
         let repo = Repository::new(GIT_PLATINUM).unwrap();
         let repo = repo.as_ref();
-        let root_dir = repo.snapshot(&Branch::local("master").into()).unwrap();
+        let root_dir = repo.snapshot(&Branch::local("master")).unwrap();
         let count = println_dir(&root_dir, 0);
         assert_eq!(count, 36); // Check total file count.
 
@@ -844,4 +839,28 @@ mod code_browsing {
             count
         }
     }
+
+    #[test]
+    fn test_tag_snapshot() {
+        let repo = Repository::new(GIT_PLATINUM).unwrap();
+        let repo_ref = repo.as_ref();
+        let tags = repo_ref.list_tags(RefScope::Local).unwrap();
+        assert_eq!(tags.len(), 6);
+        let root_dir = repo_ref.snapshot(&tags[0]).unwrap();
+        assert_eq!(root_dir.contents().count(), 1);
+    }
+
+    #[test]
+    fn test_file_history() {
+        let repo = Repository::new(GIT_PLATINUM).unwrap();
+        let repo = repo.as_ref();
+        let history = repo.history(&Branch::local("dev")).unwrap();
+        let path = unsound::path::new("README.md");
+        let mut file_history = history.by_path(path);
+        let commit = file_history.next().unwrap().unwrap();
+        let file = commit
+            .get_file(&repo, unsound::path::new("README.md"))
+            .unwrap();
+        assert_eq!(file.size(), 67);
+    }
 }