radicle-link: refactor glob

This patch changes the glob module in a couple of ways, outlined
below.

The first change is the use of PatternString in the constructor
functions. This removes the fallibility of the constructors. This
meant that the tests changed to use the `pattern!` macro instead.

The second change is splitting out the `Glob<Branch>` methods into
`Glob<Local>` and `Glob<Remote>`. This means that the local and remote
globs can be constructed separately, and are then combined via the
`branches` and `and` methods.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett

radicle-surf/src/commit.rs

@@ -195,7 +195,7 @@ pub fn commit<R: git::Revision>(repo: &Repository, rev: R) -> Result<Commit, Err
     }
 
     let branches = repo
-        .revision_branches(&sha1, Glob::heads("*")?.and_remotes("*")?)?
+        .revision_branches(&sha1, Glob::all_heads().branches().and(Glob::all_remotes()))?
         .into_iter()
         .map(|b| b.refname().into())
         .collect();

radicle-surf/src/git/glob.rs

@@ -15,16 +15,16 @@
 // 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 std::{convert::TryFrom, marker::PhantomData, str};
+use std::marker::PhantomData;
 
 use git_ref_format::{
     refname,
-    refspec::{PatternString, QualifiedPattern},
+    refspec::{self, PatternString, QualifiedPattern},
     RefString,
 };
 use thiserror::Error;
 
