From 04cbe8ac20a3c25b6135b2ddfc4676ca8584806b Mon Sep 17 00:00:00 2001
From: Thomas Frans <franske2000@gmail.com>
Date: Sat, 27 May 2023 14:15:55 +0200
Subject: [PATCH] refactor: move base path config + documentation

---
 src/application.rs | 24 +++++++-----------------
 src/config.rs      | 11 +++++++++++
 src/main.rs        | 11 +++++++++--
 src/spotify.rs     |  2 ++
 4 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/application.rs b/src/application.rs
index 3d6136c..285d04f 100644
--- a/src/application.rs
+++ b/src/application.rs
@@ -1,6 +1,5 @@
 use crate::{command, ipc, mpris, queue, spotify};
-use std::fs;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use cursive::event::EventTrigger;
@@ -15,7 +14,7 @@ use signal_hook::{consts::SIGHUP, consts::SIGTERM, iterator::Signals};
 
 use crate::command::{Command, JumpMode};
 use crate::commands::CommandManager;
-use crate::config::{self, cache_path, Config};
+use crate::config::{self, cache_path, set_configuration_base_path, Config};
 use crate::events::{Event, EventManager};
 use crate::ext_traits::CursiveExt;
 use crate::library::Library;
@@ -37,7 +36,8 @@ fn credentials_prompt(error_message: Option<String>) -> Result<Credentials, Stri
     authentication::create_credentials()
 }
 
-fn setup_logging(filename: &str) -> Result<(), fern::InitError> {
+/// Set up the global logger to log to `filename`.
+pub fn setup_logging(filename: &Path) -> Result<(), fern::InitError> {
     fern::Dispatch::new()
         // Perform allocation-free log formatting
         .format(|out, message, record| {
@@ -92,30 +92,20 @@ impl 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<PathBuf>,
         configuration_base_path: Option<PathBuf>,
         configuration_file_path: Option<PathBuf>,
     ) -> Result<Self, String> {
-        if let Some(filename) = debug_log_path {
-            setup_logging(&filename.to_string_lossy()).expect("can't setup logging");
-        }
-
-        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(basepath);
-        }
+        set_configuration_base_path(configuration_base_path);
 
         // 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<crate::config::Config> = Arc::new(Config::new(
+        let config = Arc::new(Config::new(
             configuration_file_path.unwrap_or("config.toml".into()),
         ));
+
         let mut credentials = {
             let cache = Cache::new(Some(config::cache_path("librespot")), None, None, None)
                 .expect("Could not create librespot cache");
diff --git a/src/config.rs b/src/config.rs
index ace2e78..556b031 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -254,6 +254,7 @@ impl Config {
         crate::theme::load(theme)
     }
 
+    /// Reload the configuration file.
     pub fn reload(&self) {
         let cfg = load(&self.filename.to_string_lossy()).expect("could not reload config");
         *self.values.write().expect("can't writelock config values") = cfg
@@ -327,3 +328,13 @@ pub fn cache_path(file: &str) -> PathBuf {
     pb.push(file);
     pb
 }
+
+/// Set the configuration base path. All configuration files are read/written relative to this path.
+pub fn set_configuration_base_path(base_path: Option<PathBuf>) {
+    if let Some(basepath) = base_path {
+        if !basepath.exists() {
+            fs::create_dir_all(&basepath).expect("could not create basepath directory");
+        }
+        *BASE_PATH.write().unwrap() = Some(basepath);
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index 56b30e5..ea53c82 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -7,7 +7,7 @@ extern crate serde;
 
 use std::path::PathBuf;
 
-use application::Application;
+use application::{setup_logging, Application};
 use ncspot::program_arguments;
 
 mod application;
@@ -43,13 +43,20 @@ fn main() -> Result<(), String> {
     // stdout is most likely in use by Cursive.
     panic::register_backtrace_panic_handler();
 
+    // Parse the command line arguments.
     let matches = program_arguments().get_matches();
 
+    // Enable debug logging to a file if specified on the command line.
+    if let Some(filename) = matches.get_one::<PathBuf>("debug") {
+        setup_logging(filename).expect("logger could not be initialized");
+    }
+
+    // Create the application.
     let mut application = Application::new(
-        matches.get_one::<PathBuf>("debug").cloned(),
         matches.get_one::<PathBuf>("basepath").cloned(),
         matches.get_one::<PathBuf>("config").cloned(),
     )?;
 
+    // Start the application event loop.
     application.run()
 }
diff --git a/src/spotify.rs b/src/spotify.rs
index 231a2c9..88ac916 100644
--- a/src/spotify.rs
+++ b/src/spotify.rs
@@ -40,6 +40,8 @@ pub enum PlayerEvent {
     FinishedTrack,
 }
 
+// TODO: Rename or document this as it isn't immediately clear what it represents/does from the
+// name.
 #[derive(Clone)]
 pub struct Spotify {
     events: EventManager,