Merge remote-tracking branch 'origin/surf/infallible-glob'

* origin/surf/infallible-glob:
  radicle-link: refactor glob

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

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();