From a468827f02c5243b9bd4e0183fed16854ae6c851 Mon Sep 17 00:00:00 2001 From: Jack Wills <32690432+mrjackwills@users.noreply.github.com> Date: Wed, 16 Apr 2025 15:16:16 +0000 Subject: [PATCH] fix: config merging Embarrassingly, settings from the newly implemented config file where not being correctly applied. This *should* now be fixed. Change the show standard error default to true --- example_config/example.config.jsonc | 2 +- example_config/example.config.toml | 2 +- src/app_data/container_state.rs | 2 +- src/config/config.toml | 2 +- src/config/mod.rs | 50 +++++++++++++++++++++-------- src/config/parse_args.rs | 19 +++++++++++ src/config/parse_config_file.rs | 26 ++------------- src/ui/draw_blocks/headers.rs | 4 +-- 8 files changed, 63 insertions(+), 44 deletions(-) diff --git a/example_config/example.config.jsonc b/example_config/example.config.jsonc index e3eeef1..f62f643 100644 --- a/example_config/example.config.jsonc +++ b/example_config/example.config.jsonc @@ -13,7 +13,7 @@ // Show self (the oxker container) when running as a docker container "show_self": false, // Show std_err in logs - "show_std_err": false, + "show_std_err": true, // Show a timestamp for every log entry "show_timestamp": true, // Don't draw gui - for debugging - mostly pointless diff --git a/example_config/example.config.toml b/example_config/example.config.toml index 8d9180a..b8d4534 100644 --- a/example_config/example.config.toml +++ b/example_config/example.config.toml @@ -16,7 +16,7 @@ raw_logs = false show_self = false # Show std_err in logs -show_std_err = false +show_std_err = true # Show a timestamp for every log entry show_timestamp = true diff --git a/src/app_data/container_state.rs b/src/app_data/container_state.rs index cd46865..679cb96 100644 --- a/src/app_data/container_state.rs +++ b/src/app_data/container_state.rs @@ -36,7 +36,7 @@ impl ContainerId { /// Only return first 8 chars of id, is usually more than enough for uniqueness /// TODO container id is a hex string, so can assume that 0..=8 will always return a 8 char ascii &str - /// need to update tests to use real ids, or atleast strings of the correct-ish length + /// need to update tests to use real ids, or atleast strings of the correct-ish length pub fn get_short(&self) -> String { self.0.chars().take(8).collect::() } diff --git a/src/config/config.toml b/src/config/config.toml index 8d9180a..b8d4534 100644 --- a/src/config/config.toml +++ b/src/config/config.toml @@ -16,7 +16,7 @@ raw_logs = false show_self = false # Show std_err in logs -show_std_err = false +show_std_err = true # Show a timestamp for every log entry show_timestamp = true diff --git a/src/config/mod.rs b/src/config/mod.rs index f1d306e..9a31823 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -126,19 +126,44 @@ impl Config { } /// Combine config from CLI into config file, the cli take priority - /// make sure color_logs and raw_logs can't clash + /// and also make sure color_logs and raw_logs can't clash fn merge_args(mut self, config_from_cli: Self) -> Self { - self.color_logs = config_from_cli.color_logs; - self.docker_interval_ms = config_from_cli.docker_interval_ms; - self.gui = config_from_cli.gui; - self.raw_logs = config_from_cli.raw_logs; - self.show_self = config_from_cli.show_self; - self.show_std_err = config_from_cli.show_std_err; - self.show_timestamp = config_from_cli.show_timestamp; - self.use_cli = config_from_cli.use_cli; + let default_args = Args::default(); + + if config_from_cli.color_logs != default_args.color { + self.color_logs = config_from_cli.color_logs; + } + + if config_from_cli.gui != default_args.gui { + self.gui = config_from_cli.gui; + } + + if config_from_cli.docker_interval_ms != default_args.docker_interval { + self.docker_interval_ms = config_from_cli.docker_interval_ms; + } if config_from_cli.docker_interval_ms < 1000 { - self.docker_interval_ms = 1000; + self.docker_interval_ms = default_args.docker_interval; + } + + if config_from_cli.raw_logs != default_args.raw { + self.raw_logs = config_from_cli.raw_logs; + } + + if config_from_cli.show_self != default_args.show_self { + self.show_self = config_from_cli.show_self; + } + + if config_from_cli.show_std_err != default_args.no_std_err { + self.show_std_err = config_from_cli.show_std_err; + } + + if config_from_cli.show_timestamp != default_args.timestamp { + self.show_timestamp = config_from_cli.show_timestamp; + } + + if config_from_cli.use_cli != default_args.use_cli { + self.use_cli = config_from_cli.use_cli; } if let Some(host) = config_from_cli.host { @@ -153,10 +178,7 @@ impl Config { self.timezone = Some(tz); } - if config_from_cli.raw_logs { - self.color_logs = false; - } - if config_from_cli.color_logs { + if self.color_logs && self.raw_logs { self.raw_logs = false; } self diff --git a/src/config/parse_args.rs b/src/config/parse_args.rs index c8cedc2..c79434f 100644 --- a/src/config/parse_args.rs +++ b/src/config/parse_args.rs @@ -53,3 +53,22 @@ pub struct Args { #[clap(long="use-cli", short = None)] pub use_cli: bool, } + +impl Default for Args { + fn default() -> Self { + Self { + docker_interval: 1000, + timestamp: true, + color: false, + raw: false, + show_self: false, + gui: true, + host: None, + no_std_err: true, + timezone: None, + save_dir: None, + config_file: None, + use_cli: false, + } + } +} diff --git a/src/config/parse_config_file.rs b/src/config/parse_config_file.rs index 2af4a46..e536e3e 100644 --- a/src/config/parse_config_file.rs +++ b/src/config/parse_config_file.rs @@ -133,33 +133,13 @@ impl ConfigFile { Self::parse(file_type, &input) } - /// Resolve conflict in the args, this is handled automatically by Clap, basically just by rejecting it - /// But here we can just change the options - although maybe should be also reject to follow the same behaviour as Clap? - /// TODO I think this is duplicated with the merge_args fn - fn resolve_conflict(&mut self) { - if let Some(color) = self.color_logs.as_ref() { - if *color { - self.raw_logs = Some(false); - } - } - if let Some(interval) = self.docker_interval.as_ref() { - if interval < &1000 { - self.docker_interval = Some(1000); - } - } - } - /// Try to parse the config file when the path is user supplied via cliargs pub fn try_parse_from_file(path: &str) -> Option { let path = PathBuf::from(path); let Ok(file_type) = ConfigFileType::try_from(&path) else { return None; }; - - Self::parse_config_file(file_type, &path).map_or(None, |mut config_file| { - config_file.resolve_conflict(); - Some(config_file) - }) + Self::parse_config_file(file_type, &path).ok() } /// Parse a config file using default config_file location @@ -172,11 +152,9 @@ impl ConfigFile { ConfigFileType::JsoncAsJson, ConfigFileType::Json, ] { - if let Ok(mut config_file) = + if let Ok(config_file) = Self::parse_config_file(file_type, &file_type.get_default_path_name(in_container)) { - Self::resolve_conflict(&mut config_file); - config = Some(config_file); break; } diff --git a/src/ui/draw_blocks/headers.rs b/src/ui/draw_blocks/headers.rs index 6483109..792ccf4 100644 --- a/src/ui/draw_blocks/headers.rs +++ b/src/ui/draw_blocks/headers.rs @@ -475,8 +475,8 @@ mod tests { assert_snapshot!(setup.terminal.backend()); } - - #[test] + + #[test] /// Custom keymap for help panel is correctly display, two definitions fn test_draw_blocks_headers_custom_keymap_two_definitions() { let mut setup = test_setup(140, 1, true, true);