From c200e0070d819f15a1f221b8259976241fb985f5 Mon Sep 17 00:00:00 2001 From: dark0dave Date: Mon, 29 Jul 2024 00:32:07 +0100 Subject: [PATCH] feat(panic): Remove all panics, use try froms and pass error up Signed-off-by: dark0dave --- fixtures/weidu.log | 6 +++ src/component.rs | 61 +++++++++++++++----------- src/log_file.rs | 31 ++++++------- src/main.rs | 105 +++++++++++++++++++++++++++++++++++++-------- src/weidu.rs | 1 - 5 files changed, 140 insertions(+), 64 deletions(-) create mode 100644 fixtures/weidu.log diff --git a/fixtures/weidu.log b/fixtures/weidu.log new file mode 100644 index 0000000..0588aa5 --- /dev/null +++ b/fixtures/weidu.log @@ -0,0 +1,6 @@ +// Log of Currently Installed WeiDU Mods +// The top of the file is the 'oldest' mod +// ~TP2_File~ #language_number #component_number // [Subcomponent Name -> ] Component Name [ : Version] +~TEST_MOD_NAME_1/TEST.TP2~ #0 #0 // test mod one +~TEST_MOD_NAME_2/END.TP2~ #0 #0 // test mod with subcomponent information -> Standard installation +~TEST_MOD_NAME_4/TWEAKS.TP2~ #0 #3346 // test mod with both subcomponent information and version -> Casting speed only: v16 diff --git a/src/component.rs b/src/component.rs index fe2ddc7..85fdc79 100644 --- a/src/component.rs +++ b/src/component.rs @@ -1,3 +1,5 @@ +use std::error::Error; + // This should mirror the weidu component // https://github.com/WeiDUorg/weidu/blob/devel/src/tp.ml#L98 #[derive(Debug, PartialEq, PartialOrd, Clone)] @@ -11,18 +13,18 @@ pub(crate) struct Component { pub(crate) version: String, } -impl From for Component { - fn from(line: String) -> Self { +impl TryFrom for Component { + type Error = Box; + + fn try_from(line: String) -> Result { let mut parts = line.split('~'); let install_path = parts .nth(1) - .unwrap_or_else(|| { - panic!( - "Could not get full name of mod, from provided string: {}", - line - ) - }) + .ok_or(format!( + "Could not get full name of mod, from provided string: {}", + line + ))? .to_string(); // This allows for both linux, macos and windows parsing @@ -30,47 +32,54 @@ impl From for Component { let name = install_path .split('\\') .next() - .unwrap_or_else(|| { - panic!("Could not split {} into mod into name and component", line) - }) + .ok_or(format!( + "Could not split {} into mod into name and component", + line + ))? .to_ascii_lowercase(); (windows_path.to_string(), name) } else if let Some(linux_path) = install_path.split('/').nth(1) { let name = install_path .split('/') .next() - .unwrap_or_else(|| { - panic!("Could not split {} into mod into name and component", line) - }) + .ok_or(format!( + "Could not split {} into mod into name and component", + line + ))? .to_ascii_lowercase(); (linux_path.to_string(), name) } else { - panic!( + return Err(format!( "Could not find tp2 file name, from provided string: {}", line ) + .into()); }; let mut tail = parts .next() - .unwrap_or_else(|| { - panic!( - "Could not find lang and component, from provided string {}", - line - ) - }) + .ok_or(format!( + "Could not find lang and component, from provided string {}", + line + ))? .split("//"); let mut lang_and_component = tail.next().unwrap_or_default().split(' '); let lang = lang_and_component .nth(1) - .unwrap_or_else(|| panic!("Could not find lang, from provided string: {}", line)) + .ok_or(format!( + "Could not find lang, from provided string: {}", + line + ))? .replace('#', ""); let component = lang_and_component .next() - .unwrap_or_else(|| panic!("Could not find component, from provided string {}", line)) + .ok_or(format!( + "Could not find component, from provided string {}", + line + ))? .replace('#', ""); let mut component_name_sub_component_version = tail.next().unwrap_or_default().split(':'); @@ -98,7 +107,7 @@ impl From for Component { .trim() .to_string(); - Component { + Ok(Component { tp_file, name, lang, @@ -106,7 +115,7 @@ impl From for Component { component_name, sub_component, version, - } + }) } } @@ -118,7 +127,7 @@ mod tests { #[test] fn test_parse_windows() { let mod_string = r"~TOBEX\TOBEX.TP2~ #0 #100 // TobEx - Core: v28"; - let mod_component = Component::from(mod_string.to_string()); + let mod_component = Component::try_from(mod_string.to_string()).unwrap(); let expected = Component { tp_file: "TOBEX.TP2".to_string(), name: "tobex".to_string(), diff --git a/src/log_file.rs b/src/log_file.rs index 860dc3e..881da9e 100644 --- a/src/log_file.rs +++ b/src/log_file.rs @@ -1,6 +1,7 @@ use std::{ + error::Error, fs::File, - io::{self, BufRead, BufReader}, + io::{BufRead, BufReader}, path::PathBuf, slice::Iter, }; @@ -8,7 +9,7 @@ use std::{ use crate::component::Component; #[derive(Debug, PartialEq, PartialOrd)] -pub(crate) struct LogFile(Vec); +pub(crate) struct LogFile(pub(crate) Vec); impl LogFile { pub(crate) fn len(&self) -> usize { @@ -33,28 +34,20 @@ impl<'a> IntoIterator for &'a LogFile { } impl TryFrom for LogFile { - type Error = io::Error; + type Error = Box; fn try_from(value: PathBuf) -> Result { let file = File::open(value)?; let reader = BufReader::new(file); + let mut components = vec![]; - Ok(LogFile( - reader - .lines() - .flat_map(|line| match line { - // Ignore comments and empty lines - Ok(component) - if !component.is_empty() - && !component.starts_with('\n') - && !component.starts_with("//") => - { - Some(Component::from(component)) - } - _ => None, - }) - .collect(), - )) + for line in reader.lines().map_while(|line| line.ok()) { + // Ignore comments and empty lines + if !line.is_empty() && !line.starts_with('\n') && !line.starts_with("//") { + components.push(Component::try_from(line)?) + } + } + Ok(Self(components)) } } diff --git a/src/main.rs b/src/main.rs index 5624756..25d4986 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::{collections::HashMap, error::Error, path::PathBuf, process::ExitCode}; use args::Args; use clap::Parser; @@ -18,22 +18,15 @@ mod utils; mod weidu; mod weidu_parser; -fn main() { - env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); - log::info!( - r" - /\/\ ___ __| | (_)_ __ ___| |_ __ _| | | ___ _ __ - / \ / _ \ / _` | | | '_ \/ __| __/ _` | | |/ _ \ '__| - / /\/\ \ (_) | (_| | | | | | \__ \ || (_| | | | __/ | - \/ \/\___/ \__,_| |_|_| |_|___/\__\__,_|_|_|\___|_| - " - ); - let args = Args::parse(); - - let mut mods = LogFile::try_from(args.log_file).expect("Could not open log file"); +fn find_mods( + log_file: PathBuf, + skip_installed: bool, + game_directory: PathBuf, +) -> Result> { + let mut mods = LogFile::try_from(log_file)?; let number_of_mods_found = mods.len(); - let mods_to_be_installed = if args.skip_installed { - let existing_weidu_log_file_path = args.game_directory.join("weidu").with_extension("log"); + let mods_to_be_installed = if skip_installed { + let existing_weidu_log_file_path = game_directory.join("weidu").with_extension("log"); if let Ok(installed_mods) = LogFile::try_from(existing_weidu_log_file_path) { for installed_mod in &installed_mods { mods.retain(|mod_to_install| installed_mod != mod_to_install); @@ -49,6 +42,32 @@ fn main() { number_of_mods_found, mods_to_be_installed.len() ); + Ok(mods_to_be_installed) +} + +fn main() -> ExitCode { + env_logger::Builder::from_env(Env::default().default_filter_or("info")).init(); + log::info!( + r" + /\/\ ___ __| | (_)_ __ ___| |_ __ _| | | ___ _ __ + / \ / _ \ / _` | | | '_ \/ __| __/ _` | | |/ _ \ '__| + / /\/\ \ (_) | (_| | | | | | \__ \ || (_| | | | __/ | + \/ \/\___/ \__,_| |_|_| |_|___/\__\__,_|_|_|\___|_| + " + ); + let args = Args::parse(); + + let mods_to_be_installed = match find_mods( + args.log_file, + args.skip_installed, + args.game_directory.clone(), + ) { + Ok(mods) => mods, + Err(err) => { + log::error!("Failed to find log file, {:?}", err); + return ExitCode::FAILURE; + } + }; let mut mod_folder_cache = HashMap::new(); for weidu_mod in &mods_to_be_installed { @@ -76,10 +95,12 @@ fn main() { args.timeout, ) { InstallationResult::Fail(message) => { - panic!( + log::error!( "Failed to install mod {}, error is '{}'", - weidu_mod.name, message + weidu_mod.name, + message ); + return ExitCode::FAILURE; } InstallationResult::Success => { log::info!("Installed mod {:?}", &weidu_mod); @@ -94,4 +115,52 @@ fn main() { } } } + ExitCode::SUCCESS +} + +#[cfg(test)] +mod tests { + use super::*; + use component::Component; + use pretty_assertions::assert_eq; + use std::path::PathBuf; + + #[test] + fn test_find_mods() { + let log_file = PathBuf::from("./fixtures/test.log"); + let skip_installed = false; + let game_directory = PathBuf::from("./fixtures"); + let result = find_mods(log_file.clone(), skip_installed, game_directory); + let expected = LogFile::try_from(log_file); + assert_eq!(expected.ok(), result.ok()) + } + + #[test] + fn test_find_mods_skip_installed() { + let log_file = PathBuf::from("./fixtures/test.log"); + let skip_installed = true; + let game_directory = PathBuf::from("./fixtures"); + let result = find_mods(log_file, skip_installed, game_directory).unwrap(); + let expected = LogFile(vec![ + Component { + tp_file: "TEST.TP2".to_string(), + name: "test_mod_name_1".to_string(), + lang: "0".to_string(), + component: "1".to_string(), + component_name: "test mod two".to_string(), + sub_component: "".to_string(), + version: "".to_string(), + }, + Component { + tp_file: "END.TP2".to_string(), + name: "test_mod_name_3".to_string(), + lang: "0".to_string(), + component: "0".to_string(), + component_name: "test mod with version".to_string(), + sub_component: "".to_string(), + version: "1.02".to_string(), + }, + ]); + assert_eq!(expected, result) + } } diff --git a/src/weidu.rs b/src/weidu.rs index f041bd9..f21f147 100644 --- a/src/weidu.rs +++ b/src/weidu.rs @@ -1,6 +1,5 @@ use std::{ io::{self, BufRead, BufReader, ErrorKind, Write}, - panic, path::PathBuf, process::{Child, ChildStdout, Command, Stdio}, sync::{