From 1fa1149c75eb6b520135b198286308fd2bf19bde Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 17:25:53 +0100 Subject: [PATCH 1/8] use default trait for `Config::default` --- command/src/main.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index 2f653ca8..c84e3ee4 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -1,3 +1,5 @@ +#![warn(clippy::pedantic)] + use cliclack::{ confirm, input, intro, log::{self, info}, @@ -36,10 +38,7 @@ struct Config { pairing_port: Option, } -impl Config { - const SETTINGS_TOML: &'static str = "settings.toml"; - const SETTINGS_ENV: &'static str = "settings.env"; - +impl Default for Config { fn default() -> Self { Self { compose_profiles: None, @@ -55,6 +54,11 @@ impl Config { pairing_port: None, } } +} + +impl Config { + const SETTINGS_TOML: &'static str = "settings.toml"; + const SETTINGS_ENV: &'static str = "settings.env"; fn load() -> Self { if !Path::new(Self::SETTINGS_TOML).exists() { From 7d16a3d465c5f6baa944886e4d10beaf57400625 Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 18:19:47 +0100 Subject: [PATCH 2/8] alternative `to_env` function Destructuring ensures all fields are used. --- command/Cargo.lock | 39 ------------------- command/Cargo.toml | 1 - command/src/main.rs | 91 ++++++++++++++++++++++++++------------------- 3 files changed, 53 insertions(+), 78 deletions(-) diff --git a/command/Cargo.lock b/command/Cargo.lock index cfa54473..6880770f 100644 --- a/command/Cargo.lock +++ b/command/Cargo.lock @@ -34,7 +34,6 @@ dependencies = [ "cliclack", "local-ip-address", "serde", - "struct_iterable", "toml", ] @@ -69,15 +68,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" -[[package]] -name = "erased-serde" -version = "0.3.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c138974f9d5e7fe373eb04df7cae98833802ae4b11c24ac7039a21d5af4b26c" -dependencies = [ - "serde", -] - [[package]] name = "hashbrown" version = "0.14.3" @@ -248,35 +238,6 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" -[[package]] -name = "struct_iterable" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "849a064c6470a650b72e41fa6c057879b68f804d113af92900f27574828e7712" -dependencies = [ - "struct_iterable_derive", - "struct_iterable_internal", -] - -[[package]] -name = "struct_iterable_derive" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bb939ce88a43ea4e9d012f2f6b4cc789deb2db9d47bad697952a85d6978662c" -dependencies = [ - "erased-serde", - "proc-macro2", - "quote", - "struct_iterable_internal", - "syn 2.0.39", -] - -[[package]] -name = "struct_iterable_internal" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9426b2a0c03e6cc2ea8dbc0168dbbf943f88755e409fb91bcb8f6a268305f4a" - [[package]] name = "syn" version = "1.0.109" diff --git a/command/Cargo.toml b/command/Cargo.toml index 0247344e..6a0d02b4 100644 --- a/command/Cargo.toml +++ b/command/Cargo.toml @@ -9,5 +9,4 @@ edition = "2021" cliclack = "0.1.6" local-ip-address = "0.5.6" serde = { version = "1.0.193", features = ["derive"] } -struct_iterable = "0.1.1" toml = "0.8.8" diff --git a/command/src/main.rs b/command/src/main.rs index c84e3ee4..17b88c6a 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -8,11 +8,10 @@ use cliclack::{ use local_ip_address::local_ip; use serde::{Deserialize, Serialize}; use std::{ - collections::HashMap, + format, io::Error, path::{Path, PathBuf}, }; -use struct_iterable::Iterable; const BANNER: &str = r" |\_ _ _ _ @@ -23,7 +22,7 @@ const BANNER: &str = r" |___/ "; -#[derive(Serialize, Deserialize, Iterable, Debug)] +#[derive(Serialize, Deserialize, Debug)] struct Config { compose_profiles: Option>, setup_database: Option, @@ -56,6 +55,18 @@ impl Default for Config { } } +macro_rules! to_env { + ($name_opt:ident) => { + $name_opt + .clone() + .map(|v| format!("{}={}", stringify!($name_opt).to_uppercase(), v.to_string())) + .unwrap_or_default() + }; + ($key:ident, $value:expr) => { + format!("{}={}", stringify!($key).to_uppercase(), $value) + }; +} + impl Config { const SETTINGS_TOML: &'static str = "settings.toml"; const SETTINGS_ENV: &'static str = "settings.env"; @@ -79,39 +90,37 @@ impl Config { } fn to_env(&self) -> String { - let mut contents: HashMap<&str, String> = HashMap::new(); - - for (key, value) in self.iter() { - if let Some(string_opt) = value.downcast_ref::>() { - if let Some(string_opt) = string_opt { - contents.insert(key, string_opt.to_string()); - } - } else if let Some(bool_opt) = value.downcast_ref::>() { - if let Some(bool_opt) = bool_opt { - contents.insert(key, bool_opt.to_string()); - } - } else if let Some(u16_opt) = value.downcast_ref::>() { - if let Some(u16_opt) = u16_opt { - contents.insert(key, u16_opt.to_string()); - } - } else if let Some(u32_opt) = value.downcast_ref::>() { - if let Some(u32_opt) = u32_opt { - contents.insert(key, u32_opt.to_string()); - } - } else if let Some(vec_string) = value.downcast_ref::>>() { - if let Some(vec_string) = vec_string { - contents.insert(key, vec_string.join(",")); - } - } else { - panic!("Unsupported type: Could not write [{key}] to env"); - } - } - - contents - .iter() - .map(|(k, v)| format!("{}={}", k.to_uppercase(), v)) - .collect::>() - .join("\n") + let Self { + compose_profiles, + setup_database, + enable_monitoring, + su_password, + password, + lila_domain, + lila_url, + phone_ip, + connection_port, + pairing_code, + pairing_port, + } = self; + let compose_profiles_string = compose_profiles + .clone() + .map(|v| v.join(",")) + .unwrap_or_default(); + format!( + "{}\n{}\n{}\n{}\n{}\n{}\n{}\n{}\n{}\n{}\n{}\n", + to_env!(compose_profiles, compose_profiles_string), + to_env!(setup_database), + to_env!(enable_monitoring), + to_env!(su_password), + to_env!(password), + to_env!(lila_domain), + to_env!(lila_url), + to_env!(phone_ip), + to_env!(connection_port), + to_env!(pairing_code), + to_env!(pairing_port) + ) } } @@ -551,6 +560,7 @@ fn welcome() -> std::io::Result<()> { #[cfg(test)] mod tests { use super::*; + use std::collections::HashMap; #[test] fn test_repository() { @@ -562,6 +572,12 @@ mod tests { assert_eq!(repo.clone_path(), Path::new("repos/lila")); } + #[test] + fn test_to_env_proc() { + let foo = Some("test"); + assert_eq!(to_env!(foo), "FOO=test"); + } + #[test] fn test_set_env_vars_from_struct() { let contents = Config { @@ -581,8 +597,7 @@ mod tests { let vars = contents .split("\n") - .map(|line| line.split("=")) - .map(|mut parts| (parts.next().unwrap(), parts.next().unwrap())) + .flat_map(|line| line.split_once("=")) .collect::>(); assert_eq!(vars["COMPOSE_PROFILES"], "foo,bar"); From fbe013b11164fed331579271889cc0a57d11d8f9 Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 18:20:29 +0100 Subject: [PATCH 3/8] use derive for default of `Config` should really have been detected by manual_default --- command/src/main.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index 17b88c6a..858ada53 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -22,7 +22,7 @@ const BANNER: &str = r" |___/ "; -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, Default)] struct Config { compose_profiles: Option>, setup_database: Option, @@ -37,24 +37,6 @@ struct Config { pairing_port: Option, } -impl Default for Config { - fn default() -> Self { - Self { - compose_profiles: None, - setup_database: None, - enable_monitoring: None, - su_password: None, - password: None, - lila_domain: None, - lila_url: None, - phone_ip: None, - connection_port: None, - pairing_code: None, - pairing_port: None, - } - } -} - macro_rules! to_env { ($name_opt:ident) => { $name_opt From 237d7f0f751df64f18bf2b8e11c5a408ae02aae9 Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 18:44:02 +0100 Subject: [PATCH 4/8] code-golf Config::load --- command/src/main.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index 858ada53..3509a7a3 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -54,12 +54,9 @@ impl Config { const SETTINGS_ENV: &'static str = "settings.env"; fn load() -> Self { - if !Path::new(Self::SETTINGS_TOML).exists() { - return Self::default(); - } - - let toml = std::fs::read_to_string(Self::SETTINGS_TOML).unwrap(); - toml::from_str(&toml).unwrap() + std::fs::read_to_string(Self::SETTINGS_TOML) + .map(|toml_str| toml::from_str(&toml_str).unwrap()) + .unwrap_or_else(|_| Self::default()) } fn save(&self) { From e7f8a478dd70dd7f8d3168cba15be32f0b025209 Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 18:44:14 +0100 Subject: [PATCH 5/8] bubble up the error from Config::save() since it's only used in places that return io::Result<()> --- command/src/main.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index 3509a7a3..2538ce66 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -59,9 +59,9 @@ impl Config { .unwrap_or_else(|_| Self::default()) } - fn save(&self) { - std::fs::write(Self::SETTINGS_TOML, self.to_toml()).unwrap(); - std::fs::write(Self::SETTINGS_ENV, self.to_env()).unwrap(); + fn save(&self) -> std::io::Result<()> { + std::fs::write(Self::SETTINGS_TOML, self.to_toml())?; + std::fs::write(Self::SETTINGS_ENV, self.to_env()) } fn to_toml(&self) -> String { @@ -221,7 +221,7 @@ fn setup(mut config: Config) -> std::io::Result<()> { config.lila_url = Some(gitpod.url); } - config.save(); + config.save()?; create_placeholder_dirs(); @@ -473,7 +473,7 @@ fn hostname(mut config: Config) -> std::io::Result<()> { config.lila_domain = Some(format!("{hostname}:8080")); config.lila_url = Some(format!("http://{hostname}:8080")); - config.save(); + config.save()?; outro(format!("✔ Local Lichess URL set to http://{hostname}:8080")) } @@ -507,7 +507,7 @@ fn mobile_setup(mut config: Config) -> std::io::Result<()> { config.connection_port = Some(connection_port); config.pairing_code = Some(pairing_code); config.pairing_port = Some(pairing_port); - config.save(); + config.save()?; outro("Pairing and connecting to phone...") } From f82af13f6f6a1a39098d53c93e77d5c3bafaf9d4 Mon Sep 17 00:00:00 2001 From: kraktus Date: Thu, 7 Dec 2023 18:51:18 +0100 Subject: [PATCH 6/8] refactor the input password dialog --- command/src/main.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index 2538ce66..b1f2efbf 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -171,6 +171,16 @@ fn main() -> std::io::Result<()> { } } +fn pwd_input(user_type: &str) -> std::io::Result { + input(format!( + "Choose a password for {user_type} users (blank for 'password')" + )) + .placeholder("password") + .default_input("password") + .required(false) + .interact() +} + fn setup(mut config: Config) -> std::io::Result<()> { intro(BANNER)?; @@ -182,18 +192,7 @@ fn setup(mut config: Config) -> std::io::Result<()> { .interact()?; let (su_password, password) = if setup_database { - ( - input("Choose a password for admin users (blank for 'password')") - .placeholder("password") - .default_input("password") - .required(false) - .interact()?, - input("Choose a password for regular users (blank for 'password')") - .placeholder("password") - .default_input("password") - .required(false) - .interact()?, - ) + (pwd_input("admin")?, pwd_input("regular")?) } else { (String::new(), String::new()) }; From c76c7631208dd09c18b6c112672ab4fedb15699c Mon Sep 17 00:00:00 2001 From: Trevor Fitzgerald Date: Thu, 7 Dec 2023 17:17:24 -0500 Subject: [PATCH 7/8] clippy resolve --- command/src/main.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/command/src/main.rs b/command/src/main.rs index b1f2efbf..73be6586 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -54,9 +54,10 @@ impl Config { const SETTINGS_ENV: &'static str = "settings.env"; fn load() -> Self { - std::fs::read_to_string(Self::SETTINGS_TOML) - .map(|toml_str| toml::from_str(&toml_str).unwrap()) - .unwrap_or_else(|_| Self::default()) + std::fs::read_to_string(Self::SETTINGS_TOML).map_or_else( + |_| Self::default(), + |contents| toml::from_str(&contents).unwrap_or_default(), + ) } fn save(&self) -> std::io::Result<()> { From 97256b20cafb2534b1fc396fe1882102f3d2e4c6 Mon Sep 17 00:00:00 2001 From: Trevor Fitzgerald Date: Thu, 7 Dec 2023 17:31:46 -0500 Subject: [PATCH 8/8] error on invalid toml --- command/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/src/main.rs b/command/src/main.rs index 73be6586..1e3747a2 100644 --- a/command/src/main.rs +++ b/command/src/main.rs @@ -56,7 +56,7 @@ impl Config { fn load() -> Self { std::fs::read_to_string(Self::SETTINGS_TOML).map_or_else( |_| Self::default(), - |contents| toml::from_str(&contents).unwrap_or_default(), + |contents| toml::from_str(&contents).unwrap(), ) }