Merge pull request #423 from yykamei/load-config-before-start

Load config before start()

Cargo.toml

@@ -44,6 +44,7 @@ features = ["unstable"]
 
 [dev-dependencies]
 insta = "1.6.0"
+tempfile = "3.2.0"
 
 [build-dependencies]
 structopt = "0.3"

src/common/input/config.rs

@@ -6,14 +6,17 @@ use std::io::{self, Read};
 use std::path::{Path, PathBuf};
 
 use super::keybinds::{Keybinds, KeybindsFromYaml};
-use crate::cli::ConfigCli;
+use crate::cli::{CliArgs, ConfigCli};
 use crate::common::install;
 
 use serde::Deserialize;
+use std::convert::TryFrom;
+
+const DEFAULT_CONFIG_FILE_NAME: &str = "config.yaml";
 
 type ConfigResult = Result<Config, ConfigError>;
 
-/// Intermediate deserialisation config struct
+/// Intermediate deserialization config struct
 #[derive(Debug, Deserialize)]
 pub struct ConfigFromYaml {
     pub keybinds: Option<KeybindsFromYaml>,
@@ -27,13 +30,13 @@ pub struct Config {
 
 #[derive(Debug)]
 pub enum ConfigError {
-    // Deserialisation error
+    // Deserialization error
     Serde(serde_yaml::Error),
     // Io error
     Io(io::Error),
     // Io error with path context
     IoPath(io::Error, PathBuf),
-    // Internal Deserialisation Error
+    // Internal Deserialization Error
     FromUtf8(std::string::FromUtf8Error),
 }
 
@@ -44,6 +47,35 @@ impl Default for Config {
     }
 }
 
+impl TryFrom<&CliArgs> for Config {
+    type Error = ConfigError;
+
+    fn try_from(opts: &CliArgs) -> ConfigResult {
+        if let Some(ref path) = opts.config {
+            return Config::new(&path);
+        }
+
+        if let Some(ConfigCli::Config { clean, .. }) = opts.option {
+            if clean {
+                return Config::from_default_assets();
+            }
+        }
+
+        let config_dir = opts.config_dir.clone().or_else(install::default_config_dir);
+
+        if let Some(ref config) = config_dir {
+            let path = config.join(DEFAULT_CONFIG_FILE_NAME);
+            if path.exists() {
+                Config::new(&path)
+            } else {
+                Config::from_default_assets()
+            }
+        } else {
+            Config::from_default_assets()
+        }
+    }
+}
+
 impl Config {
     /// Uses defaults, but lets config override them.
     pub fn from_yaml(yaml_config: &str) -> ConfigResult {
@@ -71,45 +103,6 @@ impl Config {
     pub fn from_default_assets() -> ConfigResult {
         Self::from_yaml(String::from_utf8(install::DEFAULT_CONFIG.to_vec())?.as_str())
     }
-
-    /// Entry point of the configuration
-    #[cfg(not(test))]
-    pub fn from_cli_config(
-        location: Option<PathBuf>,
-        cli_config: Option<ConfigCli>,
-        config_dir: Option<PathBuf>,
-    ) -> ConfigResult {
-        if let Some(path) = location {
-            return Config::new(&path);
-        }
-
-        if let Some(ConfigCli::Config { clean, .. }) = cli_config {
-            if clean {
-                return Config::from_default_assets();
-            }
-        }
-
-        if let Some(config) = config_dir {
-            let path = config.join("config.yaml");
-            if path.exists() {
-                Config::new(&path)
-            } else {
-                Config::from_default_assets()
-            }
-        } else {
-            Config::from_default_assets()
-        }
-    }
-
-    /// In order not to mess up tests from changing configurations
-    #[cfg(test)]
-    pub fn from_cli_config(
-        _: Option<PathBuf>,
-        _: Option<ConfigCli>,
-        _: Option<PathBuf>,
-    ) -> ConfigResult {
-        Ok(Config::default())
-    }
 }
 
 impl Display for ConfigError {
@@ -119,7 +112,7 @@ impl Display for ConfigError {
             ConfigError::IoPath(ref err, ref path) => {
                 write!(formatter, "IoError: {}, File: {}", err, path.display(),)
             }
-            ConfigError::Serde(ref err) => write!(formatter, "Deserialisation error: {}", err),
+            ConfigError::Serde(ref err) => write!(formatter, "Deserialization error: {}", err),
             ConfigError::FromUtf8(ref err) => write!(formatter, "FromUtf8Error: {}", err),
         }
     }
@@ -157,20 +150,56 @@ impl From<std::string::FromUtf8Error> for ConfigError {
 // The unit test location.
 #[cfg(test)]
 mod config_test {
+    use std::io::Write;
+
+    use tempfile::tempdir;
+
     use super::*;
 
     #[test]
-    fn clean_option_equals_default_config() {
-        let cli_config = ConfigCli::Config { clean: true };
-        let config = Config::from_cli_config(None, Some(cli_config), None).unwrap();
-        let default = Config::default();
-        assert_eq!(config, default);
+    fn try_from_cli_args_with_config() {
+        let arbitrary_config = PathBuf::from("nonexistent.yaml");
+        let mut opts = CliArgs::default();
+        opts.config = Some(arbitrary_config);
+        println!("OPTS= {:?}", opts);
+        let result = Config::try_from(&opts);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn try_from_cli_args_with_option_clean() {
+        let mut opts = CliArgs::default();
+        opts.option = Some(ConfigCli::Config { clean: true });
+        let result = Config::try_from(&opts);
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn try_from_cli_args_with_config_dir() {
+        let mut opts = CliArgs::default();
+        let tmp = tempdir().unwrap();
+        File::create(tmp.path().join(DEFAULT_CONFIG_FILE_NAME))
+            .unwrap()
+            .write_all(b"keybinds: invalid\n")
+            .unwrap();
+        opts.config_dir = Some(tmp.path().to_path_buf());
+        let result = Config::try_from(&opts);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn try_from_cli_args_with_config_dir_without_config() {
+        let mut opts = CliArgs::default();
+        let tmp = tempdir().unwrap();
+        opts.config_dir = Some(tmp.path().to_path_buf());
+        let result = Config::try_from(&opts);
+        assert_eq!(result.unwrap(), Config::default());
     }
 
     #[test]
-    fn no_config_option_file_equals_default_config() {
-        let config = Config::from_cli_config(None, None, None).unwrap();
-        let default = Config::default();
-        assert_eq!(config, default);
+    fn try_from_cli_args_default() {
+        let opts = CliArgs::default();
+        let result = Config::try_from(&opts);
+        assert_eq!(result.unwrap(), Config::default());
     }
 }

src/common/mod.rs

@@ -120,7 +120,7 @@ pub enum AppInstruction {
 
 /// Start Zellij with the specified [`OsApi`] and command-line arguments.
 // FIXME this should definitely be modularized and split into different functions.
-pub fn start(mut os_input: Box<dyn OsApi>, opts: CliArgs) {
+pub fn start(mut os_input: Box<dyn OsApi>, opts: CliArgs, config: Config) {
     let take_snapshot = "\u{1b}[?1049h";
     os_input.unset_raw_mode(0);
     let _ = os_input
@@ -130,15 +130,6 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: CliArgs) {
 
     env::set_var(&"ZELLIJ", "0");
 
-    let config_dir = opts.config_dir.or_else(install::default_config_dir);
-
-    let config = Config::from_cli_config(opts.config, opts.option, config_dir)
-        .map_err(|e| {
-            eprintln!("There was an error in the config file:\n{}", e);
-            std::process::exit(1);
-        })
-        .unwrap();
-
     let command_is_executing = CommandIsExecuting::new();
 
     let full_screen_ws = os_input.get_terminal_size_using_fd(0);

src/main.rs

@@ -7,6 +7,7 @@ mod client;
 
 use crate::cli::CliArgs;
 use crate::command_is_executing::CommandIsExecuting;
+use crate::common::input::config::Config;
 use crate::os_input_output::get_os_input;
 use crate::utils::{
     consts::{ZELLIJ_IPC_PIPE, ZELLIJ_TMP_DIR, ZELLIJ_TMP_LOG_DIR},
@@ -17,12 +18,20 @@ use common::{
     command_is_executing, errors, install, os_input_output, pty_bus, screen, start, utils, wasm_vm,
     ApiCommand,
 };
+use std::convert::TryFrom;
 use std::io::Write;
 use std::os::unix::net::UnixStream;
 use structopt::StructOpt;
 
 pub fn main() {
     let opts = CliArgs::from_args();
+    let config = match Config::try_from(&opts) {
+        Ok(config) => config,
+        Err(e) => {
+            eprintln!("There was an error in the config file:\n{}", e);
+            std::process::exit(1);
+        }
+    };
     if let Some(split_dir) = opts.split {
         match split_dir {
             'h' => {
@@ -66,6 +75,6 @@ pub fn main() {
         let os_input = get_os_input();
         atomic_create_dir(ZELLIJ_TMP_DIR).unwrap();
         atomic_create_dir(ZELLIJ_TMP_LOG_DIR).unwrap();
-        start(Box::new(os_input), opts);
+        start(Box::new(os_input), opts, config);
     }
 }