cli updater adaptation and clippy fixes

crates/typst-cli/Cargo.toml

@@ -21,7 +21,7 @@ doc = false
 typst = { workspace = true }
 typst-eval = { workspace = true }
 typst-html = { workspace = true }
-typst-kit = { workspace = true }
+typst-kit = { workspace = true, features = ["downloads_http"] }
 typst-macros = { workspace = true }
 typst-pdf = { workspace = true }
 typst-render = { workspace = true }

crates/typst-cli/src/update.rs

@@ -7,12 +7,12 @@ use semver::Version;
 use serde::Deserialize;
 use tempfile::NamedTempFile;
 use typst::diag::{bail, StrResult};
-use typst_kit::download::Downloader;
+use typst_kit::package_downloads::http::HttpDownloader;
 use xz2::bufread::XzDecoder;
 use zip::ZipArchive;
 
 use crate::args::UpdateCommand;
-use crate::download::{self, PrintDownload};
+use crate::download::PrintDownload;
 
 const TYPST_GITHUB_ORG: &str = "typst";
 const TYPST_REPO: &str = "typst";
@@ -91,7 +91,8 @@ pub fn update(command: &UpdateCommand) -> StrResult<()> {
     fs::copy(current_exe, &backup_path)
         .map_err(|err| eco_format!("failed to create backup ({err})"))?;
 
-    let downloader = download::downloader();
+    //no certificate is needed to download from GitHub
+    let downloader = HttpDownloader::new(HttpDownloader::default_user_agent());
 
     let release = Release::from_tag(command.version.as_ref(), &downloader)?;
     if !update_needed(&release)? && !command.force {
@@ -133,7 +134,7 @@ impl Release {
     /// Typst repository.
     pub fn from_tag(
         tag: Option<&Version>,
-        downloader: &Downloader,
+        downloader: &HttpDownloader,
     ) -> StrResult<Release> {
         let url = match tag {
             Some(tag) => format!(
@@ -144,7 +145,7 @@ impl Release {
             ),
         };
 
-        match downloader.download(&url) {
+        match downloader.perform_download(&url) {
             Ok(response) => response.into_json().map_err(|err| {
                 eco_format!("failed to parse release information ({err})")
             }),
@@ -161,7 +162,7 @@ impl Release {
     pub fn download_binary(
         &self,
         asset_name: &str,
-        downloader: &Downloader,
+        downloader: &HttpDownloader,
     ) -> StrResult<Vec<u8>> {
         let asset = self.assets.iter().find(|a| a.name.starts_with(asset_name)).ok_or(
             eco_format!(

crates/typst-kit/src/lib.rs

@@ -19,9 +19,9 @@
 //!   [download]. It is enabled by the `packages` feature flag and implies the
 //!   `downloads` feature flag.
 
-#[cfg(feature = "downloads")]
-pub mod package_downloads;
 #[cfg(feature = "fonts")]
 pub mod fonts;
 #[cfg(feature = "packages")]
 pub mod package;
+#[cfg(feature = "downloads")]
+pub mod package_downloads;

crates/typst-kit/src/package.rs

@@ -1,13 +1,13 @@
 //! Download and unpack packages and package indices.
 use std::path::{Path, PathBuf};
 
+use crate::package_downloads::{Downloader, PackageDownloader, Progress};
 use ecow::eco_format;
 use once_cell::sync::OnceCell;
 use typst_library::diag::{PackageError, PackageResult, StrResult};
 use typst_syntax::package::{
     PackageInfo, PackageSpec, PackageVersion, VersionlessPackageSpec,
 };
-use crate::package_downloads::{Downloader, PackageDownloader, Progress};
 
 /// The default packages sub directory within the package and package cache paths.
 pub const DEFAULT_PACKAGES_SUBDIR: &str = "typst/packages";
@@ -97,12 +97,12 @@ impl PackageStorage {
         &self,
         spec: &VersionlessPackageSpec,
     ) -> StrResult<PackageVersion> {
-
         // Same logical flow as per package download. Check package path, then check online.
         // Do not check in the data directory because the latter is not intended for storage
         // of local packages.
         let subdir = format!("{}/{}", spec.namespace, spec.name);
-        let res = self.package_path
+        let res = self
+            .package_path
             .iter()
             .flat_map(|dir| std::fs::read_dir(dir.join(&subdir)).ok())
             .flatten()
@@ -124,7 +124,10 @@ impl PackageStorage {
     }
 
     /// Download the package index. The result of this is cached for efficiency.
-    pub fn download_index(&self, spec: &VersionlessPackageSpec) -> StrResult<&[PackageInfo]> {
+    pub fn download_index(
+        &self,
+        spec: &VersionlessPackageSpec,
+    ) -> StrResult<&[PackageInfo]> {
         self.index
             .get_or_try_init(|| self.downloader.download_index(spec))
             .map(AsRef::as_ref)
@@ -147,8 +150,8 @@ impl PackageStorage {
                 } else {
                     Err(PackageError::NotFound(spec.clone()))
                 }
-            },
+            }
-            val => val
+            val => val,
         }
     }
 }

crates/typst-kit/src/package_downloads/git.rs

@@ -1,16 +1,14 @@
+use crate::package_downloads::{DownloadState, PackageDownloader, Progress};
+use auth_git2::GitAuthenticator;
+use ecow::{eco_format, EcoString};
+use git2::build::RepoBuilder;
+use git2::{FetchOptions, RemoteCallbacks};
 use std::collections::VecDeque;
 use std::fmt::Debug;
 use std::path::Path;
 use std::time::Instant;
-use auth_git2::GitAuthenticator;
-use crate::package_downloads::{
-    DownloadState, PackageDownloader, Progress,
-};
-use ecow::{eco_format, EcoString};
 use typst_library::diag::{PackageError, PackageResult};
 use typst_syntax::package::{PackageInfo, PackageSpec, VersionlessPackageSpec};
-use git2::{FetchOptions, RemoteCallbacks};
-use git2::build::RepoBuilder;
 
 #[derive(Debug)]
 pub struct GitDownloader;
@@ -31,38 +29,41 @@ impl GitDownloader {
 
         eprintln!("{} {} {}", repo, tag, dest.display());
 
-        let state = DownloadState{
+        let state = DownloadState {
             content_len: None,
             total_downloaded: 0,
             bytes_per_second: VecDeque::from(vec![0; 5]),
             start_time: Instant::now(),
         };
 
-
         let auth = GitAuthenticator::default();
-        let git_config = git2::Config::open_default().map_err(|err| {EcoString::from(format!("{:?}", err))})?;
+        let git_config = git2::Config::open_default()
+            .map_err(|err| EcoString::from(format!("{err}")))?;
 
         let mut fetch_options = FetchOptions::new();
         let mut remote_callbacks = RemoteCallbacks::new();
 
         remote_callbacks.credentials(auth.credentials(&git_config));
-        fetch_options
+        fetch_options.remote_callbacks(remote_callbacks);
-            .remote_callbacks(remote_callbacks);
 
         let repo = RepoBuilder::new()
             .fetch_options(fetch_options)
-            .clone(repo, dest).map_err(|err| {EcoString::from(format!("{:?}", err))})?;
+            .clone(repo, dest)
+            .map_err(|err| EcoString::from(format!("{err}")))?;
 
         let (object, reference) = repo
-            .revparse_ext(tag).map_err(|err| {EcoString::from(format!("{:?}", err))})?;
+            .revparse_ext(tag)
-        repo.checkout_tree(&object, None).map_err(|err| {EcoString::from(format!("{:?}", err))})?;
+            .map_err(|err| EcoString::from(format!("{err}")))?;
+        repo.checkout_tree(&object, None)
+            .map_err(|err| EcoString::from(format!("{err}")))?;
 
         match reference {
             // gref is an actual reference like branches or tags
             Some(gref) => repo.set_head(gref.name().unwrap()),
             // this is a commit, not a reference
             None => repo.set_head_detached(object.id()),
-        }.map_err(|err| {EcoString::from(format!("{:?}", err))})?;
+        }
+        .map_err(|err| EcoString::from(format!("{err}")))?;
 
         progress.print_finish(&state);
         Ok(())
@@ -78,33 +79,31 @@ impl GitDownloader {
     /// v<major>.<minor>.<patch>
     ///
     /// For example, the package
-    /// @git:git@github.com:typst/package:0.1.0
+    /// @git:git@github.com:typst/package:0.0
     /// will result in the cloning of the repository git@github.com:typst/package.git
     /// and the checkout and detached head state at tag v0.1.0
     ///
     /// NOTE: no index download is possible.
     fn parse_namespace(ns: &str, name: &str) -> Result<String, EcoString> {
-
         let mut parts = ns.splitn(2, ":");
-        let schema = parts.next().ok_or_else(|| {
+        let schema =
-            eco_format!("expected schema in {}", ns)
+            parts.next().ok_or_else(|| eco_format!("expected schema in {}", ns))?;
-        })?;
+        let repo = parts
-        let repo = parts.next().ok_or_else(|| {
+            .next()
-            eco_format!("invalid package repo {}", ns)
+            .ok_or_else(|| eco_format!("invalid package repo {}", ns))?;
-        })?;
 
         if !schema.eq("git") {
             Err(eco_format!("invalid schema in {}", ns))?
         }
 
-        Ok(format!("{}/{}.git", repo, name))
+        Ok(format!("{repo}/{name}.git"))
     }
 }
 
 impl PackageDownloader for GitDownloader {
     fn download_index(
         &self,
-       _spec: &VersionlessPackageSpec,
+        _spec: &VersionlessPackageSpec,
     ) -> Result<Vec<PackageInfo>, EcoString> {
         Err(eco_format!("Downloading index is not supported for git repositories"))
     }
@@ -115,8 +114,10 @@ impl PackageDownloader for GitDownloader {
         package_dir: &Path,
         progress: &mut dyn Progress,
     ) -> PackageResult<()> {
-        let repo = Self::parse_namespace(spec.namespace.as_str(), spec.name.as_str()).map_err(|x| PackageError::Other(Some(x)))?;
+        let repo = Self::parse_namespace(spec.namespace.as_str(), spec.name.as_str())
+            .map_err(|x| PackageError::Other(Some(x)))?;
         let tag = format!("v{}", spec.version);
-        self.download_with_progress(repo.as_str(), tag.as_str(), package_dir, progress).map_err(|x| PackageError::Other(Some(x)))
+        self.download_with_progress(repo.as_str(), tag.as_str(), package_dir, progress)
+            .map_err(|x| PackageError::Other(Some(x)))
     }
 }

crates/typst-kit/src/package_downloads/http.rs

@@ -13,18 +13,19 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
+use crate::package_downloads::{
+    DownloadState, PackageDownloader, Progress, DEFAULT_NAMESPACE,
+};
 use ecow::{eco_format, EcoString};
 use native_tls::{Certificate, TlsConnector};
 use once_cell::sync::OnceCell;
-use ureq::Response;
 use typst_library::diag::{bail, PackageError, PackageResult};
 use typst_syntax::package::{PackageInfo, PackageSpec, VersionlessPackageSpec};
-use crate::package_downloads::{DownloadState, PackageDownloader, Progress, DEFAULT_NAMESPACE};
+use ureq::Response;
 
 /// The default Typst registry.
 pub const DEFAULT_REGISTRY: &str = "https://packages.typst.org";
 
-
 /// An implementation of [`Progress`] with no-op reporting, i.e., reporting
 /// events are swallowed.
 pub struct ProgressSink;
@@ -43,6 +44,10 @@ pub struct HttpDownloader {
 }
 
 impl HttpDownloader {
+    pub fn default_user_agent() -> String {
+        format!("typst-kit/{}", env!("CARGO_PKG_VERSION"))
+    }
+
     /// Crates a new downloader with the given user agent and no certificate.
     pub fn new(user_agent: impl Into<EcoString>) -> Self {
         Self {
@@ -135,25 +140,24 @@ impl HttpDownloader {
     /// @https:packages.typst.org:preview/package-name>:package-version
     fn parse_namespace(ns: &str) -> Result<(String, String), EcoString> {
         if ns.eq(DEFAULT_NAMESPACE) {
-            return Ok((DEFAULT_REGISTRY.to_string(), DEFAULT_NAMESPACE.to_string()))
+            return Ok((DEFAULT_REGISTRY.to_string(), DEFAULT_NAMESPACE.to_string()));
         }
         let mut parts = ns.splitn(3, ":");
 
-        let schema = parts.next().ok_or_else(|| {
+        let schema =
-            eco_format!("expected schema in {}", ns)
+            parts.next().ok_or_else(|| eco_format!("expected schema in {}", ns))?;
-        })?;
+        let registry = parts
-        let registry = parts.next().ok_or_else(|| {
+            .next()
-            eco_format!("invalid package registry in namespace {}", ns)
+            .ok_or_else(|| eco_format!("invalid package registry in namespace {}", ns))?;
-        })?;
+        let ns = parts
-        let ns = parts.next().ok_or_else(|| {
+            .next()
-            eco_format!("invalid package namespace in {}", ns)
+            .ok_or_else(|| eco_format!("invalid package namespace in {}", ns))?;
-        })?;
 
         if !schema.eq("http") && !schema.eq("https") {
             Err(eco_format!("invalid schema in {}", ns))?
         }
 
-        Ok((format!("{}://{}", schema, registry), ns.to_string()))
+        Ok((format!("{schema}://{registry}"), ns.to_string()))
     }
 }
 
@@ -267,15 +271,17 @@ impl<'p> RemoteReader<'p> {
     }
 }
 
-
 impl PackageDownloader for HttpDownloader {
-    fn download_index(&self, spec: &VersionlessPackageSpec) -> Result<Vec<PackageInfo>, EcoString> {
+    fn download_index(
+        &self,
+        spec: &VersionlessPackageSpec,
+    ) -> Result<Vec<PackageInfo>, EcoString> {
         let (registry, namespace) = Self::parse_namespace(spec.namespace.as_str())?;
         let url = format!("{registry}/{namespace}/index.json");
         match self.perform_download(&url) {
-            Ok(response) => response.into_json().map_err(|err| {
+            Ok(response) => response
-                eco_format!("failed to parse package index: {err}")
+                .into_json()
-            }),
+                .map_err(|err| eco_format!("failed to parse package index: {err}")),
             Err(ureq::Error::Status(404, _)) => {
                 bail!("failed to fetch package index (not found)")
             }
@@ -283,21 +289,23 @@ impl PackageDownloader for HttpDownloader {
         }
     }
 
-    fn download(&self, spec: &PackageSpec, package_dir: &Path, progress: &mut dyn Progress) -> PackageResult<()> {
+    fn download(
-        let (registry, namespace) = Self::parse_namespace(spec.namespace.as_str()).map_err(|x| PackageError::Other(Some(x)))?;
+        &self,
+        spec: &PackageSpec,
+        package_dir: &Path,
+        progress: &mut dyn Progress,
+    ) -> PackageResult<()> {
+        let (registry, namespace) = Self::parse_namespace(spec.namespace.as_str())
+            .map_err(|x| PackageError::Other(Some(x)))?;
 
-        let url = format!(
+        let url =
-            "{}/{}/{}-{}.tar.gz",
+            format!("{}/{}/{}-{}.tar.gz", registry, namespace, spec.name, spec.version);
-            registry, namespace, spec.name, spec.version
-        );
         let data = match self.download_with_progress(&url, progress) {
             Ok(data) => data,
             Err(ureq::Error::Status(404, _)) => {
                 Err(PackageError::NotFound(spec.clone()))?
             }
-            Err(err) => {
+            Err(err) => Err(PackageError::NetworkFailed(Some(eco_format!("{err}"))))?,
-                Err(PackageError::NetworkFailed(Some(eco_format!("{err}"))))?
-            }
         };
 
         let decompressed = flate2::read::GzDecoder::new(data.as_slice());
@@ -306,4 +314,4 @@ impl PackageDownloader for HttpDownloader {
             PackageError::MalformedArchive(Some(eco_format!("{err}")))
         })
     }
-}
+}

crates/typst-kit/src/package_downloads/mod.rs

@@ -1,33 +1,39 @@
+use ecow::{eco_format, EcoString};
 use std::collections::VecDeque;
 use std::fmt::Debug;
 use std::path::{Path, PathBuf};
 use std::time::Instant;
-use ecow::{eco_format, EcoString};
 use typst_library::diag::{PackageError, PackageResult};
 use typst_syntax::package::{PackageInfo, PackageSpec, VersionlessPackageSpec};
-use crate::package_downloads::git::GitDownloader;
 
 /// The public namespace in the default Typst registry.
 pub const DEFAULT_NAMESPACE: &str = "preview";
 
 /*========BEGIN DOWNLOAD METHODS DECLARATION=========*/
 #[cfg(feature = "downloads_http")]
-mod http;
+pub mod http;
 
 #[cfg(feature = "downloads_git")]
 mod git;
 /*========END DOWNLOAD METHODS DECLARATION===========*/
 
 /// Trait abstraction for package a downloader.
-pub trait PackageDownloader : Debug + Sync + Send {
+pub trait PackageDownloader: Debug + Sync + Send {
-
     /// Download the repository index and returns the
     /// list of PackageInfo elements contained in it.
-    fn download_index(&self, spec: &VersionlessPackageSpec) -> Result<Vec<PackageInfo>, EcoString>;
+    fn download_index(
+        &self,
+        spec: &VersionlessPackageSpec,
+    ) -> Result<Vec<PackageInfo>, EcoString>;
 
     /// Download a package from a remote repository/registry
     /// and writes it in the file system cache directory
-    fn download(&self, spec: &PackageSpec, package_dir: &Path, progress: &mut dyn Progress) -> PackageResult<()>;
+    fn download(
+        &self,
+        spec: &PackageSpec,
+        package_dir: &Path,
+        progress: &mut dyn Progress,
+    ) -> PackageResult<()>;
 }
 
 /// The current state of an in progress or finished download.
@@ -58,13 +64,12 @@ pub trait Progress {
 
 /// The downloader object used for downloading packages
 #[derive(Debug)]
-pub struct Downloader{
+pub struct Downloader {
     ///List of all available downloaders which can be instantiated at runtime
     http_downloader: Option<Box<dyn PackageDownloader>>,
     git_downloader: Option<Box<dyn PackageDownloader>>,
 }
 
-
 impl Downloader {
     /// Construct the Downloader object instantiating all the available methods.
     /// The methods can be compile-time selected by features.
@@ -76,31 +81,57 @@ impl Downloader {
     }
 
     /// Creation function for the HTTP(S) download method
-    fn make_http_downloader(cert: Option<PathBuf>) -> Option<Box<dyn PackageDownloader>>{
+    fn make_http_downloader(cert: Option<PathBuf>) -> Option<Box<dyn PackageDownloader>> {
         #[cfg(not(feature = "downloads_http"))]
-        { None }
+        {
+            None
+        }
 
         #[cfg(feature = "downloads_http")]
         {
-            let user_agent = concat!("typst/", env!("CARGO_PKG_VERSION"));
             match cert {
-                Some(cert_path) => Some(Box::new(http::HttpDownloader::with_path(user_agent, cert_path))),
+                Some(cert_path) => Some(Box::new(http::HttpDownloader::with_path(
-                None => Some(Box::new(http::HttpDownloader::new(user_agent))),
+                    http::HttpDownloader::default_user_agent(),
+                    cert_path,
+                ))),
+                None => Some(Box::new(http::HttpDownloader::new(
+                    http::HttpDownloader::default_user_agent(),
+                ))),
             }
         }
     }
 
+    fn get_http_downloader(&self) -> Result<&dyn PackageDownloader, PackageError> {
+        let reference = self.http_downloader.as_ref().ok_or_else(|| {
+            PackageError::Other(Some(EcoString::from(
+                "Http downloader has not been initialized correctly",
+            )))
+        })?;
+        Ok(&**reference)
+    }
+
     /// Creation function for the GIT clone method
-    fn make_git_downloader(_cert: Option<PathBuf>) -> Option<Box<dyn PackageDownloader>>{
+    fn make_git_downloader(_cert: Option<PathBuf>) -> Option<Box<dyn PackageDownloader>> {
         #[cfg(not(feature = "downloads_git"))]
-        { None }
+        {
+            None
+        }
 
         #[cfg(feature = "downloads_git")]
         {
-            Some(Box::new(GitDownloader::new()))
+            Some(Box::new(git::GitDownloader::new()))
         }
     }
 
+    fn get_git_downloader(&self) -> Result<&dyn PackageDownloader, PackageError> {
+        let reference = self.git_downloader.as_ref().ok_or_else(|| {
+            PackageError::Other(Some(EcoString::from(
+                "Http downloader has not been initialized correctly",
+            )))
+        })?;
+        Ok(&**reference)
+    }
+
     /// Returns the correct downloader in function of the package namespace.
     /// The remote location of a package is encoded in its namespace in the form
     /// @<source type>:<source path>
@@ -108,37 +139,43 @@ impl Downloader {
     /// It's the downloader instance's job to parse the source path in any substructure.
     ///
     /// NOTE: Treating @preview as a special case of the https downloader.
-    fn get_downloader(&self, ns: &str) -> Result<&Box<dyn PackageDownloader>, PackageError> {
+    fn get_downloader(&self, ns: &str) -> Result<&dyn PackageDownloader, PackageError> {
-        let download_type = ns.splitn(2, ":").next();
+        let download_type = ns.split(":").next();
 
         match download_type {
             #[cfg(feature = "downloads_http")]
-            Some("http") => self.http_downloader.as_ref().ok_or_else(|| PackageError::Other(Some(EcoString::from("Http downloader has not been initialized correctly")))),
+            Some("http") | Some("https") | Some("preview") => self.get_http_downloader(),
-            #[cfg(feature = "downloads_http")]
-            Some("https") => self.http_downloader.as_ref().ok_or_else(|| PackageError::Other(Some(EcoString::from("Https downloader has not been initialized correctly")))),
-            #[cfg(feature = "downloads_http")]
-            Some("preview") => self.http_downloader.as_ref().ok_or_else(|| PackageError::Other(Some(EcoString::from("Https downloader has not been initialized correctly")))),
 
             #[cfg(feature = "downloads_git")]
-            Some("git") => self.git_downloader.as_ref().ok_or_else(|| PackageError::Other(Some(EcoString::from("Git downloader has not been initialized correctly")))),
+            Some("git") => self.get_git_downloader(),
 
-            Some(dwld) => Err(PackageError::Other(Some(eco_format!("Unknown downloader type: {}", dwld)))),
+            Some(dwld) => Err(PackageError::Other(Some(eco_format!(
-            None => Err(PackageError::Other(Some(EcoString::from("No downloader type specified")))),
+                "Unknown downloader type: {}",
+                dwld
+            )))),
+            None => Err(PackageError::Other(Some(EcoString::from(
+                "No downloader type specified",
+            )))),
         }
     }
 }
 
-
 impl PackageDownloader for Downloader {
-    fn download_index(&self, spec: &VersionlessPackageSpec) -> Result<Vec<PackageInfo>, EcoString> {
+    fn download_index(
+        &self,
+        spec: &VersionlessPackageSpec,
+    ) -> Result<Vec<PackageInfo>, EcoString> {
         let downloader = self.get_downloader(spec.namespace.as_str())?;
         downloader.download_index(spec)
     }
 
-    fn download(&self, spec: &PackageSpec, package_dir: &Path, progress: &mut dyn Progress) -> PackageResult<()> {
+    fn download(
+        &self,
+        spec: &PackageSpec,
+        package_dir: &Path,
+        progress: &mut dyn Progress,
+    ) -> PackageResult<()> {
         let downloader = self.get_downloader(spec.namespace.as_str())?;
         downloader.download(spec, package_dir, progress)
     }
 }
-
-

crates/typst-syntax/src/package.rs

@@ -264,17 +264,18 @@ impl Display for VersionlessPackageSpec {
 }
 
 fn is_namespace_valid(namespace: &str) -> bool {
-    if is_ident(namespace){
+    if is_ident(namespace) {
         //standard namespace
-        return true
+        return true;
     }
 
     //if not ident, the namespace should be formed as @<package_remote_type>:<package_path>
     let mut tokenized = namespace.splitn(2, ":");
 
     //package type
-    if tokenized.next().is_none_or(|x| !is_ident(x)) {
+    let package_remote_type = tokenized.next();
-        return false
+    if package_remote_type.is_none() || !is_ident(package_remote_type.unwrap()) {
+        return false;
     }
 
     //the package_path parsing is left to the downloader implementation