Implement parsing of provider in tool ids, dont require provider arg …

…for artifact source

lib/sources/artifact.rs

@@ -17,9 +17,12 @@ use super::{
 
 /**
     An artifact provider supported by Rokit.
+
+    The default provider is [`ArtifactProvider::GitHub`].
 */
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ArtifactProvider {
+    #[default]
     GitHub,
 }
 

lib/sources/source.rs

@@ -57,12 +57,8 @@ impl ArtifactSource {
 
         - If the latest release could not be fetched.
     */
-    pub async fn get_latest_release(
-        &self,
-        provider: ArtifactProvider,
-        id: &ToolId,
-    ) -> RokitResult<Vec<Artifact>> {
-        Ok(match provider {
+    pub async fn get_latest_release(&self, id: &ToolId) -> RokitResult<Vec<Artifact>> {
+        Ok(match id.provider() {
             ArtifactProvider::GitHub => self.github.get_latest_release(id).await?,
         })
     }
@@ -74,12 +70,8 @@ impl ArtifactSource {
 
         - If the specific release could not be fetched.
     */
-    pub async fn get_specific_release(
-        &self,
-        provider: ArtifactProvider,
-        spec: &ToolSpec,
-    ) -> RokitResult<Vec<Artifact>> {
-        Ok(match provider {
+    pub async fn get_specific_release(&self, spec: &ToolSpec) -> RokitResult<Vec<Artifact>> {
+        Ok(match spec.provider() {
             ArtifactProvider::GitHub => self.github.get_specific_release(spec).await?,
         })
     }

lib/tool/id.rs

@@ -4,6 +4,8 @@ use semver::Version;
 use serde_with::{DeserializeFromStr, SerializeDisplay};
 use thiserror::Error;
 
+use crate::sources::ArtifactProvider;
+
 use super::{util::is_invalid_identifier, ToolAlias, ToolSpec};
 
 /**
@@ -15,6 +17,8 @@ pub enum ToolIdParseError {
     Empty,
     #[error("missing '/' separator")]
     MissingSeparator,
+    #[error("artifact provider '{0}' is invalid")]
+    InvalidProvider(String),
     #[error("author '{0}' is empty or invalid")]
     InvalidAuthor(String),
     #[error("name '{0}' is empty or invalid")]
@@ -24,17 +28,23 @@ pub enum ToolIdParseError {
 /**
     A tool identifier, which includes the author and name of a tool.
 
+    Also includes the provider of the artifact, which by default is `GitHub`.
+
     Used to uniquely identify a tool, but not its version.
 */
-#[derive(
-    Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, DeserializeFromStr, SerializeDisplay,
-)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, DeserializeFromStr, SerializeDisplay)]
 pub struct ToolId {
+    pub(super) provider: ArtifactProvider,
     pub(super) author: String,
     pub(super) name: String,
 }
 
 impl ToolId {
+    #[must_use]
+    pub fn provider(&self) -> ArtifactProvider {
+        self.provider
+    }
+
     #[must_use]
     pub fn author(&self) -> &str {
         &self.author
@@ -56,14 +66,37 @@ impl ToolId {
     }
 }
 
+impl Ord for ToolId {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.author
+            .cmp(&other.author)
+            .then_with(|| self.name.cmp(&other.name))
+    }
+}
+
+impl PartialOrd for ToolId {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 impl FromStr for ToolId {
     type Err = ToolIdParseError;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if s.is_empty() {
             return Err(ToolIdParseError::Empty);
         }
 
-        let Some((before, after)) = s.split_once('/') else {
+        let (provider, after_provider) = match s.split_once(':') {
+            None => (ArtifactProvider::default(), s),
+            Some((left, right)) => {
+                let provider = ArtifactProvider::from_str(left)
+                    .map_err(|e| ToolIdParseError::InvalidProvider(e.to_string()))?;
+                (provider, right)
+            }
+        };
+
+        let Some((before, after)) = after_provider.split_once('/') else {
             return Err(ToolIdParseError::MissingSeparator);
         };
 
@@ -78,6 +111,7 @@ impl FromStr for ToolId {
         }
 
         Ok(Self {
+            provider,
             author: before.to_string(),
             name: after.to_string(),
         })
@@ -96,6 +130,15 @@ mod tests {
 
     fn new_id(author: &str, name: &str) -> ToolId {
         ToolId {
+            provider: ArtifactProvider::default(),
+            author: author.to_string(),
+            name: name.to_string(),
+        }
+    }
+
+    fn new_id_with_provider(provider: ArtifactProvider, author: &str, name: &str) -> ToolId {
+        ToolId {
+            provider,
             author: author.to_string(),
             name: name.to_string(),
         }
@@ -136,6 +179,17 @@ mod tests {
         assert_eq!("a/b ".parse::<ToolId>().unwrap(), id);
     }
 
+    #[test]
+    fn parse_valid_provider() {
+        // Known provider strings should parse ok
+        assert!("github:a/b".parse::<ToolId>().is_ok());
+        // The parsed ToolId should match the input
+        assert_eq!(
+            "github:a/b".parse::<ToolId>().unwrap(),
+            new_id_with_provider(ArtifactProvider::GitHub, "a", "b")
+        );
+    }
+
     #[test]
     fn parse_invalid_missing() {
         // Empty strings or parts should not be allowed
@@ -151,4 +205,16 @@ mod tests {
         assert!("a/b/".parse::<ToolId>().is_err());
         assert!("a/b/c".parse::<ToolId>().is_err());
     }
+
+    #[test]
+    fn parse_invalid_provider() {
+        // Empty provider should not be allowed
+        assert!(":a/b".parse::<ToolId>().is_err());
+        assert!(":a/b".parse::<ToolId>().is_err());
+        assert!(":a/b".parse::<ToolId>().is_err());
+        // Unrecognized provider should not be allowed
+        assert!("unknown:a/b".parse::<ToolId>().is_err());
+        assert!("hubgit:a/b".parse::<ToolId>().is_err());
+        assert!("bitbab:a/b".parse::<ToolId>().is_err());
+    }
 }

lib/tool/spec.rs

@@ -4,6 +4,8 @@ use semver::Version;
 use serde_with::{DeserializeFromStr, SerializeDisplay};
 use thiserror::Error;
 
+use crate::sources::ArtifactProvider;
+
 use super::{util::is_invalid_identifier, ToolId, ToolIdParseError};
 
 /**
@@ -38,14 +40,19 @@ pub struct ToolSpec {
 }
 
 impl ToolSpec {
+    #[must_use]
+    pub fn provider(&self) -> ArtifactProvider {
+        self.id.provider()
+    }
+
     #[must_use]
     pub fn author(&self) -> &str {
-        &self.id.author
+        self.id.author()
     }
 
     #[must_use]
     pub fn name(&self) -> &str {
-        &self.id.name
+        self.id.name()
     }
 
     #[must_use]
@@ -115,6 +122,7 @@ mod tests {
     fn new_spec(author: &str, name: &str, version: &str) -> ToolSpec {
         ToolSpec {
             id: ToolId {
+                provider: ArtifactProvider::default(),
                 author: author.to_string(),
                 name: name.to_string(),
             },

lib/tool/util.rs

@@ -1,5 +1,9 @@
 pub fn is_invalid_identifier(s: &str) -> bool {
     s.is_empty() // Must not be empty
         || s.chars().all(char::is_whitespace) // Must contain some information
-        || s.chars().any(|c| c == '/') // Must not contain the separator character
+        || s.chars().any(|c|
+               c == ':' // Must not contain the provider separator character
+            || c == '/' // Must not contain the author/name separator character
+            || c == '@' // Must not contain the version separator character
+        )
 }

src/cli/add.rs

@@ -5,7 +5,7 @@ use console::style;
 use rokit::{
     discovery::discover_all_manifests,
     manifests::RokitManifest,
-    sources::{Artifact, ArtifactProvider},
+    sources::Artifact,
     storage::Home,
     tool::{ToolAlias, ToolId},
 };
@@ -84,9 +84,7 @@ impl AddSubcommand {
         let pb = new_progress_bar("Fetching", 3, 1);
         let (spec, artifact) = match self.tool.clone() {
             ToolIdOrSpec::Spec(spec) => {
-                let artifacts = source
-                    .get_specific_release(ArtifactProvider::GitHub, &spec)
-                    .await?;
+                let artifacts = source.get_specific_release(&spec).await?;
                 let artifact = Artifact::sort_by_system_compatibility(&artifacts)
                     .first()
                     .cloned()
@@ -95,9 +93,7 @@ impl AddSubcommand {
                 (spec, artifact)
             }
             ToolIdOrSpec::Id(id) => {
-                let artifacts = source
-                    .get_latest_release(ArtifactProvider::GitHub, &id)
-                    .await?;
+                let artifacts = source.get_latest_release(&id).await?;
                 let artifact = Artifact::sort_by_system_compatibility(&artifacts)
                     .first()
                     .cloned()

src/cli/init.rs

@@ -1,7 +1,7 @@
 use anyhow::{bail, Context, Result};
 use clap::Parser;
-
 use console::style;
+
 use rokit::{manifests::RokitManifest, storage::Home, system::current_dir};
 
 use crate::util::{finish_progress_bar, new_progress_bar};

src/cli/install.rs

@@ -5,11 +5,7 @@ use clap::Parser;
 
 use console::style;
 use futures::{stream::FuturesUnordered, TryStreamExt};
-use rokit::{
-    discovery::discover_all_manifests,
-    sources::{Artifact, ArtifactProvider},
-    storage::Home,
-};
+use rokit::{discovery::discover_all_manifests, sources::Artifact, storage::Home};
 
 use crate::util::{finish_progress_bar, new_progress_bar, prompt_for_trust_specs};
 
@@ -84,9 +80,7 @@ impl InstallSubcommand {
                     return anyhow::Ok(tool_spec);
                 }
 
-                let artifacts = source
-                    .get_specific_release(ArtifactProvider::GitHub, &tool_spec)
-                    .await?;
+                let artifacts = source.get_specific_release(&tool_spec).await?;
                 pb.inc(1);
 
                 let artifact = Artifact::sort_by_system_compatibility(&artifacts)

src/cli/list.rs

@@ -1,7 +1,7 @@
 use anyhow::Result;
 use clap::Parser;
-
 use console::style;
+
 use rokit::{storage::Home, tool::ToolId};
 
 /// Lists all existing tools managed by Rokit.

src/cli/self_update.rs

@@ -1,14 +1,10 @@
 use anyhow::{bail, Context, Result};
 use clap::Parser;
-
 use console::style;
-use rokit::{
-    sources::{Artifact, ArtifactProvider},
-    storage::Home,
-    tool::ToolId,
-};
 use semver::Version;
 
+use rokit::{sources::Artifact, storage::Home, tool::ToolId};
+
 use crate::util::{finish_progress_bar, new_progress_bar};
 
 /// Updates Rokit to the latest version.
@@ -38,9 +34,7 @@ impl SelfUpdateSubcommand {
         pb.inc(1);
         pb.set_message("Fetching");
 
-        let artifacts = source
-            .get_latest_release(ArtifactProvider::GitHub, &tool_id)
-            .await?;
+        let artifacts = source.get_latest_release(&tool_id).await?;
 
         // Skip updating if we are already on the latest version
         let version_current = env!("CARGO_PKG_VERSION").parse::<Version>().unwrap();

src/cli/update.rs

@@ -1,13 +1,10 @@
 use anyhow::{bail, Context, Result};
 use clap::Parser;
-
 use console::style;
 use futures::{stream::FuturesUnordered, TryStreamExt};
+
 use rokit::{
-    discovery::discover_all_manifests,
-    manifests::RokitManifest,
-    sources::{Artifact, ArtifactProvider},
-    storage::Home,
+    discovery::discover_all_manifests, manifests::RokitManifest, sources::Artifact, storage::Home,
 };
 
 use crate::util::{finish_progress_bar, new_progress_bar, ToolAliasOrIdOrSpec, ToolIdOrSpec};
@@ -123,10 +120,8 @@ impl UpdateSubcommand {
             .map(|(alias, tool)| async {
                 let (alias, id, artifacts) = match tool {
                     ToolIdOrSpec::Spec(spec) => {
-                        let artifacts = source
-                            .get_specific_release(ArtifactProvider::GitHub, &spec)
-                            .await
-                            .with_context(|| {
+                        let artifacts =
+                            source.get_specific_release(&spec).await.with_context(|| {
                                 format!(
                                     "Failed to fetch release for '{spec}'!\
                                     \nMake sure the given tool version exists."
@@ -135,10 +130,8 @@ impl UpdateSubcommand {
                         (alias, spec.id().clone(), artifacts)
                     }
                     ToolIdOrSpec::Id(id) => {
-                        let artifacts = source
-                            .get_latest_release(ArtifactProvider::GitHub, &id)
-                            .await
-                            .with_context(|| {
+                        let artifacts =
+                            source.get_latest_release(&id).await.with_context(|| {
                                 format!(
                                     "Failed to fetch latest release for '{id}'!\
                                     \nMake sure the given tool identifier exists."