From d7b5186d8c5c24ac00e672f19edc854d03b99390 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Fri, 18 Nov 2022 17:26:28 +0000 Subject: [PATCH] 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` methods into `Glob` and `Glob`. 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 X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/src/commit.rs | 2 +- radicle-surf/src/git/glob.rs | 288 ++++++++++++++++++++-------- radicle-surf/src/git/repo.rs | 22 ++- radicle-surf/src/revision.rs | 33 ++-- radicle-surf/t/src/git/namespace.rs | 12 +- radicle-surf/t/src/git/reference.rs | 21 +- radicle-surf/t/src/git/threading.rs | 2 +- 7 files changed, 268 insertions(+), 112 deletions(-) diff --git a/radicle-surf/src/commit.rs b/radicle-surf/src/commit.rs index a5d39b8..d2b77cf 100644 --- a/radicle-surf/src/commit.rs +++ b/radicle-surf/src/commit.rs @@ -195,7 +195,7 @@ pub fn commit(repo: &Repository, rev: R) -> Result. -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 { globs: Vec>, glob_type: PhantomData, // To support different methods for different T. } impl Glob { + /// Return the [`QualifiedPattern`] globs of this `Glob`. pub fn globs(&self) -> impl Iterator> { 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` can only combine with `Glob`, + /// `Glob` can combine with `Glob`, etc. + pub fn and(mut self, other: impl Into) -> Self { + self.globs.extend(other.into().globs); + self + } } impl Glob { - /// Creates a `Glob` for namespaces. - pub fn namespaces(glob: &str) -> Result { - 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.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, 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 for Glob { 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 for Glob { } } +impl Extend for Glob { + fn extend>(&mut self, iter: T) { + self.globs.extend(iter.into_iter().map(|pat| { + qualify(&refname!("refs/namespaces"), pat).expect("BUG: pattern is qualified") + })) + } +} + impl Glob { - /// Creates a `Glob` for local tags. - pub fn tags(glob: &str) -> Result { - 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.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, 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 for Glob { fn from_iter>(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 for Glob { } } -impl Glob { - /// Creates a `Glob` for local branches. - pub fn heads(glob: &str) -> Result { - 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 { - 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.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.globs.push(Self::qualify_remotes(glob)?); - Ok(self) - } - - fn qualify_heads(glob: &str) -> Result, Error> { - Ok( - qualify(refname!("refs/heads"), PatternString::try_from(glob)?) - .expect("BUG: pattern is qualified"), - ) - } - - fn qualify_remotes(glob: &str) -> Result, Error> { - Ok( - qualify(refname!("refs/remotes"), PatternString::try_from(glob)?) - .expect("BUG: pattern is qualified"), +impl Extend for Glob { + fn extend>(&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> { +impl Glob { + /// 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` with `Glob`, use + /// `branches` to convert this `Glob` into a + /// `Glob`. + /// + /// # Example + /// ```no_run + /// Glob::heads(pattern!("features/*")) + /// .insert(pattern!("qa/*")) + /// .branches() + /// .and(Glob::remotes(pattern!("origin/features/*"))) + /// ``` + pub fn branches(self) -> Glob { + self.into() + } + + fn qualify_heads(glob: PatternString) -> QualifiedPattern<'static> { + qualify(&refname!("refs/heads"), glob).expect("BUG: pattern is qualified") + } +} + +impl FromIterator for Glob { + fn from_iter>(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 for Glob { + fn extend>(&mut self, iter: T) { + self.globs.extend( + iter.into_iter().map(|pat| { + qualify(&refname!("refs/heads"), pat).expect("BUG: pattern is qualified") + }), + ) + } +} + +impl From> for Glob { + fn from(Glob { globs, .. }: Glob) -> Self { + Self { + globs, + glob_type: PhantomData, + } + } +} + +impl Glob { + /// 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` with `Glob`, use + /// `branches` to convert this `Glob` into a + /// `Glob`. + /// + /// # Example + /// ```no_run + /// Glob::remotes(pattern!("origin/features/*")) + /// .insert(pattern!("origin/qa/*")) + /// .branches() + /// .and(Glob::heads(pattern!("features/*"))) + /// ``` + pub fn branches(self) -> Glob { + self.into() + } + + fn qualify_remotes(glob: PatternString) -> QualifiedPattern<'static> { + qualify(&refname!("refs/remotes"), glob).expect("BUG: pattern is qualified") + } +} + +impl FromIterator for Glob { + fn from_iter>(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 for Glob { + fn extend>(&mut self, iter: T) { + self.globs.extend( + iter.into_iter().map(|pat| { + qualify(&refname!("refs/remotes"), pat).expect("BUG: pattern is qualified") + }), + ) + } +} + +impl From> for Glob { + fn from(Glob { globs, .. }: Glob) -> Self { + Self { + globs, + glob_type: PhantomData, + } + } +} + +fn qualify(prefix: &RefString, glob: PatternString) -> Option> { prefix.to_pattern(glob).qualified().map(|q| q.into_owned()) } diff --git a/radicle-surf/src/git/repo.rs b/radicle-surf/src/git/repo.rs index ad2cb30..a1770ce 100644 --- a/radicle-surf/src/git/repo.rs +++ b/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) -> Result { + pub fn branches(&self, pattern: G) -> Result + where + G: Into>, + { + 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(&self, commit: C) -> Result { - 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) -> Result { + pub fn branch_names(&self, filter: G) -> Result + where + G: Into>, + { Ok(self.branches(filter)?.names()) } /// Lists tag names in the local RefScope. pub fn tag_names(&self) -> Result { - 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) -> Result, 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())?; diff --git a/radicle-surf/src/revision.rs b/radicle-surf/src/revision.rs index 37fb261..9651cf7 100644 --- a/radicle-surf/src/revision.rs +++ b/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 { /// # Errors /// /// * If we cannot get the branches from the `Browser` -pub fn remote(repo: &Repository, peer_id: P, user: U) -> Result, Error> -where - P: Clone + ToString, -{ +pub fn remote( + repo: &Repository, + peer_id: RefString, + user: U, +) -> Result, 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::>()?; Ok(Revisions { @@ -144,12 +146,13 @@ where /// # Errors /// /// * If we cannot get the branches from the `Browser` -pub fn local(repo: &Repository, peer_id: P, user: U) -> Result, Error> -where - P: Clone + ToString, -{ +pub fn local( + repo: &Repository, + peer_id: RefString, + user: U, +) -> Result, Error> { let branches = repo - .branch_names(&Glob::heads("*")?)? + .branch_names(Glob::all_heads())? .map(|name| name.map(RefString::from)) .collect::>()?; let tags = repo @@ -175,10 +178,10 @@ where /// # Errors /// /// * If we cannot get the branches from the `Browser` -pub fn revisions(repo: &Repository, peer: Category) -> Result, Error> -where - P: Clone + ToString, -{ +pub fn revisions( + repo: &Repository, + peer: Category, +) -> Result, Error> { match peer { Category::Local { peer_id, user } => local(repo, peer_id, user), Category::Remote { peer_id, user } => remote(repo, peer_id, user), diff --git a/radicle-surf/t/src/git/namespace.rs b/radicle-surf/t/src/git/namespace.rs index 5d34125..4bc7a29 100644 --- a/radicle-surf/t/src/git/namespace.rs +++ b/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 = vec![Branch::local(refname!("feature/#1194"))]; let mut branches = repo - .branches(&Glob::heads("*")?)? + .branches(Glob::all_heads())? .collect::, _>>()?; 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::, _>>()?; 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::, _>>()?; 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::, _>>()?; branches.sort(); @@ -111,7 +111,7 @@ fn silver_namespace() -> Result<(), Error> { let expected_branches: Vec = vec![Branch::local(refname!("master"))]; let mut branches = repo - .branches(&Glob::heads("*")?.and_remotes("*")?)? + .branches(Glob::all_heads().branches().and(Glob::all_remotes()))? .collect::, _>>()?; branches.sort(); diff --git a/radicle-surf/t/src/git/reference.rs b/radicle-surf/t/src/git/reference.rs index ac2d4f6..2a1b91a 100644 --- a/radicle-surf/t/src/git/reference.rs +++ b/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::, _>>() .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); } diff --git a/radicle-surf/t/src/git/threading.rs b/radicle-surf/t/src/git/threading.rs index 7448cf0..8392a92 100644 --- a/radicle-surf/t/src/git/threading.rs +++ b/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 = shared_repo.lock().unwrap(); let mut branches = locked_repo - .branches(&Glob::heads("*")?.and_remotes("*")?)? + .branches(Glob::all_heads().branches().and(Glob::all_remotes()))? .collect::, _>>()?; branches.sort();