From 2d4b183fce8c9de0d9548f7087786bc919cae7b4 Mon Sep 17 00:00:00 2001 From: David Knaack Date: Sat, 26 Mar 2022 10:42:19 +0100 Subject: [PATCH] refactor: replace module_config_derive with serde (#3786) * refactor: replace module_config_derive with serde Changes include: * Removing `starship_module_config_derive` and replacing it with `serde::Deserialize` * Removing `RootModuleConfig::load_config`. While potentially useful, it was only used in tests. And it would require something like `serde::DeserializeSeed` which is not derived by serde. * Merging `RootModuleConfig` into `ModuleConfig` * Implementing a `ValueDeserializer` that holds a reference to a `toml::Value` in `serde_utils.rs` * Deserialization errors (invalid type) are now logged and include the current key and the struct names * Unknown keys are now considered an error. "Did you mean?"-messages are still possible * fix typo Co-authored-by: Matan Kushner Co-authored-by: Matan Kushner --- CONTRIBUTING.md | 4 +- Cargo.lock | 10 - Cargo.toml | 1 - src/config.rs | 380 ++++++++-------------- src/configs/aws.rs | 7 +- src/configs/azure.rs | 7 +- src/configs/battery.rs | 12 +- src/configs/buf.rs | 8 +- src/configs/c.rs | 8 +- src/configs/character.rs | 8 +- src/configs/cmake.rs | 8 +- src/configs/cmd_duration.rs | 8 +- src/configs/cobol.rs | 8 +- src/configs/conda.rs | 8 +- src/configs/container.rs | 8 +- src/configs/crystal.rs | 8 +- src/configs/custom.rs | 8 +- src/configs/dart.rs | 8 +- src/configs/deno.rs | 8 +- src/configs/directory.rs | 7 +- src/configs/docker_context.rs | 8 +- src/configs/dotnet.rs | 8 +- src/configs/elixir.rs | 8 +- src/configs/elm.rs | 8 +- src/configs/env_var.rs | 8 +- src/configs/erlang.rs | 8 +- src/configs/fill.rs | 8 +- src/configs/gcloud.rs | 7 +- src/configs/git_branch.rs | 8 +- src/configs/git_commit.rs | 8 +- src/configs/git_metrics.rs | 8 +- src/configs/git_state.rs | 8 +- src/configs/git_status.rs | 8 +- src/configs/go.rs | 8 +- src/configs/haskell.rs | 8 +- src/configs/helm.rs | 8 +- src/configs/hg_branch.rs | 8 +- src/configs/hostname.rs | 8 +- src/configs/java.rs | 8 +- src/configs/jobs.rs | 8 +- src/configs/julia.rs | 8 +- src/configs/kotlin.rs | 8 +- src/configs/kubernetes.rs | 8 +- src/configs/line_break.rs | 7 +- src/configs/localip.rs | 8 +- src/configs/lua.rs | 8 +- src/configs/memory_usage.rs | 8 +- src/configs/mod.rs | 184 +++++------ src/configs/nim.rs | 8 +- src/configs/nix_shell.rs | 8 +- src/configs/nodejs.rs | 8 +- src/configs/ocaml.rs | 8 +- src/configs/openstack.rs | 7 +- src/configs/package.rs | 8 +- src/configs/perl.rs | 8 +- src/configs/php.rs | 8 +- src/configs/pulumi.rs | 8 +- src/configs/purescript.rs | 8 +- src/configs/python.rs | 8 +- src/configs/red.rs | 8 +- src/configs/rlang.rs | 8 +- src/configs/ruby.rs | 8 +- src/configs/rust.rs | 8 +- src/configs/scala.rs | 8 +- src/configs/shell.rs | 8 +- src/configs/shlvl.rs | 8 +- src/configs/singularity.rs | 8 +- src/configs/starship_root.rs | 66 +--- src/configs/status.rs | 8 +- src/configs/sudo.rs | 8 +- src/configs/swift.rs | 8 +- src/configs/terraform.rs | 8 +- src/configs/time.rs | 8 +- src/configs/username.rs | 8 +- src/configs/v.rs | 8 +- src/configs/vagrant.rs | 8 +- src/configs/vcsh.rs | 8 +- src/configs/zig.rs | 8 +- src/configure.rs | 4 +- src/context.rs | 2 +- src/lib.rs | 1 + src/modules/aws.rs | 15 +- src/modules/azure.rs | 2 +- src/modules/battery.rs | 2 +- src/modules/buf.rs | 2 +- src/modules/c.rs | 2 +- src/modules/character.rs | 2 +- src/modules/cmake.rs | 2 +- src/modules/cmd_duration.rs | 2 +- src/modules/cobol.rs | 2 +- src/modules/conda.rs | 2 +- src/modules/container.rs | 2 +- src/modules/crystal.rs | 2 +- src/modules/custom.rs | 2 +- src/modules/dart.rs | 2 +- src/modules/deno.rs | 2 +- src/modules/directory.rs | 2 +- src/modules/docker_context.rs | 2 +- src/modules/dotnet.rs | 2 +- src/modules/elixir.rs | 2 +- src/modules/elm.rs | 2 +- src/modules/env_var.rs | 2 +- src/modules/erlang.rs | 2 +- src/modules/fill.rs | 2 +- src/modules/gcloud.rs | 2 +- src/modules/git_branch.rs | 2 +- src/modules/git_commit.rs | 2 +- src/modules/git_metrics.rs | 2 +- src/modules/git_state.rs | 2 +- src/modules/git_status.rs | 2 +- src/modules/golang.rs | 2 +- src/modules/haskell.rs | 2 +- src/modules/helm.rs | 2 +- src/modules/hg_branch.rs | 2 +- src/modules/hostname.rs | 2 +- src/modules/java.rs | 2 +- src/modules/jobs.rs | 2 +- src/modules/julia.rs | 2 +- src/modules/kotlin.rs | 2 +- src/modules/kubernetes.rs | 2 +- src/modules/localip.rs | 3 +- src/modules/lua.rs | 2 +- src/modules/memory_usage.rs | 2 +- src/modules/mod.rs | 2 +- src/modules/nim.rs | 2 +- src/modules/nix_shell.rs | 2 +- src/modules/nodejs.rs | 2 +- src/modules/ocaml.rs | 2 +- src/modules/openstack.rs | 2 +- src/modules/package.rs | 2 +- src/modules/perl.rs | 2 +- src/modules/php.rs | 2 +- src/modules/pulumi.rs | 2 +- src/modules/purescript.rs | 2 +- src/modules/python.rs | 2 +- src/modules/red.rs | 2 +- src/modules/rlang.rs | 2 +- src/modules/ruby.rs | 2 +- src/modules/rust.rs | 2 +- src/modules/scala.rs | 2 +- src/modules/shell.rs | 2 +- src/modules/shlvl.rs | 2 +- src/modules/singularity.rs | 2 +- src/modules/status.rs | 2 +- src/modules/sudo.rs | 2 +- src/modules/swift.rs | 2 +- src/modules/terraform.rs | 2 +- src/modules/time.rs | 2 +- src/modules/username.rs | 2 +- src/modules/vagrant.rs | 2 +- src/modules/vcsh.rs | 2 +- src/modules/vlang.rs | 2 +- src/modules/zig.rs | 2 +- src/serde_utils.rs | 385 +++++++++++++++++++++++ src/test/mod.rs | 2 +- starship_module_config_derive/Cargo.toml | 25 -- starship_module_config_derive/LICENSE | 15 - starship_module_config_derive/README.md | 1 - starship_module_config_derive/src/lib.rs | 90 ------ 159 files changed, 891 insertions(+), 1011 deletions(-) create mode 100644 src/serde_utils.rs delete mode 100644 starship_module_config_derive/Cargo.toml delete mode 100644 starship_module_config_derive/LICENSE delete mode 100644 starship_module_config_derive/README.md delete mode 100644 starship_module_config_derive/src/lib.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eb68a0e1d..2a635ef9c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,7 +53,7 @@ pub fn module<'a>(context: &'a Context) -> Option> { To run a external command (e.g. to get the version of a tool) and to allow for mocking use the `context.exec_cmd` function. Here's a quick example: ```rust -use super::{Context, Module, RootModuleConfig}; +use super::{Context, Module, ModuleConfig}; use crate::configs::php::PhpConfig; use crate::formatter::StringFormatter; @@ -163,7 +163,7 @@ Unit tests are written using the built-in Rust testing library in the same file All tests that test the rendered output of a module should use `ModuleRenderer`. For Example: ```rust -use super::{Context, Module, RootModuleConfig}; +use super::{Context, Module, ModuleConfig}; use crate::configs::php::PhpConfig; use crate::formatter::StringFormatter; diff --git a/Cargo.lock b/Cargo.lock index 1aedb44d1..9cd299eee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1583,7 +1583,6 @@ dependencies = [ "shadow-rs", "shell-words", "starship-battery", - "starship_module_config_derive", "strsim", "sys-info", "tempfile", @@ -1617,15 +1616,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "starship_module_config_derive" -version = "0.2.1" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "static_assertions" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index fde4eb2cd..a85762885 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,6 @@ shadow-rs = "0.11.0" # battery is optional (on by default) because the crate doesn't currently build for Termux # see: https://github.com/svartalf/rust-battery/issues/33 starship-battery = { version = "0.7.9", optional = true } -starship_module_config_derive = { version = "0.2.1", path = "starship_module_config_derive" } strsim = "0.10.0" sys-info = "0.9.1" terminal_size = "0.1.17" diff --git a/src/config.rs b/src/config.rs index bd2eae4ff..982fe688c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,199 +1,59 @@ +use crate::serde_utils::ValueDeserializer; use crate::utils; -use ansi_term::{Color, Style}; -use indexmap::IndexMap; -use serde::Serialize; +use ansi_term::Color; +use serde::{ + de::value::Error as ValueError, de::Error as SerdeError, Deserialize, Deserializer, Serialize, +}; +use std::borrow::Cow; use std::clone::Clone; -use std::collections::HashMap; use std::io::ErrorKind; -use std::marker::Sized; use std::env; use toml::Value; /// Root config of a module. -pub trait RootModuleConfig<'a> +pub trait ModuleConfig<'a, E> where - Self: ModuleConfig<'a> + Default, -{ - /// Load root module config from given Value and fill unset variables with default - /// values. - fn load(config: &'a Value) -> Self { - let mut out = Self::default(); - out.load_config(config); - out - } - - /// Helper function that will call RootModuleConfig::load(config) if config is Some, - /// or RootModuleConfig::new() if config is None. - fn try_load(config: Option<&'a Value>) -> Self { - if let Some(config) = config { - Self::load(config) - } else { - Self::default() - } - } -} - -impl<'a, T: ModuleConfig<'a> + Default> RootModuleConfig<'a> for T {} - -/// Parsable config. -pub trait ModuleConfig<'a> -where - Self: Sized + Clone, + Self: Default, + E: SerdeError, { /// Construct a `ModuleConfig` from a toml value. - fn from_config(_config: &'a Value) -> Option { - None - } + fn from_config(config: &'a Value) -> Result; - /// Merge `self` with config from a toml table. - fn load_config(&mut self, config: &'a Value) { - if let Some(value) = Self::from_config(config) { - let _ = std::mem::replace(self, value); - } - } -} - -// TODO: Add logging to default implementations -impl<'a> ModuleConfig<'a> for &'a str { - fn from_config(config: &'a Value) -> Option { - config.as_str() - } -} - -impl<'a> ModuleConfig<'a> for String { - fn from_config(config: &'a Value) -> Option { - config.as_str().map(std::borrow::ToOwned::to_owned) - } -} - -impl<'a> ModuleConfig<'a> for Style { - fn from_config(config: &Value) -> Option { - parse_style_string(config.as_str()?) - } -} - -impl<'a> ModuleConfig<'a> for bool { - fn from_config(config: &Value) -> Option { - config.as_bool() - } -} - -impl<'a> ModuleConfig<'a> for i64 { - fn from_config(config: &Value) -> Option { - config.as_integer() - } -} - -impl<'a> ModuleConfig<'a> for u64 { - fn from_config(config: &Value) -> Option { - match config { - Value::Integer(value) => { - // Converting i64 to u64 - if *value > 0 { - Some(*value as Self) - } else { - None - } + /// Loads the TOML value into the config. + /// Missing values are set to their default values. + /// On error, logs an error message. + fn load(config: &'a Value) -> Self { + match Self::from_config(config) { + Ok(config) => config, + Err(e) => { + log::warn!("Failed to load config value: {}", e); + Self::default() } - Value::String(value) => value.parse::().ok(), - _ => None, } } -} -impl<'a> ModuleConfig<'a> for f64 { - fn from_config(config: &Value) -> Option { - config.as_float() + /// Helper function that will call ModuleConfig::from_config(config) if config is Some, + /// or ModuleConfig::default() if config is None. + fn try_load(config: Option<&'a Value>) -> Self { + config.map(Self::load).unwrap_or_default() } } -impl<'a> ModuleConfig<'a> for u32 { - fn from_config(config: &Value) -> Option { - match config { - Value::Integer(value) => { - // Converting i64 to u32 - if *value > 0 && *value <= u32::MAX.into() { - Some(*value as Self) - } else { - None - } - } - Value::String(value) => value.parse::().ok(), - _ => None, - } +impl<'a, T: Deserialize<'a> + Default> ModuleConfig<'a, ValueError> for T { + /// Create ValueDeserializer wrapper and use it to call Deserialize::deserialize on it. + fn from_config(config: &'a Value) -> Result { + let deserializer = ValueDeserializer::new(config); + T::deserialize(deserializer) } } -impl<'a> ModuleConfig<'a> for usize { - fn from_config(config: &Value) -> Option { - match config { - Value::Integer(value) => { - if *value > 0 { - Some(*value as Self) - } else { - None - } - } - Value::String(value) => value.parse::().ok(), - _ => None, - } - } -} - -impl<'a, T> ModuleConfig<'a> for Vec -where - T: ModuleConfig<'a>, -{ - fn from_config(config: &'a Value) -> Option { - config - .as_array()? - .iter() - .map(|value| T::from_config(value)) - .collect() - } -} - -impl<'a, T, S: ::std::hash::BuildHasher + Default> ModuleConfig<'a> for HashMap -where - T: ModuleConfig<'a>, - S: Clone, -{ - fn from_config(config: &'a Value) -> Option { - let mut hm = Self::default(); - - for (x, y) in config.as_table()? { - hm.insert(x.clone(), T::from_config(y)?); - } - - Some(hm) - } -} - -impl<'a, T, S: ::std::hash::BuildHasher + Default> ModuleConfig<'a> for IndexMap -where - T: ModuleConfig<'a>, - S: Clone, -{ - fn from_config(config: &'a Value) -> Option { - let mut im = Self::default(); - - for (x, y) in config.as_table()? { - im.insert(x.clone(), T::from_config(y)?); - } - - Some(im) - } -} - -impl<'a, T> ModuleConfig<'a> for Option -where - T: ModuleConfig<'a> + Sized, -{ - fn from_config(config: &'a Value) -> Option { - Some(T::from_config(config)) - } +#[derive(Clone, Deserialize, Serialize)] +#[serde(untagged)] +pub enum Either { + First(A), + Second(B), } /// A wrapper around `Vec` that implements `ModuleConfig`, and either @@ -201,22 +61,19 @@ where #[derive(Clone, Default, Serialize)] pub struct VecOr(pub Vec); -impl<'a, T> ModuleConfig<'a> for VecOr +impl<'de, T> Deserialize<'de> for VecOr where - T: ModuleConfig<'a> + Sized, + T: Deserialize<'de>, { - fn from_config(config: &'a Value) -> Option { - if let Some(item) = T::from_config(config) { - return Some(Self(vec![item])); + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let either = Either::, T>::deserialize(deserializer)?; + match either { + Either::First(v) => Ok(VecOr(v)), + Either::Second(s) => Ok(VecOr(vec![s])), } - - let vec = config - .as_array()? - .iter() - .map(|value| T::from_config(value)) - .collect::>>()?; - - Some(Self(vec)) } } @@ -374,6 +231,16 @@ impl StarshipConfig { } } +/// Deserialize a style string in the starship format with serde +pub fn deserialize_style<'de, D>(de: D) -> Result +where + D: Deserializer<'de>, +{ + Cow::<'_, str>::deserialize(de).and_then(|s| { + parse_style_string(s.as_ref()).ok_or_else(|| D::Error::custom("Invalid style string")) + }) +} + /** Parse a style string which represents an ansi style. Valid tokens in the style string include the following: - 'fg:' (specifies that the color read should be a foreground color) @@ -501,11 +368,24 @@ fn parse_color_string(color_string: &str) -> Option { #[cfg(test)] mod tests { use super::*; - use starship_module_config_derive::ModuleConfig; + use ansi_term::Style; + + // Small wrapper to allow deserializing Style without a struct with #[serde(deserialize_with=)] + #[derive(Default, Clone, Debug, PartialEq)] + struct StyleWrapper(Style); + + impl<'de> Deserialize<'de> for StyleWrapper { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserialize_style(deserializer).map(Self) + } + } #[test] fn test_load_config() { - #[derive(Clone, Default, ModuleConfig)] + #[derive(Clone, Default, Deserialize)] struct TestConfig<'a> { pub symbol: &'a str, pub disabled: bool, @@ -517,12 +397,7 @@ mod tests { disabled = true some_array = ["A"] }; - let mut rust_config = TestConfig { - symbol: "S ", - disabled: false, - some_array: vec!["A", "B", "C"], - }; - rust_config.load_config(&config); + let rust_config = TestConfig::from_config(&config).unwrap(); assert_eq!(rust_config.symbol, "T "); assert!(rust_config.disabled); @@ -531,15 +406,20 @@ mod tests { #[test] fn test_load_nested_config() { - #[derive(Clone, Default, ModuleConfig)] + #[derive(Clone, Default, Deserialize)] + #[serde(default)] struct TestConfig<'a> { + #[serde(borrow)] pub untracked: SegmentDisplayConfig<'a>, + #[serde(borrow)] pub modified: SegmentDisplayConfig<'a>, } - #[derive(PartialEq, Debug, Clone, Default, ModuleConfig)] + #[derive(PartialEq, Debug, Clone, Default, Deserialize)] + #[serde(default)] struct SegmentDisplayConfig<'a> { pub value: &'a str, + #[serde(deserialize_with = "deserialize_style")] pub style: Style, } @@ -548,23 +428,13 @@ mod tests { modified = { value = "∙", style = "red" } }; - let mut git_status_config = TestConfig { - untracked: SegmentDisplayConfig { - value: "?", - style: Color::Red.bold(), - }, - modified: SegmentDisplayConfig { - value: "!", - style: Color::Red.bold(), - }, - }; - git_status_config.load_config(&config); + let git_status_config = TestConfig::from_config(&config).unwrap(); assert_eq!( git_status_config.untracked, SegmentDisplayConfig { value: "x", - style: Color::Red.bold(), + style: Style::default(), } ); assert_eq!( @@ -578,7 +448,8 @@ mod tests { #[test] fn test_load_optional_config() { - #[derive(Clone, Default, ModuleConfig)] + #[derive(Clone, Default, Deserialize)] + #[serde(default)] struct TestConfig<'a> { pub optional: Option<&'a str>, pub hidden: Option<&'a str>, @@ -587,11 +458,7 @@ mod tests { let config = toml::toml! { optional = "test" }; - let mut rust_config = TestConfig { - optional: None, - hidden: None, - }; - rust_config.load_config(&config); + let rust_config = TestConfig::from_config(&config).unwrap(); assert_eq!(rust_config.optional, Some("test")); assert_eq!(rust_config.hidden, None); @@ -599,7 +466,8 @@ mod tests { #[test] fn test_load_enum_config() { - #[derive(Clone, Default, ModuleConfig)] + #[derive(Clone, Default, Deserialize)] + #[serde(default)] struct TestConfig { pub switch_a: Switch, pub switch_b: Switch, @@ -612,19 +480,22 @@ mod tests { Off, } - impl Default for Switch { - fn default() -> Self { - Self::Off + impl<'de> Deserialize<'de> for Switch { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + match s.to_ascii_lowercase().as_str() { + "on" => Ok(Switch::On), + _ => Ok(Switch::Off), + } } } - impl<'a> ModuleConfig<'a> for Switch { - fn from_config(config: &'a Value) -> Option { - match config.as_str()? { - "on" => Some(Self::On), - "off" => Some(Self::Off), - _ => None, - } + impl Default for Switch { + fn default() -> Self { + Self::Off } } @@ -632,12 +503,7 @@ mod tests { switch_a = "on" switch_b = "any" }; - let mut rust_config = TestConfig { - switch_a: Switch::Off, - switch_b: Switch::Off, - switch_c: Switch::Off, - }; - rust_config.load_config(&config); + let rust_config = TestConfig::from_config(&config).unwrap(); assert_eq!(rust_config.switch_a, Switch::On); assert_eq!(rust_config.switch_b, Switch::Off); @@ -665,23 +531,26 @@ mod tests { #[test] fn test_from_style() { let config = Value::from("red bold"); - assert_eq!(