From abffb3c2a9ea1332fb60c385387d276df3cf8c15 Mon Sep 17 00:00:00 2001 From: Thomas Frans Date: Wed, 24 May 2023 00:02:43 +0200 Subject: [PATCH] refactor: move cli argument parsing to main Command line arguments are part of the OS process and should be under main. --- src/application.rs | 34 +++++++++++++++++++--------------- src/config.rs | 10 +++++----- src/lib.rs | 4 ++++ src/main.rs | 14 +++++++++++--- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/application.rs b/src/application.rs index 1a5b7a7..3d6136c 100644 --- a/src/application.rs +++ b/src/application.rs @@ -1,7 +1,6 @@ use crate::{command, ipc, mpris, queue, spotify}; use std::fs; use std::path::PathBuf; -use std::str::FromStr; use std::sync::Arc; use cursive::event::EventTrigger; @@ -11,7 +10,6 @@ use librespot_core::authentication::Credentials; use librespot_core::cache::Cache; use log::{error, info, trace}; -use ncspot::program_arguments; #[cfg(unix)] use signal_hook::{consts::SIGHUP, consts::SIGTERM, iterator::Signals}; @@ -90,27 +88,33 @@ pub struct Application { } impl Application { - pub fn new() -> Result { - let matches = program_arguments().get_matches(); - - if let Some(filename) = matches.get_one::("debug") { - setup_logging(filename).expect("can't setup logging"); + /// Create a new ncspot application. + /// + /// # Arguments + /// + /// * `debug_log_path` - Path where an optional debug log should be written to + /// * `configuration_base_path` - Path to the configuration directory + /// * `configuration_file_path` - Relative path to the configuration file inside the base path + pub fn new( + debug_log_path: Option, + configuration_base_path: Option, + configuration_file_path: Option, + ) -> Result { + if let Some(filename) = debug_log_path { + setup_logging(&filename.to_string_lossy()).expect("can't setup logging"); } - if let Some(basepath) = matches.get_one::("basepath") { - let path = PathBuf::from_str(basepath).expect("invalid path"); - if !path.exists() { - fs::create_dir_all(&path).expect("could not create basepath directory"); + if let Some(basepath) = configuration_base_path { + if !basepath.exists() { + fs::create_dir_all(&basepath).expect("could not create basepath directory"); } - *config::BASE_PATH.write().unwrap() = Some(path); + *config::BASE_PATH.write().unwrap() = Some(basepath); } // Things here may cause the process to abort; we must do them before creating curses windows // otherwise the error message will not be seen by a user let config: Arc = Arc::new(Config::new( - matches - .get_one::("config") - .unwrap_or(&"config.toml".to_string()), + configuration_file_path.unwrap_or("config.toml".into()), )); let mut credentials = { let cache = Cache::new(Some(config::cache_path("librespot")), None, None, None) diff --git a/src/config.rs b/src/config.rs index a7ac036..6bea25e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -169,14 +169,14 @@ lazy_static! { } pub struct Config { - filename: String, + filename: PathBuf, values: RwLock, state: RwLock, } impl Config { - pub fn new(filename: &str) -> Self { - let values = load(filename).unwrap_or_else(|e| { + pub fn new(filename: PathBuf) -> Self { + let values = load(&filename.to_string_lossy()).unwrap_or_else(|e| { eprintln!("could not load config: {e}"); process::exit(1); }); @@ -200,7 +200,7 @@ impl Config { } Self { - filename: filename.to_string(), + filename, values: RwLock::new(values), state: RwLock::new(userstate), } @@ -239,7 +239,7 @@ impl Config { } pub fn reload(&self) { - let cfg = load(&self.filename).expect("could not reload config"); + let cfg = load(&self.filename.to_string_lossy()).expect("could not reload config"); *self.values.write().expect("can't writelock config values") = cfg } } diff --git a/src/lib.rs b/src/lib.rs index 3db26d2..b3b8a87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +use clap::builder::PathBufValueParser; use librespot_playback::audio_backend; pub const AUTHOR: &str = "Henrik Friedrichsen and contributors"; @@ -22,6 +23,7 @@ pub fn program_arguments() -> clap::Command { .short('d') .long("debug") .value_name("FILE") + .value_parser(PathBufValueParser::new()) .help("Enable debug logging to the specified file"), ) .arg( @@ -29,6 +31,7 @@ pub fn program_arguments() -> clap::Command { .short('b') .long("basepath") .value_name("PATH") + .value_parser(PathBufValueParser::new()) .help("custom basepath to config/cache files"), ) .arg( @@ -36,6 +39,7 @@ pub fn program_arguments() -> clap::Command { .short('c') .long("config") .value_name("FILE") + .value_parser(PathBufValueParser::new()) .help("Filename of config file in basepath") .default_value("config.toml"), ) diff --git a/src/main.rs b/src/main.rs index ab2eecf..2d8c564 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,10 @@ extern crate serde; use std::backtrace; use std::fs::File; use std::io::Write; +use std::path::PathBuf; + +use application::Application; +use ncspot::program_arguments; mod application; mod authentication; @@ -36,8 +40,6 @@ mod ipc; #[cfg(feature = "mpris")] mod mpris; -use crate::application::Application; - /// Register a custom panic handler to write the backtrace to a file since stdout is in use by the /// Cursive TUI library during most of the application. fn register_backtrace_panic_handler() { @@ -64,7 +66,13 @@ fn register_backtrace_panic_handler() { fn main() -> Result<(), String> { register_backtrace_panic_handler(); - let mut application = Application::new()?; + let matches = program_arguments().get_matches(); + + let mut application = Application::new( + matches.get_one::("debug").cloned(), + matches.get_one::("basepath").cloned(), + matches.get_one::("config").cloned(), + )?; application.run() }