From c216652a5cdce736dab034b63f2d3a11639581d7 Mon Sep 17 00:00:00 2001 From: Jamy Golden Date: Sun, 6 Oct 2024 13:56:49 +0200 Subject: [PATCH] Use `impl AsRef` in place of `&Path` for more flexibility --- CHANGELOG.md | 7 +++ tinted-builder-rust/src/operations/build.rs | 45 ++++++++++------ .../src/operations/build/utils.rs | 20 ++++--- tinted-builder-rust/src/operations/sync.rs | 52 ++++++++++++------- tinted-builder-rust/tests/test_utils.rs | 11 ++-- 5 files changed, 85 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3224bf6..a9b209c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +### Changed + +- Change `get_scheme_files` and `SchemeFile::new` type arguments from + `&Path` to `impl AsRef` to allow for more flexibility + ## [0.12.1] - 2024-10-06 ### Fixed diff --git a/tinted-builder-rust/src/operations/build.rs b/tinted-builder-rust/src/operations/build.rs index 8aebe49..732e012 100644 --- a/tinted-builder-rust/src/operations/build.rs +++ b/tinted-builder-rust/src/operations/build.rs @@ -21,10 +21,12 @@ const REPO_NAME: &str = env!("CARGO_PKG_NAME"); /// /// # Arguments /// -/// * `theme_template_path` - A reference to a `Path` representing the path to the theme template -/// directory or file. * `user_schemes_path` - A reference to a `Path` representing the directory -/// where user schemes are stored. * `is_quiet` - A boolean flag that, when set to `true`, -/// suppresses most of the output, making the build process quieter. +/// * `theme_template_path` - A `impl AsRef` representing the path to the theme template +/// directory or file. +/// * `user_schemes_path` - A `impl AsRef` representing the directory where user schemes are +/// stored. +/// * `is_quiet` - A boolean flag that, when set to `true`, suppresses most of the output, +/// making the build process quieter. /// /// # Returns /// @@ -50,8 +52,12 @@ const REPO_NAME: &str = env!("CARGO_PKG_NAME"); /// /// The function will read the configuration from the specified paths and generate the /// corresponding themes. -pub fn build(theme_template_path: &Path, user_schemes_path: &Path, is_quiet: bool) -> Result<()> { - if !user_schemes_path.exists() { +pub fn build( + theme_template_path: impl AsRef, + user_schemes_path: impl AsRef, + is_quiet: bool, +) -> Result<()> { + if !user_schemes_path.as_ref().exists() { return Err(anyhow!( "Schemes don't exist locally. First run `{} sync` and try again", REPO_NAME @@ -59,10 +65,14 @@ pub fn build(theme_template_path: &Path, user_schemes_path: &Path, is_quiet: boo } let template_config_path = { - if theme_template_path.join("templates/config.yml").is_file() { - theme_template_path.join("templates/config.yml") + if theme_template_path + .as_ref() + .join("templates/config.yml") + .is_file() + { + theme_template_path.as_ref().join("templates/config.yml") } else { - theme_template_path.join("templates/config.yaml") + theme_template_path.as_ref().join("templates/config.yaml") } }; if !template_config_path.exists() || !template_config_path.is_file() { @@ -114,7 +124,7 @@ pub fn build(theme_template_path: &Path, user_schemes_path: &Path, is_quiet: boo generate_themes_for_config( template_item_config_name, template_item_config_value, - theme_template_path, + &theme_template_path, &template_item_scheme_files, is_quiet, )?; @@ -126,7 +136,7 @@ pub fn build(theme_template_path: &Path, user_schemes_path: &Path, is_quiet: boo fn generate_themes_for_config( config_name: &str, config_value: &TemplateConfig, - theme_template_path: &Path, + theme_template_path: impl AsRef, scheme_files: &Vec<(PathBuf, Scheme)>, is_quiet: bool, ) -> Result<()> { @@ -173,8 +183,9 @@ fn generate_themes_for_config( "Config file is missing \"filepath\" or \"extension\" and \"output\" properties" )), }?; - let mustache_template_path = - theme_template_path.join(format!("templates/{}.mustache", config_name)); + let mustache_template_path = theme_template_path + .as_ref() + .join(format!("templates/{}.mustache", config_name)); let supported_systems = &config_value .supported_systems .clone() @@ -202,7 +213,7 @@ fn generate_themes_for_config( .replace("{{ scheme-system }}", &scheme_system.to_string()) .replace("{{scheme-system}}", &scheme_system.to_string()); - let parsed_filename = parse_filename(theme_template_path, &filepath)?; + let parsed_filename = parse_filename(&theme_template_path, &filepath)?; if !parsed_filename.directory.exists() { create_dir_all(&parsed_filename.directory)? } @@ -224,7 +235,7 @@ fn generate_themes_for_config( .collect::>() .join(", "), config_name, - theme_template_path.join(filename).display(), + theme_template_path.as_ref().join(filename).display(), ); } @@ -244,7 +255,7 @@ fn generate_themes_for_config( /// /// * `template_content` - A reference to a string slice containing the template's content. /// * `output_dir` - A reference to a `PathBuf` representing the directory where the output file will be written. -/// * `scheme_path` - A reference to a `Path` representing the file path to the scheme file. +/// * `scheme_path` - A `impl AsRef` representing the file path to the scheme file. /// * `system` - The `SchemeSystem` that the scheme file should match. /// * `explicit_extension` - A string slice representing the file extension for the generated theme /// file. The parameter is named "explict" extension because it includes the "dot" or lack thereof @@ -273,7 +284,7 @@ fn generate_themes_for_config( fn generate_theme( template_content: &str, parsed_filename: ParsedFilename, - scheme_path: &Path, + scheme_path: impl AsRef, system: SchemeSystem, ) -> Result<()> { let scheme_file_type = SchemeFile::new(scheme_path)?; diff --git a/tinted-builder-rust/src/operations/build/utils.rs b/tinted-builder-rust/src/operations/build/utils.rs index aa16ee3..c8fc85b 100644 --- a/tinted-builder-rust/src/operations/build/utils.rs +++ b/tinted-builder-rust/src/operations/build/utils.rs @@ -12,16 +12,17 @@ pub enum SchemeFile { } impl SchemeFile { - pub fn new(path: &Path) -> Result { + pub fn new(path: impl AsRef) -> Result { let extension = path + .as_ref() .extension() .unwrap_or_default() .to_str() .unwrap_or_default(); match extension { - "yaml" => Ok(Self::Yaml(path.to_path_buf())), - "yml" => Ok(Self::Yml(path.to_path_buf())), + "yaml" => Ok(Self::Yaml(path.as_ref().to_path_buf())), + "yml" => Ok(Self::Yml(path.as_ref().to_path_buf())), _ => Err(anyhow!("Invalid file extension: {}", extension.to_string())), } } @@ -122,10 +123,10 @@ impl ParsedFilename { /// * If the directory cannot be read. /// * If there is an issue accessing the contents of the directory. /// * If there is an issue creating a `SchemeFile` from a file path. -pub fn get_scheme_files(dirpath: &Path, is_recursive: bool) -> Result> { +pub fn get_scheme_files(dirpath: impl AsRef, is_recursive: bool) -> Result> { let mut scheme_paths: Vec = vec![]; - for item in dirpath.read_dir()? { + for item in dirpath.as_ref().read_dir()? { let file_path = item?.path(); let file_stem = file_path .file_stem() @@ -170,7 +171,10 @@ pub fn get_scheme_files(dirpath: &Path, is_recursive: bool) -> Result Result { +pub(crate) fn parse_filename( + template_path: impl AsRef, + filepath: &str, +) -> Result { let re = Regex::new(r"^(?P.*/)?(?P[^/\.]+)(?:\.(?P[^/]+))?$") .unwrap(); @@ -178,8 +182,8 @@ pub(crate) fn parse_filename(template_path: &Path, filepath: &str) -> Result` representing the directory where the schemes +/// repository is or should be located. +/// * `is_quiet` - A boolean flag that, when set to `true`, /// suppresses most of the output, making the operation quieter. /// /// # Returns @@ -45,9 +46,9 @@ const SCHEMES_URL: &str = "https://github.com/tinted-theming/schemes"; /// /// The function will ensure that the schemes repository is up-to-date, either by pulling the /// latest changes or by cloning the repository if it does not already exist. -pub(crate) fn sync(schemes_path: &Path, is_quiet: bool) -> Result<()> { - if schemes_path.is_dir() { - let is_diff = git_diff(schemes_path)?; +pub(crate) fn sync(schemes_path: impl AsRef, is_quiet: bool) -> Result<()> { + if schemes_path.as_ref().is_dir() { + let is_diff = git_diff(&schemes_path)?; if !is_diff { git_pull(schemes_path, is_quiet).with_context(|| { @@ -71,18 +72,18 @@ pub(crate) fn sync(schemes_path: &Path, is_quiet: bool) -> Result<()> { Ok(()) } -fn git_clone(repo_url: &str, target_dir: &Path, is_quiet: bool) -> Result<()> { - if target_dir.exists() { +fn git_clone(repo_url: &str, target_dir: impl AsRef, is_quiet: bool) -> Result<()> { + if target_dir.as_ref().exists() { return Err(anyhow!( "Error cloning {}. Target directory '{}' already exists", repo_url, - target_dir.display() + target_dir.as_ref().display() )); } let mut cmd = Command::new("git"); - cmd.arg("clone").arg(repo_url).arg(target_dir); + cmd.arg("clone").arg(repo_url).arg(target_dir.as_ref()); if is_quiet { cmd.stdout(Stdio::null()).stderr(Stdio::null()); @@ -94,40 +95,51 @@ fn git_clone(repo_url: &str, target_dir: &Path, is_quiet: bool) -> Result<()> { Ok(()) } -fn git_pull(repo_path: &Path, is_quiet: bool) -> Result<()> { - if !repo_path.is_dir() { +fn git_pull(repo_path: impl AsRef, is_quiet: bool) -> Result<()> { + if !repo_path.as_ref().is_dir() { return Err(anyhow!( "Error with git pull. {} is not a directory", - repo_path.display() + repo_path.as_ref().display() )); } let mut cmd = Command::new("git"); - cmd.arg("pull").current_dir(repo_path); + cmd.arg("pull").current_dir(&repo_path); if is_quiet { cmd.stdout(Stdio::null()).stderr(Stdio::null()); } - let status = cmd - .status() - .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; + let status = cmd.status().with_context(|| { + format!( + "Failed to execute process in {}", + repo_path.as_ref().display() + ) + })?; if status.success() { Ok(()) } else { - Err(anyhow!("Error wth git pull in {}", repo_path.display())) + Err(anyhow!( + "Error wth git pull in {}", + repo_path.as_ref().display() + )) } } -fn git_diff(target_dir: &Path) -> Result { +fn git_diff(target_dir: impl AsRef) -> Result { let output = Command::new("git") .arg("status") .arg("--porcelain") - .current_dir(target_dir) + .current_dir(&target_dir) .output() - .with_context(|| format!("Failed to execute process in {}", target_dir.display()))?; + .with_context(|| { + format!( + "Failed to execute process in {}", + target_dir.as_ref().display() + ) + })?; let stdout = str::from_utf8(&output.stdout).expect("Not valid UTF-8"); if stdout.is_empty() { diff --git a/tinted-builder-rust/tests/test_utils.rs b/tinted-builder-rust/tests/test_utils.rs index 524a776..42bf088 100644 --- a/tinted-builder-rust/tests/test_utils.rs +++ b/tinted-builder-rust/tests/test_utils.rs @@ -31,13 +31,14 @@ pub fn run_command(command_vec: Vec) -> Result<(String, String), Box Result<()> { - if path.exists() { - remove_file(path).with_context(|| format!("Unable to remove file: {}", path.display()))?; +pub fn write_to_file(path: impl AsRef, contents: &str) -> Result<()> { + if path.as_ref().exists() { + remove_file(&path) + .with_context(|| format!("Unable to remove file: {}", path.as_ref().display()))?; } - let mut file = - File::create(path).with_context(|| format!("Unable to create file: {}", path.display()))?; + let mut file = File::create(&path) + .with_context(|| format!("Unable to create file: {}", path.as_ref().display()))?; file.write_all(contents.as_bytes())?;