0.2.1 pt. 2 #76
0.2.1 pt. 2 #76
Conversation
…latform dependency checks for Linux
…usTotal URL handling
There was a problem hiding this comment.
Pull Request Overview
This PR represents version 0.2.1 part 2, introducing Ukrainian language support while cleaning up code and updating dependencies. The changes focus on internationalization, code modernization, and system improvements across both frontend and backend components.
Key changes:
- Adds Ukrainian (ua) translation support with complete translation file and UI integration
- Modernizes Rust code by replacing
lazy_staticwithstd::sync::LazyLockthroughout the codebase - Improves error handling and startup dependency checking with better Windows WebView2 detection
- Updates build system with enhanced CI workflows and dependency version bumps
Reviewed Changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/locales/ua.json | Complete Ukrainian translation file with 1138+ translation strings |
| src/i18n/index.ts | Registers Ukrainian language support in i18n configuration |
| src-tauri/src/lib.rs | Major refactoring with new startup error handling and dependency checking |
| src-tauri/src/core/platform/ | New platform-specific dependency checking modules for Windows and Linux |
| src-tauri/src/core/storage/ | Modernizes all storage modules to use LazyLock instead of lazy_static |
| src-tauri/src/core/utils/ | Updates logging and globals modules with modern Rust patterns |
| package.json | Updates frontend dependencies to latest versions |
| .github/workflows/build.yml | Enhanced CI with skip checks and improved artifact handling |
Comments suppressed due to low confidence (1)
src-tauri/src/core/clients/client.rs:1
- Inconsistent variable naming:
file_namevsfilename. The code usesData::get_filename(&self.filename)to get a processed filename but then checksfilename.contains(\"fabric/\")using the original field. This could lead to confusion and potential bugs. Consider using consistent variable names throughout this function.
use std::{
| pub use self::linux::{check_platform_dependencies, check_webkit_environment}; | ||
|
|
||
| #[cfg(not(any(target_os = "windows", target_os = "linux")))] | ||
| pub fn check_platform_dependencies() -> Result<(), StartupError> { |
There was a problem hiding this comment.
Missing import for StartupError. This will cause a compilation error since StartupError is not in scope for non-Windows, non-Linux platforms.
| pub fn get_all_presets(&self) -> Vec<ThemePreset> { | ||
| self.presets.values().cloned().collect() |
There was a problem hiding this comment.
The function now clones all presets instead of returning references. This change from Vec<&ThemePreset> to Vec<ThemePreset> creates unnecessary memory allocation and copying. Consider keeping the original implementation that returns references, or document why cloning is necessary here.
| pub fn get_all_presets(&self) -> Vec<ThemePreset> { | |
| self.presets.values().cloned().collect() | |
| pub fn get_all_presets(&self) -> Vec<&ThemePreset> { | |
| self.presets.values().collect() |
| .and_then(|manager| manager.get_active_account().map(|a| a.username.clone())) | ||
| .unwrap_or_else(|| { | ||
| let random_digits = rand::random::<u32>() % 100000; | ||
| let random_digits = rand::random::<u32>() % 100_000; |
There was a problem hiding this comment.
Using modulo with rand::random() can introduce bias in the random number distribution. Consider using rand::thread_rng().gen_range(0..100_000) instead for a more uniform distribution.
| PRESET_MANAGER | ||
| .lock() | ||
| .map(|p| p.get_all_presets()) | ||
| .map_err(|e| { | ||
| log_warn!("Failed to get presets: {}", e); | ||
| "Failed to get presets".to_string() | ||
| }) | ||
| } |
There was a problem hiding this comment.
This function doesn't handle the case where the lock is poisoned correctly. map() and map_err() expect Result but Mutex::lock() returns LockResult which may contain a PoisonError. This should use unwrap_or_else() or proper error handling for poisoned mutexes.
| PRESET_MANAGER | |
| .lock() | |
| .map(|p| p.get_all_presets()) | |
| .map_err(|e| { | |
| log_warn!("Failed to get presets: {}", e); | |
| "Failed to get presets".to_string() | |
| }) | |
| } | |
| match PRESET_MANAGER.lock() { | |
| Ok(preset_manager) => Ok(preset_manager.get_all_presets()), | |
| Err(poisoned) => { | |
| log_warn!("PRESET_MANAGER mutex was poisoned: {}", poisoned); | |
| // Attempt to recover the data | |
| Ok(poisoned.into_inner().get_all_presets()) | |
| } | |
| } |
| let mut command = std::process::Command::new(&path); | ||
| command.arg("/install"); |
There was a problem hiding this comment.
The WebView2 installer is executed without validating its signature or integrity. Consider adding hash verification or signature validation before executing the downloaded installer to prevent potential security vulnerabilities from compromised downloads.
No description provided.