-use crate::git::{Branch, Namespace, Tag};
+use crate::git::{Branch, Local, Namespace, Remote, Tag};
 
 #[derive(Debug, Error)]
 pub enum Error {
@@ -33,38 +33,52 @@ pub enum Error {
 }
 
 /// A collection of globs for T (a git reference type).
+#[derive(Clone, Debug)]
 pub struct Glob<T> {
     globs: Vec<QualifiedPattern<'static>>,
     glob_type: PhantomData<T>, // To support different methods for different T.
 }
 
 impl<T> Glob<T> {
+    /// Return the [`QualifiedPattern`] globs of this `Glob`.
     pub fn globs(&self) -> impl Iterator<Item = &QualifiedPattern<'static>> {
         self.globs.iter()
     }
+
+    /// Combine two `Glob`s together by combining their glob lists together.
+    ///
+    /// Note that the `Glob`s must result in the same type,
+    /// e.g. `Glob<Tag>` can only combine with `Glob<Tag>`,
+    /// `Glob<Local>` can combine with `Glob<Remote>`, etc.
+    pub fn and(mut self, other: impl Into<Self>) -> Self {
+        self.globs.extend(other.into().globs);
+        self
+    }
 }
 
 impl Glob<Namespace> {
-    /// Creates a `Glob` for namespaces.
-    pub fn namespaces(glob: &str) -> Result<Self, Error> {
-        let globs = vec![Self::qualify(glob)?];
-        Ok(Self {
+    /// Creates the `Glob` that mathces all `refs/namespaces`.
+    pub fn all_namespaces() -> Self {
+        Self::namespaces(refspec::pattern!("*"))
+    }
+
+    /// Creates a `Glob` for `refs/namespaces`, starting with `glob`.
+    pub fn namespaces(glob: PatternString) -> Self {
+        let globs = vec![Self::qualify(glob)];
+        Self {
             globs,
             glob_type: PhantomData,
-        })
+        }
     }
 
-    /// Adds namespaces patterns to existing `Glob`.
-    pub fn and(mut self, glob: &str) -> Result<Self, Error> {
-        self.globs.push(Self::qualify(glob)?);
-        Ok(self)
+    /// Adds a `refs/namespaces` pattern to this `Glob`.
+    pub fn insert(mut self, glob: PatternString) -> Self {
+        self.globs.push(Self::qualify(glob));
+        self
     }
 
-    fn qualify(glob: &str) -> Result<QualifiedPattern<'static>, Error> {
-        Ok(
-            qualify(refname!("refs/namespaces"), PatternString::try_from(glob)?)
-                .expect("BUG: pattern is qualified"),
-        )
+    fn qualify(glob: PatternString) -> QualifiedPattern<'static> {
+        qualify(&refname!("refs/namespaces"), glob).expect("BUG: pattern is qualified")
     }
 }
 
@@ -73,7 +87,7 @@ impl FromIterator<PatternString> for Glob<Namespace> {
         let globs = iter
             .into_iter()
             .map(|pat| {
-                qualify(refname!("refs/namespaces"), pat).expect("BUG: pattern is qualified")
+                qualify(&refname!("refs/namespaces"), pat).expect("BUG: pattern is qualified")
             })
             .collect();
 
@@ -84,28 +98,37 @@ impl FromIterator<PatternString> for Glob<Namespace> {
     }
 }
 
+impl Extend<PatternString> for Glob<Namespace> {
+    fn extend<T: IntoIterator<Item = PatternString>>(&mut self, iter: T) {
+        self.globs.extend(iter.into_iter().map(|pat| {
+            qualify(&refname!("refs/namespaces"), pat).expect("BUG: pattern is qualified")
+        }))
+    }
+}
+
 impl Glob<Tag> {
-    /// Creates a `Glob` for local tags.
-    pub fn tags(glob: &str) -> Result<Self, Error> {
-        let pattern = Self::qualify(glob)?;
-        let globs = vec![pattern];
-        Ok(Self {
+    /// Creates a `Glob` that matches all `refs/tags`.
+    pub fn all_tags() -> Self {
+        Self::tags(refspec::pattern!("*"))
+    }
+
+    /// Creates a `Glob` for `refs/tags`, starting with `glob`.
+    pub fn tags(glob: PatternString) -> Self {
+        let globs = vec![Self::qualify(glob)];
+        Self {
             globs,
             glob_type: PhantomData,
-        })
+        }
     }
 
-    /// Updates a `Glob` to include other tags.
-    pub fn and_tags(mut self, glob: &str) -> Result<Self, Error> {
-        self.globs.push(Self::qualify(glob)?);
-        Ok(self)
+    /// Adds a `refs/tags` pattern to this `Glob`.
+    pub fn insert(mut self, glob: PatternString) -> Self {
+        self.globs.push(Self::qualify(glob));
+        self
     }
 
-    fn qualify(glob: &str) -> Result<QualifiedPattern<'static>, Error> {
-        Ok(
-            qualify(refname!("refs/tags"), PatternString::try_from(glob)?)
-                .expect("BUG: pattern is qualified"),
-        )
+    fn qualify(glob: PatternString) -> QualifiedPattern<'static> {
+        qualify(&refname!("refs/tags"), glob).expect("BUG: pattern is qualified")
     }
 }
 
@@ -113,7 +136,7 @@ impl FromIterator<PatternString> for Glob<Tag> {
     fn from_iter<T: IntoIterator<Item = PatternString>>(iter: T) -> Self {
         let globs = iter
             .into_iter()
-            .map(|pat| qualify(refname!("refs/tags"), pat).expect("BUG: pattern is qualified"))
+            .map(|pat| qualify(&refname!("refs/tags"), pat).expect("BUG: pattern is qualified"))
             .collect();
 
         Self {
@@ -123,52 +146,163 @@ impl FromIterator<PatternString> for Glob<Tag> {
     }
 }
 
-impl Glob<Branch> {
-    /// Creates a `Glob` for local branches.
-    pub fn heads(glob: &str) -> Result<Self, Error> {
-        let globs = vec![Self::qualify_heads(glob)?];
-        Ok(Self {
-            globs,
-            glob_type: PhantomData,
-        })
-    }
-
-    /// Creates a `Glob` for remote branches.
-    pub fn remotes(glob: &str) -> Result<Self, Error> {
-        let globs = vec![Self::qualify_remotes(glob)?];
-        Ok(Self {
-            globs,
-            glob_type: PhantomData,
-        })
-    }
-
-    /// Updates a `Glob` to include local branches.
-    pub fn and_heads(mut self, glob: &str) -> Result<Self, Error> {
-        self.globs.push(Self::qualify_heads(glob)?);
-        Ok(self)
-    }
-
-    /// Updates a `Glob` to include remote branches.
-    pub fn and_remotes(mut self, glob: &str) -> Result<Self, Error> {
-        self.globs.push(Self::qualify_remotes(glob)?);
-        Ok(self)
-    }
-
-    fn qualify_heads(glob: &str) -> Result<QualifiedPattern<'static>, Error> {
-        Ok(
-            qualify(refname!("refs/heads"), PatternString::try_from(glob)?)
-                .expect("BUG: pattern is qualified"),
-        )
-    }
-
-    fn qualify_remotes(glob: &str) -> Result<QualifiedPattern<'static>, Error> {
-        Ok(
-            qualify(refname!("refs/remotes"), PatternString::try_from(glob)?)
-                .expect("BUG: pattern is qualified"),
+impl Extend<PatternString> for Glob<Tag> {
+    fn extend<T: IntoIterator<Item = PatternString>>(&mut self, iter: T) {
+        self.globs.extend(
+            iter.into_iter()
+                .map(|pat| qualify(&refname!("refs/tag"), pat).expect("BUG: pattern is qualified")),
         )
     }
 }
 
-fn qualify(prefix: RefString, glob: PatternString) -> Option<QualifiedPattern<'static>> {
+impl Glob<Local> {
+    /// Creates the `Glob` that mathces all `refs/heads`.
+    pub fn all_heads() -> Self {
+        Self::heads(refspec::pattern!("*"))
+    }
+
+    /// Creates a `Glob` for `refs/heads`, starting with `glob`.
+    pub fn heads(glob: PatternString) -> Self {
+        let globs = vec![Self::qualify_heads(glob)];
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+
+    /// Adds a `refs/heads` pattern to this `Glob`.
+    pub fn insert(mut self, glob: PatternString) -> Self {
+        self.globs.push(Self::qualify_heads(glob));
+        self
+    }
+
+    /// When chaining `Glob<Local>` with `Glob<Remote>`, use
+    /// `branches` to convert this `Glob<Local>` into a
+    /// `Glob<Branch>`.
+    ///
+    /// # Example
+    /// ```no_run
+    /// Glob::heads(pattern!("features/*"))
+    ///     .insert(pattern!("qa/*"))
+    ///     .branches()
+    ///     .and(Glob::remotes(pattern!("origin/features/*")))
+    /// ```
+    pub fn branches(self) -> Glob<Branch> {
+        self.into()
+    }
+
+    fn qualify_heads(glob: PatternString) -> QualifiedPattern<'static> {
+        qualify(&refname!("refs/heads"), glob).expect("BUG: pattern is qualified")
+    }
+}
+
+impl FromIterator<PatternString> for Glob<Local> {
+    fn from_iter<T: IntoIterator<Item = PatternString>>(iter: T) -> Self {
+        let globs = iter
+            .into_iter()
+            .map(|pat| qualify(&refname!("refs/heads"), pat).expect("BUG: pattern is qualified"))
+            .collect();
+
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+}
+
+impl Extend<PatternString> for Glob<Local> {
+    fn extend<T: IntoIterator<Item = PatternString>>(&mut self, iter: T) {
+        self.globs.extend(
+            iter.into_iter().map(|pat| {
+                qualify(&refname!("refs/heads"), pat).expect("BUG: pattern is qualified")
+            }),
+        )
+    }
+}
+
+impl From<Glob<Local>> for Glob<Branch> {
+    fn from(Glob { globs, .. }: Glob<Local>) -> Self {
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+}
+
+impl Glob<Remote> {
+    /// Creates the `Glob` that mathces all `refs/remotes`.
+    pub fn all_remotes() -> Self {
+        Self::remotes(refspec::pattern!("*"))
+    }
+
+    /// Creates a `Glob` for `refs/remotes`, starting with `glob`.
+    pub fn remotes(glob: PatternString) -> Self {
+        let globs = vec![Self::qualify_remotes(glob)];
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+
+    /// Adds a `refs/remotes` pattern to this `Glob`.
+    pub fn insert(mut self, glob: PatternString) -> Self {
+        self.globs.push(Self::qualify_remotes(glob));
+        self
+    }
+
+    /// When chaining `Glob<Remote>` with `Glob<Local>`, use
+    /// `branches` to convert this `Glob<Remote>` into a
+    /// `Glob<Branch>`.
+    ///
+    /// # Example
+    /// ```no_run
+    /// Glob::remotes(pattern!("origin/features/*"))
+    ///     .insert(pattern!("origin/qa/*"))
+    ///     .branches()
+    ///     .and(Glob::heads(pattern!("features/*")))
+    /// ```
+    pub fn branches(self) -> Glob<Branch> {
+        self.into()
+    }
+
+    fn qualify_remotes(glob: PatternString) -> QualifiedPattern<'static> {
+        qualify(&refname!("refs/remotes"), glob).expect("BUG: pattern is qualified")
+    }
+}
+
+impl FromIterator<PatternString> for Glob<Remote> {
+    fn from_iter<T: IntoIterator<Item = PatternString>>(iter: T) -> Self {
+        let globs = iter
+            .into_iter()
+            .map(|pat| qualify(&refname!("refs/remotes"), pat).expect("BUG: pattern is qualified"))
+            .collect();
+
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+}
+
+impl Extend<PatternString> for Glob<Remote> {
+    fn extend<T: IntoIterator<Item = PatternString>>(&mut self, iter: T) {
+        self.globs.extend(
+            iter.into_iter().map(|pat| {
+                qualify(&refname!("refs/remotes"), pat).expect("BUG: pattern is qualified")
+            }),
+        )
+    }
+}
+
+impl From<Glob<Remote>> for Glob<Branch> {
+    fn from(Glob { globs, .. }: Glob<Remote>) -> Self {
+        Self {
+            globs,
+            glob_type: PhantomData,
+        }
+    }
+}
+
+fn qualify(prefix: &RefString, glob: PatternString) -> Option<QualifiedPattern<'static>> {
     prefix.to_pattern(glob).qualified().map(|q| q.into_owned())
 }

radicle-surf/src/git/repo.rs

@@ -22,7 +22,10 @@ use std::{
     str,
 };
 
-use git_ref_format::{refspec::QualifiedPattern, Qualified};
+use git_ref_format::{
+    refspec::{self, QualifiedPattern},
+    Qualified,
+};
 use radicle_git_ext::Oid;
 use thiserror::Error;
 
@@ -108,7 +111,11 @@ impl Repository {
     }
 
     /// Returns an iterator of branches that match `pattern`.
-    pub fn branches(&self, pattern: &Glob<Branch>) -> Result<Branches, Error> {
+    pub fn branches<G>(&self, pattern: G) -> Result<Branches, Error>
+    where
+        G: Into<Glob<Branch>>,
+    {
+        let pattern = pattern.into();
         let mut branches = Branches::default();
         for glob in pattern.globs() {
             let namespaced = self.namespaced_pattern(glob)?;
@@ -209,7 +216,7 @@ impl Repository {
 
     /// Gets stats of `commit`.
     pub fn get_commit_stats<C: ToCommit>(&self, commit: C) -> Result<Stats, Error> {
-        let branches = self.branches(&Glob::heads("*")?)?.count();
+        let branches = self.branches(Glob::all_heads())?.count();
         let history = self.history(commit)?;
         let mut commits = 0;
 
@@ -255,13 +262,16 @@ impl Repository {
     }
 
     /// Lists branch names with `filter`.
-    pub fn branch_names(&self, filter: &Glob<Branch>) -> Result<BranchNames, Error> {
+    pub fn branch_names<G>(&self, filter: G) -> Result<BranchNames, Error>
+    where
+        G: Into<Glob<Branch>>,
+    {
         Ok(self.branches(filter)?.names())
     }
 
     /// Lists tag names in the local RefScope.
     pub fn tag_names(&self) -> Result<TagNames, Error> {
-        Ok(self.tags(&Glob::tags("*")?)?.names())
+        Ok(self.tags(&Glob::tags(refspec::pattern!("*")))?.names())
     }
 
     /// Returns the Oid of the current HEAD
@@ -336,7 +346,7 @@ impl Repository {
     /// Lists branches that are reachable from `oid`.
     pub fn revision_branches(&self, oid: &Oid, glob: Glob<Branch>) -> Result<Vec<Branch>, Error> {
         let mut contained_branches = vec![];
-        for branch in self.branches(&glob)? {
+        for branch in self.branches(glob)? {
             let branch = branch?;
             let namespaced = self.namespaced_refname(&branch.refname())?;
             let reference = self.inner.find_reference(namespaced.as_str())?;

radicle-surf/src/revision.rs

@@ -19,7 +19,7 @@
 
 use std::collections::BTreeSet;
 
-use git_ref_format::{lit, Qualified, RefString};
+use git_ref_format::{lit, refspec, Qualified, RefString};
 
 #[cfg(feature = "serialize")]
 use serde::{Deserialize, Serialize};
@@ -118,12 +118,14 @@ pub struct Revisions<P, U> {
 /// # Errors
 ///
 ///   * If we cannot get the branches from the `Browser`
-pub fn remote<P, U>(repo: &Repository, peer_id: P, user: U) -> Result<Revisions<P, U>, Error>
-where
-    P: Clone + ToString,
-{
+pub fn remote<U>(
+    repo: &Repository,
+    peer_id: RefString,
+    user: U,
+) -> Result<Revisions<RefString, U>, Error> {
+    let pattern = peer_id.to_pattern(refspec::pattern!("*"));
     let branches = repo
-        .branch_names(&Glob::remotes(&format!("{}/*", peer_id.to_string()))?)?
+        .branch_names(Glob::remotes(pattern))?
         .map(|name| name.map(RefString::from))
         .collect::<Result<_, _>>()?;
     Ok(Revisions {
@@ -144,12 +146,13 @@ where
 /// # Errors
 ///
 ///   * If we cannot get the branches from the `Browser`
-pub fn local<P, U>(repo: &Repository, peer_id: P, user: U) -> Result<Revisions<P, U>, Error>
-where
-    P: Clone + ToString,
-{
+pub fn local<U>(
+    repo: &Repository,
+    peer_id: RefString,
+    user: U,
+) -> Result<Revisions<RefString, U>, Error> {
     let branches = repo
-        .branch_names(&Glob::heads("*")?)?
+        .branch_names(Glob::all_heads())?
         .map(|name| name.map(RefString::from))
         .collect::<Result<_, _>>()?;
     let tags = repo
@@ -175,10 +178,10 @@ where
 /// # Errors
 ///
 ///   * If we cannot get the branches from the `Browser`
-pub fn revisions<P, U>(repo: &Repository, peer: Category<P, U>) -> Result<Revisions<P, U>, Error>
-where
-    P: Clone + ToString,
-{
+pub fn revisions<U>(
+    repo: &Repository,
+    peer: Category<RefString, U>,
+) -> Result<Revisions<RefString, U>, Error> {
     match peer {
         Category::Local { peer_id, user } => local(repo, peer_id, user),
         Category::Remote { peer_id, user } => remote(repo, peer_id, user),

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

@@ -1,4 +1,4 @@
-use git_ref_format::{name::component, refname};
+use git_ref_format::{name::component, refname, refspec};
 use pretty_assertions::{assert_eq, assert_ne};
 use radicle_surf::git::{Branch, Error, Glob, Repository};
 
@@ -31,7 +31,7 @@ fn me_namespace() -> Result<(), Error> {
 
     let expected_branches: Vec<Branch> = vec![Branch::local(refname!("feature/#1194"))];
     let mut branches = repo
-        .branches(&Glob::heads("*")?)?
+        .branches(Glob::all_heads())?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();
 
@@ -42,7 +42,7 @@ fn me_namespace() -> Result<(), Error> {
         refname!("heads/feature/#1194"),
     )];
     let mut branches = repo
-        .branches(&Glob::remotes("fein/*")?)?
+        .branches(Glob::remotes(refspec::pattern!("fein/*")))?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();
 
@@ -70,7 +70,7 @@ fn golden_namespace() -> Result<(), Error> {
         Branch::local(refname!("master")),
     ];
     let mut branches = repo
-        .branches(&Glob::heads("*")?)?
+        .branches(Glob::all_heads())?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();
 
@@ -85,7 +85,7 @@ fn golden_namespace() -> Result<(), Error> {
         Branch::remote(remote, refname!("tags/v0.1.0")),
     ];
     let mut branches = repo
-        .branches(&Glob::remotes("kickflip/*")?)?
+        .branches(Glob::remotes(refspec::pattern!("kickflip/*")))?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();
 
@@ -111,7 +111,7 @@ fn silver_namespace() -> Result<(), Error> {
 
     let expected_branches: Vec<Branch> = vec![Branch::local(refname!("master"))];
     let mut branches = repo
-        .branches(&Glob::heads("*")?.and_remotes("*")?)?
+        .branches(Glob::all_heads().branches().and(Glob::all_remotes()))?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();
 

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

@@ -1,3 +1,4 @@
+use git_ref_format::refspec;
 use radicle_surf::git::{Glob, Repository};
 
 use super::GIT_PLATINUM;
@@ -5,12 +6,17 @@ use super::GIT_PLATINUM;
 #[test]
 fn test_branches() {
     let repo = Repository::open(GIT_PLATINUM).unwrap();
-    let branches = repo.branches(&Glob::heads("*").unwrap()).unwrap();
+    let heads = Glob::all_heads();
+    let branches = repo.branches(heads.clone()).unwrap();
     for b in branches {
         println!("{}", b.unwrap().refname());
     }
     let branches = repo
-        .branches(&Glob::heads("*").unwrap().and_remotes("banana/*").unwrap())
+        .branches(
+            heads
+                .branches()
+                .and(Glob::remotes(refspec::pattern!("banana/*"))),
+        )
         .unwrap();
     for b in branches {
         println!("{}", b.unwrap().refname());
@@ -21,7 +27,7 @@ fn test_branches() {
 fn test_tag_snapshot() {
     let repo = Repository::open(GIT_PLATINUM).unwrap();
     let tags = repo
-        .tags(&Glob::tags("*").unwrap())
+        .tags(&Glob::all_tags())
         .unwrap()
         .collect::<Result<Vec<_>, _>>()
         .unwrap();
@@ -33,14 +39,17 @@ fn test_tag_snapshot() {
 #[test]
 fn test_namespaces() {
     let repo = Repository::open(GIT_PLATINUM).unwrap();
-    let namespaces = repo.namespaces(&Glob::namespaces("*").unwrap()).unwrap();
+
+    let namespaces = repo.namespaces(&Glob::all_namespaces()).unwrap();
     assert_eq!(namespaces.count(), 3);
     let namespaces = repo
-        .namespaces(&Glob::namespaces("golden/*").unwrap())
+        .namespaces(&Glob::namespaces(refspec::pattern!("golden/*")))
         .unwrap();
     assert_eq!(namespaces.count(), 2);
     let namespaces = repo
-        .namespaces(&Glob::namespaces("golden/*").unwrap().and("me/*").unwrap())
+        .namespaces(
+            &Glob::namespaces(refspec::pattern!("golden/*")).insert(refspec::pattern!("me/*")),
+        )
         .unwrap();
     assert_eq!(namespaces.count(), 3);
 }

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

@@ -10,7 +10,7 @@ fn basic_test() -> Result<(), Error> {
     let shared_repo = Mutex::new(Repository::open(GIT_PLATINUM)?);
     let locked_repo: MutexGuard<Repository> = shared_repo.lock().unwrap();
     let mut branches = locked_repo
-        .branches(&Glob::heads("*")?.and_remotes("*")?)?
+        .branches(Glob::all_heads().branches().and(Glob::all_remotes()))?
         .collect::<Result<Vec<_>, _>>()?;
     branches.sort();