From 7d673bb0fc52f2a7ac88e5ff36f654149223ba50 Mon Sep 17 00:00:00 2001 From: Adaline Simonian Date: Fri, 17 Jan 2025 18:46:55 -0800 Subject: [PATCH] fix: don't pollute stdout when running godot Fixes issues with scripts expecting to operate off of Godot's output. --- .github/workflows/main.yml | 14 ++++++ .github/workflows/pr.yml | 14 ++++++ .github/workflows/release.yml | 12 +++++ .vscode/settings.json | 23 ++++++++- scripts/find-missing-i18n.ps1 | 2 +- scripts/find-missing-i18n.sh | 2 +- src/download_utils.rs | 4 +- src/godot_manager.rs | 66 ++++++++++++------------- src/i18n.rs | 42 +++++++++++++--- src/main.rs | 85 +++++++++++++++++++-------------- src/project_version_detector.rs | 16 ++----- src/version_utils.rs | 30 +++--------- src/zip_utils.rs | 4 +- 13 files changed, 197 insertions(+), 117 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8f2fb21..b938f51 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,6 +7,10 @@ on: paths-ignore: - "gh-pages/**" +concurrency: + group: main + cancel-in-progress: true + jobs: build-and-test: runs-on: ${{ matrix.os }} @@ -61,6 +65,16 @@ jobs: shell: bash run: scripts/find-missing-i18n.sh + - name: Check formatting + run: cargo fmt -- --check + + - name: Check clippy + run: | + cargo clippy --all-targets --all-features -- \ + -D clippy::suspicious -D clippy::style -D clippy::complexity \ + -D clippy::perf -D clippy::dbg_macro -D clippy::todo \ + -D clippy::unimplemented -D warnings + - name: Test if: always() && (matrix.os == 'ubuntu-latest' && matrix.target == 'x86_64-unknown-linux-gnu') || (matrix.os == 'windows-latest' && matrix.target == 'x86_64-pc-windows-msvc') || (matrix.os == 'macos-latest' && matrix.target == 'x86_64-apple-darwin') run: cargo test --target ${{ matrix.target }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 727463b..7ee3cad 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -7,6 +7,10 @@ on: paths-ignore: - "gh-pages/**" +concurrency: + group: pr-${{ github.event.number }} + cancel-in-progress: true + jobs: build-and-test: runs-on: ${{ matrix.os }} @@ -61,6 +65,16 @@ jobs: shell: bash run: scripts/find-missing-i18n.sh + - name: Check formatting + run: cargo fmt -- --check + + - name: Check clippy + run: | + cargo clippy --all-targets --all-features -- \ + -D clippy::suspicious -D clippy::style -D clippy::complexity \ + -D clippy::perf -D clippy::dbg_macro -D clippy::todo \ + -D clippy::unimplemented -D warnings + - name: Test if: always() && (matrix.os == 'ubuntu-latest' && matrix.target == 'x86_64-unknown-linux-gnu') || (matrix.os == 'windows-latest' && matrix.target == 'x86_64-pc-windows-msvc') || (matrix.os == 'macos-latest' && matrix.target == 'x86_64-apple-darwin') run: cargo test --target ${{ matrix.target }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index be028ec..13d5cf9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,6 +68,18 @@ jobs: run: scripts/find-missing-i18n.sh continue-on-error: true + - name: Check formatting + run: cargo fmt -- --check + continue-on-error: true + + - name: Check clippy + run: | + cargo clippy --all-targets --all-features -- \ + -D clippy::suspicious -D clippy::style -D clippy::complexity \ + -D clippy::perf -D clippy::dbg_macro -D clippy::todo \ + -D clippy::unimplemented -D warnings + continue-on-error: true + - name: Test if: (matrix.os == 'ubuntu-latest' && matrix.target == 'x86_64-unknown-linux-gnu') || (matrix.os == 'windows-latest' && matrix.target == 'x86_64-pc-windows-msvc') || (matrix.os == 'macos-latest' && matrix.target == 'x86_64-apple-darwin') run: cargo test --target ${{ matrix.target }} diff --git a/.vscode/settings.json b/.vscode/settings.json index 7958498..d7cfb70 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -15,5 +15,26 @@ }, "search.exclude": { "**/target": true - } + }, + "rust-analyzer.check.command": "clippy", + "rust-analyzer.check.extraArgs": [ + "--no-default-features", + "--", + "-D", + "clippy::suspicious", + "-D", + "clippy::style", + "-D", + "clippy::complexity", + "-D", + "clippy::perf", + "-D", + "clippy::dbg_macro", + "-D", + "clippy::todo", + "-D", + "clippy::unimplemented", + "-D", + "warnings" + ] } diff --git a/scripts/find-missing-i18n.ps1 b/scripts/find-missing-i18n.ps1 index 8ee0cc2..6b16926 100644 --- a/scripts/find-missing-i18n.ps1 +++ b/scripts/find-missing-i18n.ps1 @@ -10,7 +10,7 @@ $rsFiles = Get-ChildItem -Path "$ScriptDir\..\src" -Recurse -Filter *.rs $keys = @() # Define the regex pattern with single-line and multi-line options -$pattern = '(?:i18n\.t(?:_args)?(?:_w)?\s*\(\s*|println_i18n!\s*\(\s*[^,\s]+,\s*)"([^"\\]*(?:\\.[^"\\]*)*)"' +$pattern = '(?:i18n\.t(?:_args)?(?:_w)?\s*\(\s*|[xe]?println_i18n!\s*\(\s*[^,\s]+,\s*)"([^"\\]*(?:\\.[^"\\]*)*)"' foreach ($file in $rsFiles) { # Read the entire file content as a single string diff --git a/scripts/find-missing-i18n.sh b/scripts/find-missing-i18n.sh index 5293124..757ee46 100755 --- a/scripts/find-missing-i18n.sh +++ b/scripts/find-missing-i18n.sh @@ -7,7 +7,7 @@ SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck disable=SC2016 strings=$(find "$SCRIPT_DIR/../src" -type f -name '*.rs' -print0 | \ xargs -0 perl -0777 -ne ' - while (/(?:i18n\.t(?:_args)?(?:_w)?\s*\(\s*|println_i18n!\s*\(\s*[^,\s]+,\s*)"([^"\\]*(?:\\.[^"\\]*)*)"/g) { + while (/(?:i18n\.t(?:_args)?(?:_w)?\s*\(\s*|[xe]?println_i18n!\s*\(\s*[^,\s]+,\s*)"([^"\\]*(?:\\.[^"\\]*)*)"/g) { print "$1\n"; } ' | sort | uniq) diff --git a/src/download_utils.rs b/src/download_utils.rs index 0ae8080..3613e09 100644 --- a/src/download_utils.rs +++ b/src/download_utils.rs @@ -8,11 +8,11 @@ use std::{ use indicatif::{ProgressBar, ProgressStyle}; -use crate::{i18n::I18n, println_i18n}; +use crate::{eprintln_i18n, i18n::I18n}; pub fn download_file(url: &str, dest: &Path, i18n: &I18n) -> Result<()> { // Print downloading URL message - println_i18n!(i18n, "operation-downloading-url", [("url", url)]); + eprintln_i18n!(i18n, "operation-downloading-url", [("url", url)]); let response = reqwest::blocking::get(url)?; diff --git a/src/godot_manager.rs b/src/godot_manager.rs index 640f933..afed1f8 100644 --- a/src/godot_manager.rs +++ b/src/godot_manager.rs @@ -13,10 +13,10 @@ use std::path::{Path, PathBuf}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use crate::download_utils::download_file; -use crate::println_i18n; use crate::version_utils; use crate::version_utils::GodotVersionDeterminate; use crate::zip_utils; +use crate::{eprintln_i18n, println_i18n}; use crate::{i18n, project_version_detector}; use version_utils::{GodotVersion, GodotVersionDeterminateVecExt}; @@ -65,6 +65,7 @@ pub struct GodotManager<'a> { i18n: &'a I18n, } +#[allow(clippy::unimplemented)] fn get_archive_name(version: &GodotVersionDeterminate, i18n: &I18n) -> String { let is_csharp = version.is_csharp == Some(true); let tag = version.to_remote_str(); @@ -151,8 +152,8 @@ fn verify_sha512( ); let sum_file = file_path.with_extension("SHA512-SUMS"); - if let Err(_) = download_file(&sha_url, &sum_file, &i18n) { - println_i18n!(i18n, "warning-sha-sums-missing"); + if download_file(&sha_url, &sum_file, i18n).is_err() { + eprintln_i18n!(i18n, "warning-sha-sums-missing"); return Ok(()); } @@ -238,7 +239,7 @@ fn find_godot_executable(version_dir: &Path, console: bool) -> Result Result Result GodotManager<'a> { if std::env::current_exe().ok() != Some(exe_path.clone()) { if let Err(err) = copy_over_binary_if_different(&gdvm_exe_source, &exe_path) { - println_i18n!( + eprintln_i18n!( self.i18n, "error-ensure-godot-binaries-failed", [ @@ -380,8 +381,8 @@ impl<'a> GodotManager<'a> { if version_path.exists() { if force { - self.remove(&gv)?; - println_i18n!( + self.remove(gv)?; + eprintln_i18n!( self.i18n, "force-reinstalling-version", [("version", gv.to_display_str())] @@ -392,7 +393,7 @@ impl<'a> GodotManager<'a> { } if !gv.is_stable() { - println_i18n!( + eprintln_i18n!( self.i18n, "warning-prerelease", [("branch", &gv.release_type)] @@ -402,15 +403,15 @@ impl<'a> GodotManager<'a> { fs::create_dir_all(&self.cache_path)?; let release_tag = gv.to_remote_str(); - let archive_name = get_archive_name(&gv, self.i18n); + let archive_name = get_archive_name(gv, self.i18n); let download_url = get_download_url(&release_tag, &archive_name); let cache_zip_path = self.cache_path.join(&archive_name); if !redownload && cache_zip_path.exists() { - println_i18n!(self.i18n, "using-cached-zip"); + eprintln_i18n!(self.i18n, "using-cached-zip"); } else { if redownload && cache_zip_path.exists() { - println_i18n!( + eprintln_i18n!( self.i18n, "force-redownload", [("version", gv.to_display_str())] @@ -422,12 +423,12 @@ impl<'a> GodotManager<'a> { .join(format!("{}.zip", gv.to_install_str())); // Download the archive - download_file(&download_url, &tmp_file, &self.i18n)?; - verify_sha512(&tmp_file, &archive_name, &release_tag, &self.i18n)?; + download_file(&download_url, &tmp_file, self.i18n)?; + verify_sha512(&tmp_file, &archive_name, &release_tag, self.i18n)?; // Move the verified zip to cache_dir fs::rename(&tmp_file, &cache_zip_path)?; - println_i18n!(self.i18n, "cached-zip-stored"); + eprintln_i18n!(self.i18n, "cached-zip-stored"); } fs::create_dir_all(&version_path)?; @@ -445,9 +446,8 @@ impl<'a> GodotManager<'a> { let entry = entry?; if entry.file_type()?.is_dir() { let name = entry.file_name().to_string_lossy().to_string(); - match GodotVersion::from_install_str(&name) { - Ok(gv) => versions.push(gv.to_determinate()), - Err(_) => (), // Ignore invalid version directories + if let Ok(gv) = GodotVersion::from_install_str(&name) { + versions.push(gv.to_determinate()) } } } @@ -478,7 +478,7 @@ impl<'a> GodotManager<'a> { &self, gv: &GodotVersionDeterminate, console: bool, - godot_args: &Vec, + godot_args: &[String], ) -> Result<()> { let version_dir = self.install_path.join(gv.to_install_str()); if !version_dir.exists() { @@ -525,7 +525,7 @@ impl<'a> GodotManager<'a> { // "No main scene set" message. let current_dir = std::env::current_dir()?; let project_file = current_dir.join("project.godot"); - let mut godot_args = godot_args.clone(); + let mut godot_args = godot_args.to_vec(); if project_file.exists() && godot_args.iter().all(|arg| !Path::new(arg).exists()) { godot_args.push(project_file.to_string_lossy().to_string()); } @@ -857,7 +857,7 @@ impl<'a> GodotManager<'a> { let target_dir = self.install_path.join(version_str); // Make sure bin directory exists - fs::create_dir_all(&symlink_dir.parent().unwrap())?; + fs::create_dir_all(symlink_dir.parent().unwrap())?; if symlink_dir.exists() { fs::remove_dir_all(&symlink_dir)?; @@ -923,7 +923,7 @@ impl<'a> GodotManager<'a> { pub fn determine_version(&self) -> Option { let current_dir = std::env::current_dir().ok()?; - return project_version_detector::detect_godot_version_in_path(&self.i18n, current_dir); + project_version_detector::detect_godot_version_in_path(self.i18n, current_dir) } /// Pin a version to .gdvmrc in the current directory @@ -954,7 +954,7 @@ impl<'a> GodotManager<'a> { // Check if version is installed, if not, install if !self.is_version_installed(&actual_version)? { - println_i18n!( + eprintln_i18n!( self.i18n, "auto-installing-version", [("version", &actual_version.to_display_str())] @@ -1023,10 +1023,10 @@ impl<'a> GodotManager<'a> { progress.finish_and_clear(); if let Some(new_version) = new_version { - print!("\x1b[1;32m"); // Bold and green - println_i18n!(self.i18n, "upgrade-available", [("version", &new_version)]); - print!("\x1b[0m"); // Reset - println!(); + eprint!("\x1b[1;32m"); // Bold and green + eprintln_i18n!(self.i18n, "upgrade-available", [("version", &new_version)]); + eprint!("\x1b[0m"); // Reset + eprintln!(); self.save_gdvm_cache(&GdvmCache { last_update_check: now, @@ -1042,14 +1042,14 @@ impl<'a> GodotManager<'a> { if let Ok(new_version) = Version::parse(new_version.trim_start_matches('v')) { let current_version = Version::parse(env!("CARGO_PKG_VERSION"))?; if new_version > current_version { - print!("\x1b[1;32m"); // Bold and green - println_i18n!( + eprint!("\x1b[1;32m"); // Bold and green + eprintln_i18n!( self.i18n, "upgrade-available", [("version", new_version.to_string())] ); - print!("\x1b[0m"); // Reset - println!(); + eprint!("\x1b[0m"); // Reset + eprintln!(); } else { self.save_gdvm_cache(&GdvmCache { last_update_check: now, @@ -1121,7 +1121,7 @@ impl<'a> GodotManager<'a> { // Download the new binary if let Err(err) = download_file(&bin_url, &out_file, self.i18n) { - println_i18n!(self.i18n, "upgrade-download-failed"); + eprintln_i18n!(self.i18n, "upgrade-download-failed"); return Err(err); } diff --git a/src/i18n.rs b/src/i18n.rs index eaebc8f..dd82c4d 100644 --- a/src/i18n.rs +++ b/src/i18n.rs @@ -167,10 +167,9 @@ impl I18n { } #[macro_export] -macro_rules! println_i18n { - // With arguments - ($i18n:expr, $key:expr, [$( ($arg_key:expr, $arg_val:expr) ),*]) => { - println!( +macro_rules! i18n_print { + ($print_fn:ident, $i18n:expr, $key:expr, [$( ($arg_key:expr, $arg_val:expr) ),*]) => { + $print_fn!( "{}", $i18n.t_args_w( $key, @@ -180,8 +179,39 @@ macro_rules! println_i18n { ).as_str() ) }; - // Without arguments + ($print_fn:ident, $i18n:expr, $key:expr) => { + $print_fn!("{}", $i18n.t_w($key).as_str()) + }; +} + +#[macro_export] +macro_rules! println_i18n { + ($i18n:expr, $key:expr, [$( ($arg_key:expr, $arg_val:expr) ),*]) => { + $crate::i18n_print!(println, $i18n, $key, [$( ($arg_key, $arg_val) ),*]) + }; + ($i18n:expr, $key:expr) => { + $crate::i18n_print!(println, $i18n, $key) + }; +} + +#[macro_export] +macro_rules! eprintln_i18n { + ($i18n:expr, $key:expr, [$( ($arg_key:expr, $arg_val:expr) ),*]) => { + $crate::i18n_print!(eprintln, $i18n, $key, [$( ($arg_key, $arg_val) ),*]) + }; ($i18n:expr, $key:expr) => { - println!("{}", $i18n.t_w($key).as_str()) + $crate::i18n_print!(eprintln, $i18n, $key) + }; +} + +/// Prints to stdout if the result of an operation is true, otherwise prints to stderr +#[macro_export] +macro_rules! xprintln_i18n { + ($result:expr, $i18n:expr, $success_key:expr, $failure_key:expr) => { + if $result { + $crate::println_i18n!($i18n, $success_key); + } else { + $crate::eprintln_i18n!($i18n, $failure_key); + } }; } diff --git a/src/main.rs b/src/main.rs index 216d2ca..92ebcd7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -56,16 +56,16 @@ fn main() -> Result<()> { } } - if let Err(err) = sub_run_inner( - &i18n, - &GodotManager::new(&i18n)?, - None, - false, - false, - console_mode, - &args, - false, - ) { + if let Err(err) = sub_run_inner(RunConfig { + i18n: &i18n, + manager: &GodotManager::new(&i18n)?, + version_input: None, + csharp_given: false, + csharp_flag: false, + console: console_mode, + raw_args: &args, + force_on_mismatch: false, + }) { eprintln!("{}", err); // Wait for 5 seconds before exiting on Windows to allow the user to read the error. @@ -298,7 +298,7 @@ fn sub_install(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Res let force_reinstall = matches.get_flag("force"); let redownload = matches.get_flag("redownload"); - let requested_version = GodotVersion::from_match_str(&version_input)?; + let requested_version = GodotVersion::from_match_str(version_input)?; let mut gv = manager .resolve_available_version(&requested_version, false) .ok_or_else(|| anyhow!(i18n.t("error-version-not-found")))?; @@ -366,36 +366,51 @@ fn sub_run(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Result< let console = matches.get_flag("console"); let force_on_mismatch = matches.get_flag("force"); - sub_run_inner( + sub_run_inner(RunConfig { i18n, manager, version_input, csharp_given, csharp_flag, console, - &raw_args, + raw_args: &raw_args, force_on_mismatch, - ) + }) } -fn sub_run_inner( - i18n: &I18n, - manager: &GodotManager, - version_input: Option<&String>, +/// Configuration for the `sub_run_inner` function +struct RunConfig<'a> { + i18n: &'a I18n, + manager: &'a GodotManager<'a>, + version_input: Option<&'a String>, csharp_given: bool, csharp_flag: bool, console: bool, - raw_args: &Vec, + raw_args: &'a Vec, force_on_mismatch: bool, -) -> Result<()> { +} + +/// Run the Godot executable +fn sub_run_inner(config: RunConfig) -> Result<()> { + let RunConfig { + i18n, + manager, + version_input, + csharp_given, + csharp_flag, + console, + raw_args, + force_on_mismatch, + } = config; + let resolved_version = if let Some(v) = version_input { - let mut requested_version = GodotVersion::from_match_str(&v)?; + let mut requested_version = GodotVersion::from_match_str(v)?; requested_version.is_csharp = Some(csharp_flag); if warn_project_version_mismatch(i18n, manager, &requested_version, false) { if force_on_mismatch { - println_i18n!( + eprintln_i18n!( i18n, "warning-project-version-mismatch-force", [ @@ -415,7 +430,7 @@ fn sub_run_inner( } else if let Some(pinned) = manager.get_pinned_version() { if warn_project_version_mismatch(i18n, manager, &pinned, true) { if force_on_mismatch { - println_i18n!( + eprintln_i18n!( i18n, "warning-project-version-mismatch-force", [ @@ -433,7 +448,7 @@ fn sub_run_inner( manager.auto_install_version(&pinned)? } else if let Some(project_version) = manager.determine_version() { - println_i18n!( + eprintln_i18n!( i18n, "warning-using-project-version", [("version", project_version.to_display_str())] @@ -449,13 +464,13 @@ fn sub_run_inner( return Err(anyhow!(i18n.t("no-default-set"))); }; - println_i18n!( + eprintln_i18n!( i18n, "running-version", [("version", &resolved_version.to_display_str())] ); - manager.run(&resolved_version, console, &raw_args)?; + manager.run(&resolved_version, console, raw_args)?; Ok(()) } @@ -478,7 +493,7 @@ fn warn_project_version_mismatch( // If the project version is C#, the pinned version must also be C#, and vice versa || project_version.is_csharp.unwrap_or(false) != requested.is_csharp.unwrap_or(false) { - println_i18n!( + eprintln_i18n!( i18n, "warning-project-version-mismatch", [ @@ -487,7 +502,7 @@ fn warn_project_version_mismatch( ("pinned", is_pin as i32) ] ); - println!(); + eprintln!(); return true; } @@ -500,7 +515,7 @@ fn warn_project_version_mismatch( fn sub_remove(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Result<()> { let version_input = matches.get_one::("version").unwrap(); let csharp = matches.get_flag("csharp"); - let mut requested_version = GodotVersion::from_match_str(&version_input)?; + let mut requested_version = GodotVersion::from_match_str(version_input)?; requested_version.is_csharp = Some(csharp); @@ -508,7 +523,7 @@ fn sub_remove(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Resu match resolved_versions.len() { 0 => { - println_i18n!(i18n, "error-version-not-found"); + eprintln_i18n!(i18n, "error-version-not-found"); } 1 => { let gv = &resolved_versions[0]; @@ -525,11 +540,11 @@ fn sub_remove(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Resu return Ok(()); } } - manager.remove(&gv)?; + manager.remove(gv)?; println_i18n!(i18n, "removed-version", [("version", gv.to_display_str())]); } _ => { - println_i18n!(i18n, "error-multiple-versions-found"); + eprintln_i18n!(i18n, "error-multiple-versions-found"); for v in resolved_versions { println!("- {}", v.to_display_str()); } @@ -598,7 +613,7 @@ fn sub_use(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Result< return Ok(()); } - let mut requested_version = GodotVersion::from_match_str(&version_input)?; + let mut requested_version = GodotVersion::from_match_str(version_input)?; requested_version.is_csharp = Some(csharp); @@ -623,7 +638,7 @@ fn sub_upgrade(manager: &GodotManager) -> Result<()> { fn sub_pin(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Result<()> { let version_str = matches.get_one::("version").unwrap(); let csharp = matches.get_flag("csharp"); - let mut version = GodotVersion::from_match_str(&version_str)?; + let mut version = GodotVersion::from_match_str(version_str)?; version.is_csharp = Some(csharp); @@ -637,7 +652,7 @@ fn sub_pin(i18n: &I18n, manager: &GodotManager, matches: &ArgMatches) -> Result< "pinned-success", [("version", &resolved_version.to_display_str())] ), - Err(_) => println_i18n!( + Err(_) => eprintln_i18n!( i18n, "error-pin-version-not-found", [("version", &resolved_version.to_display_str())] diff --git a/src/project_version_detector.rs b/src/project_version_detector.rs index 7534ba3..177aa11 100644 --- a/src/project_version_detector.rs +++ b/src/project_version_detector.rs @@ -2,8 +2,8 @@ use std::ffi::OsStr; use std::fs; use std::path::{Path, PathBuf}; +use crate::eprintln_i18n; use crate::i18n::I18n; -use crate::println_i18n; use crate::version_utils::GodotVersion; /// Detect the Godot version by looking for a `project.godot` file in the @@ -24,7 +24,7 @@ pub fn detect_godot_version_in_path>(i18n: &I18n, path: P) -> Opt let contents = match fs::read_to_string(&project_file) { Ok(s) => s, Err(_) => { - println_i18n!(i18n, "error-failed-reading-project-godot"); + eprintln_i18n!(i18n, "error-failed-reading-project-godot"); return None; } }; @@ -152,9 +152,7 @@ fn extract_application_section(contents: &str) -> Option> { /// and returns the first substring that matches `x.x` or `x.x.x`. fn parse_packed_string_array_for_version(line: &str) -> Option { // Strip out the `config/features=` prefix. - let Some(eq_index) = line.find('=') else { - return None; - }; + let eq_index = line.find('=')?; let value_part = line[(eq_index + 1)..].trim(); // Expect something like `PackedStringArray("4.3", "Forward Plus")`. @@ -196,13 +194,7 @@ fn parse_packed_string_array_for_version(line: &str) -> Option { } // Find the first string that looks like a version `x.x` or `x.x.x`. - for v in versions { - if is_version_format(&v) { - return Some(v); - } - } - - None + versions.into_iter().find(|v| is_version_format(v)) } /// Check if a string matches `x.x` or `x.x.x` where x are digits. diff --git a/src/version_utils.rs b/src/version_utils.rs index 46c5399..b0d31a4 100644 --- a/src/version_utils.rs +++ b/src/version_utils.rs @@ -1,7 +1,7 @@ use regex::Regex; use std::fmt; -#[derive(Debug)] +#[derive(Debug, Default)] pub struct GodotVersion { pub major: Option, pub minor: Option, @@ -23,19 +23,6 @@ impl From<&GodotVersionDeterminate> for GodotVersion { } } -impl Default for GodotVersion { - fn default() -> Self { - Self { - major: None, - minor: None, - patch: None, - subpatch: None, - release_type: None, - is_csharp: None, - } - } -} - impl Clone for GodotVersion { fn clone(&self) -> Self { Self { @@ -100,7 +87,7 @@ impl Clone for GodotVersionDeterminate { impl GodotVersion { /// Parse an installed version folder name (e.g. "4.1.1-rc1-csharp", "2.0.4.1-stable", "3-csharp"). pub fn from_install_str(s: &str) -> Result { - Ok(Self::parse_with_csharp_and_pre_release(s, false)?) + Self::parse_with_csharp_and_pre_release(s, false) } /// Parse a remote version tag (e.g. "4.1.stable", "3.rc1", "2.0.4.1.stable"). @@ -141,10 +128,7 @@ impl GodotVersion { }); // If all components are zero or only major is present - let last_non_zero = match last_non_zero { - Some(index) => index, - None => 0, // Only major is present - }; + let last_non_zero = last_non_zero.unwrap_or(0); // - If the last non-zero component is before minor but minor is present, include minor. // - This ensures "4.0.0.0" becomes "4.0" instead of "4". @@ -155,10 +139,8 @@ impl GodotVersion { }; // Ensure all components up to `truncate_to` are present - for i in 0..=truncate_to { - if components[i].is_none() { - return None; // Missing intermediate component - } + for component in components.iter().take(truncate_to + 1) { + (*component)?; } // Collect and format the version string @@ -528,7 +510,7 @@ mod tests { let gv = GodotVersion::from_install_str("3.5-stable").unwrap(); assert_eq!(gv.major, Some(3)); assert_eq!(gv.minor, Some(5)); - assert_eq!(gv.is_stable(), true); + assert!(gv.is_stable()); } #[test] diff --git a/src/zip_utils.rs b/src/zip_utils.rs index 3128a61..6adbeab 100644 --- a/src/zip_utils.rs +++ b/src/zip_utils.rs @@ -1,5 +1,5 @@ +use crate::eprintln_i18n; use crate::i18n::I18n; -use crate::println_i18n; use anyhow::{anyhow, Result}; use fluent_bundle::FluentValue; use indicatif::{ProgressBar, ProgressStyle}; @@ -11,7 +11,7 @@ use zip::ZipArchive; pub fn extract_zip(zip_path: &Path, extract_to: &Path, i18n: &I18n) -> Result<()> { // Print extracting message - println_i18n!(i18n, "operation-extracting"); + eprintln_i18n!(i18n, "operation-extracting"); // First pass: Calculate total uncompressed size and collect top-level entries let file = fs::File::open(zip_path).map_err(|e| {