From afffdbe612cb8b573016184400e0deeb0137ccb9 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Thu, 15 Jun 2023 10:59:12 +0200 Subject: [PATCH] Fix `alacritty msg config` toml replacement This fixes a regression introduced in bd49067 which broke the override of configuration file variables using `alacritty msg config`. To fix this the `replace` functionality was rewritten to behave more like the `serde_utils::merge` where entire values are inserted into the existing structure rather than separating the keys from the values. Fixes: bd49067 (Switch to TOML configuration format) --- alacritty/src/window_context.rs | 27 ++++----------- alacritty_config/src/lib.rs | 24 ++++++------- alacritty_config_derive/src/serde_replace.rs | 36 ++++++++++---------- alacritty_config_derive/tests/config.rs | 12 +++---- alacritty_terminal/src/ansi.rs | 7 ++-- 5 files changed, 43 insertions(+), 63 deletions(-) diff --git a/alacritty/src/window_context.rs b/alacritty/src/window_context.rs index a450c77d..502907ad 100644 --- a/alacritty/src/window_context.rs +++ b/alacritty/src/window_context.rs @@ -70,7 +70,7 @@ pub struct WindowContext { master_fd: RawFd, #[cfg(not(windows))] shell_pid: u32, - ipc_config: Vec<(String, toml::Value)>, + ipc_config: Vec, config: Rc, } @@ -273,13 +273,12 @@ impl WindowContext { // Apply each option, removing broken ones. let mut i = 0; while i < self.ipc_config.len() { - let (key, value) = &self.ipc_config[i]; - - match config.replace(key, value.clone()) { + let option = &self.ipc_config[i]; + match config.replace(option.clone()) { Err(err) => { error!( target: LOG_TARGET_IPC_CONFIG, - "Unable to override option '{}': {}", key, err + "Unable to override option '{}': {}", option, err ); self.ipc_config.swap_remove(i); }, @@ -367,21 +366,9 @@ impl WindowContext { self.ipc_config.clear(); } else { for option in &ipc_config.options { - // Separate config key/value. - let (key, value) = match option.split_once('=') { - Some(split) => split, - None => { - error!( - target: LOG_TARGET_IPC_CONFIG, - "'{}': IPC config option missing value", option - ); - continue; - }, - }; - - // Try and parse value as toml. - match toml::from_str(value) { - Ok(value) => self.ipc_config.push((key.to_owned(), value)), + // Try and parse option as toml. + match toml::from_str(option) { + Ok(value) => self.ipc_config.push(value), Err(err) => error!( target: LOG_TARGET_IPC_CONFIG, "'{}': Invalid IPC config value: {:?}", option, err diff --git a/alacritty_config/src/lib.rs b/alacritty_config/src/lib.rs index c9d73622..1e5282d7 100644 --- a/alacritty_config/src/lib.rs +++ b/alacritty_config/src/lib.rs @@ -6,15 +6,15 @@ use serde::Deserialize; use toml::Value; pub trait SerdeReplace { - fn replace(&mut self, key: &str, value: Value) -> Result<(), Box>; + fn replace(&mut self, value: Value) -> Result<(), Box>; } macro_rules! impl_replace { ($($ty:ty),*$(,)*) => { $( impl SerdeReplace for $ty { - fn replace(&mut self, key: &str, value: Value) -> Result<(), Box> { - replace_simple(self, key, value) + fn replace(&mut self, value: Value) -> Result<(), Box> { + replace_simple(self, value) } } )* @@ -35,33 +35,29 @@ impl_replace!( #[cfg(target_os = "macos")] impl_replace!(winit::platform::macos::OptionAsAlt,); -fn replace_simple<'de, D>(data: &mut D, key: &str, value: Value) -> Result<(), Box> +fn replace_simple<'de, D>(data: &mut D, value: Value) -> Result<(), Box> where D: Deserialize<'de>, { - if !key.is_empty() { - let error = format!("Fields \"{key}\" do not exist"); - return Err(error.into()); - } *data = D::deserialize(value)?; Ok(()) } impl<'de, T: Deserialize<'de>> SerdeReplace for Vec { - fn replace(&mut self, key: &str, value: Value) -> Result<(), Box> { - replace_simple(self, key, value) + fn replace(&mut self, value: Value) -> Result<(), Box> { + replace_simple(self, value) } } impl<'de, T: Deserialize<'de>> SerdeReplace for Option { - fn replace(&mut self, key: &str, value: Value) -> Result<(), Box> { - replace_simple(self, key, value) + fn replace(&mut self, value: Value) -> Result<(), Box> { + replace_simple(self, value) } } impl<'de, T: Deserialize<'de>> SerdeReplace for HashMap { - fn replace(&mut self, key: &str, value: Value) -> Result<(), Box> { - replace_simple(self, key, value) + fn replace(&mut self, value: Value) -> Result<(), Box> { + replace_simple(self, value) } } diff --git a/alacritty_config_derive/src/serde_replace.rs b/alacritty_config_derive/src/serde_replace.rs index 7ca5ca7c..0a703b4c 100644 --- a/alacritty_config_derive/src/serde_replace.rs +++ b/alacritty_config_derive/src/serde_replace.rs @@ -28,11 +28,7 @@ pub fn derive(input: TokenStream) -> TokenStream { pub fn derive_direct(ident: Ident, generics: Generics) -> TokenStream2 { quote! { impl <#generics> alacritty_config::SerdeReplace for #ident <#generics> { - fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box> { - if !key.is_empty() { - let error = format!("Fields \"{}\" do not exist", key); - return Err(error.into()); - } + fn replace(&mut self, value: toml::Value) -> Result<(), Box> { *self = serde::Deserialize::deserialize(value)?; Ok(()) @@ -53,19 +49,23 @@ pub fn derive_recursive( quote! { #[allow(clippy::extra_unused_lifetimes)] impl <'de, #constrained> alacritty_config::SerdeReplace for #ident <#unconstrained> { - fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box> { - if key.is_empty() { - *self = serde::Deserialize::deserialize(value)?; - return Ok(()); - } + fn replace(&mut self, value: toml::Value) -> Result<(), Box> { + match value.as_table() { + Some(table) => { + for (field, next_value) in table { + let next_value = next_value.clone(); + let value = value.clone(); - let (field, next_key) = key.split_once('.').unwrap_or((key, "")); - match field { - #replace_arms - _ => { - let error = format!("Field \"{}\" does not exist", field); - return Err(error.into()); + match field.as_str() { + #replace_arms + _ => { + let error = format!("Field \"{}\" does not exist", field); + return Err(error.into()); + }, + } + } }, + None => *self = serde::Deserialize::deserialize(value)?, } Ok(()) @@ -95,11 +95,11 @@ fn match_arms(fields: &Punctuated) -> TokenStream2 { return Error::new(ident.span(), MULTIPLE_FLATTEN_ERROR).to_compile_error(); } else if flatten { flattened_arm = Some(quote! { - _ => alacritty_config::SerdeReplace::replace(&mut self.#ident, key, value)?, + _ => alacritty_config::SerdeReplace::replace(&mut self.#ident, value)?, }); } else { stream.extend(quote! { - #literal => alacritty_config::SerdeReplace::replace(&mut self.#ident, next_key, value)?, + #literal => alacritty_config::SerdeReplace::replace(&mut self.#ident, next_value)?, }); } } diff --git a/alacritty_config_derive/tests/config.rs b/alacritty_config_derive/tests/config.rs index 15bc62a9..3130bcda 100644 --- a/alacritty_config_derive/tests/config.rs +++ b/alacritty_config_derive/tests/config.rs @@ -173,8 +173,8 @@ impl Log for Logger { fn field_replacement() { let mut test = Test::default(); - let value = toml::Value::Integer(13); - test.replace("nesting.field2", value).unwrap(); + let value = toml::from_str("nesting.field2=13").unwrap(); + test.replace(value).unwrap(); assert_eq!(test.nesting.field2, Some(13)); } @@ -183,8 +183,8 @@ fn field_replacement() { fn replace_derive() { let mut test = Test::default(); - let value = toml::Value::Integer(9); - test.replace("nesting.newtype", value).unwrap(); + let value = toml::from_str("nesting.newtype=9").unwrap(); + test.replace(value).unwrap(); assert_eq!(test.nesting.newtype, NewType(9)); } @@ -193,8 +193,8 @@ fn replace_derive() { fn replace_flatten() { let mut test = Test::default(); - let value = toml::Value::Integer(7); - test.replace("flatty", value).unwrap(); + let value = toml::from_str("flatty=7").unwrap(); + test.replace(value).unwrap(); assert_eq!(test.flatten.flatty, 7); } diff --git a/alacritty_terminal/src/ansi.rs b/alacritty_terminal/src/ansi.rs index 5a28bda5..694520bd 100644 --- a/alacritty_terminal/src/ansi.rs +++ b/alacritty_terminal/src/ansi.rs @@ -52,12 +52,9 @@ impl<'de> serde::Deserialize<'de> for CursorShapeShim { } impl alacritty_config::SerdeReplace for CursorShapeShim { - fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box> { - if !key.is_empty() { - return Err(format!("Fields \"{0}\" do not exist", key).into()); - } - + fn replace(&mut self, value: toml::Value) -> Result<(), Box> { *self = serde::Deserialize::deserialize(value)?; + Ok(()) } }