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)
This commit is contained in:
Christian Duerr 2023-06-15 10:59:12 +02:00 committed by GitHub
parent be03effdbe
commit afffdbe612
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 63 deletions

View File

@ -70,7 +70,7 @@ pub struct WindowContext {
master_fd: RawFd, master_fd: RawFd,
#[cfg(not(windows))] #[cfg(not(windows))]
shell_pid: u32, shell_pid: u32,
ipc_config: Vec<(String, toml::Value)>, ipc_config: Vec<toml::Value>,
config: Rc<UiConfig>, config: Rc<UiConfig>,
} }
@ -273,13 +273,12 @@ impl WindowContext {
// Apply each option, removing broken ones. // Apply each option, removing broken ones.
let mut i = 0; let mut i = 0;
while i < self.ipc_config.len() { while i < self.ipc_config.len() {
let (key, value) = &self.ipc_config[i]; let option = &self.ipc_config[i];
match config.replace(option.clone()) {
match config.replace(key, value.clone()) {
Err(err) => { Err(err) => {
error!( error!(
target: LOG_TARGET_IPC_CONFIG, target: LOG_TARGET_IPC_CONFIG,
"Unable to override option '{}': {}", key, err "Unable to override option '{}': {}", option, err
); );
self.ipc_config.swap_remove(i); self.ipc_config.swap_remove(i);
}, },
@ -367,21 +366,9 @@ impl WindowContext {
self.ipc_config.clear(); self.ipc_config.clear();
} else { } else {
for option in &ipc_config.options { for option in &ipc_config.options {
// Separate config key/value. // Try and parse option as toml.
let (key, value) = match option.split_once('=') { match toml::from_str(option) {
Some(split) => split, Ok(value) => self.ipc_config.push(value),
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)),
Err(err) => error!( Err(err) => error!(
target: LOG_TARGET_IPC_CONFIG, target: LOG_TARGET_IPC_CONFIG,
"'{}': Invalid IPC config value: {:?}", option, err "'{}': Invalid IPC config value: {:?}", option, err

View File

@ -6,15 +6,15 @@ use serde::Deserialize;
use toml::Value; use toml::Value;
pub trait SerdeReplace { pub trait SerdeReplace {
fn replace(&mut self, key: &str, value: Value) -> Result<(), Box<dyn Error>>; fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>>;
} }
macro_rules! impl_replace { macro_rules! impl_replace {
($($ty:ty),*$(,)*) => { ($($ty:ty),*$(,)*) => {
$( $(
impl SerdeReplace for $ty { impl SerdeReplace for $ty {
fn replace(&mut self, key: &str, value: Value) -> Result<(), Box<dyn Error>> { fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>> {
replace_simple(self, key, value) replace_simple(self, value)
} }
} }
)* )*
@ -35,33 +35,29 @@ impl_replace!(
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
impl_replace!(winit::platform::macos::OptionAsAlt,); impl_replace!(winit::platform::macos::OptionAsAlt,);
fn replace_simple<'de, D>(data: &mut D, key: &str, value: Value) -> Result<(), Box<dyn Error>> fn replace_simple<'de, D>(data: &mut D, value: Value) -> Result<(), Box<dyn Error>>
where where
D: Deserialize<'de>, D: Deserialize<'de>,
{ {
if !key.is_empty() {
let error = format!("Fields \"{key}\" do not exist");
return Err(error.into());
}
*data = D::deserialize(value)?; *data = D::deserialize(value)?;
Ok(()) Ok(())
} }
impl<'de, T: Deserialize<'de>> SerdeReplace for Vec<T> { impl<'de, T: Deserialize<'de>> SerdeReplace for Vec<T> {
fn replace(&mut self, key: &str, value: Value) -> Result<(), Box<dyn Error>> { fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>> {
replace_simple(self, key, value) replace_simple(self, value)
} }
} }
impl<'de, T: Deserialize<'de>> SerdeReplace for Option<T> { impl<'de, T: Deserialize<'de>> SerdeReplace for Option<T> {
fn replace(&mut self, key: &str, value: Value) -> Result<(), Box<dyn Error>> { fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>> {
replace_simple(self, key, value) replace_simple(self, value)
} }
} }
impl<'de, T: Deserialize<'de>> SerdeReplace for HashMap<String, T> { impl<'de, T: Deserialize<'de>> SerdeReplace for HashMap<String, T> {
fn replace(&mut self, key: &str, value: Value) -> Result<(), Box<dyn Error>> { fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>> {
replace_simple(self, key, value) replace_simple(self, value)
} }
} }

View File

@ -28,11 +28,7 @@ pub fn derive(input: TokenStream) -> TokenStream {
pub fn derive_direct(ident: Ident, generics: Generics) -> TokenStream2 { pub fn derive_direct(ident: Ident, generics: Generics) -> TokenStream2 {
quote! { quote! {
impl <#generics> alacritty_config::SerdeReplace for #ident <#generics> { impl <#generics> alacritty_config::SerdeReplace for #ident <#generics> {
fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> { fn replace(&mut self, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> {
if !key.is_empty() {
let error = format!("Fields \"{}\" do not exist", key);
return Err(error.into());
}
*self = serde::Deserialize::deserialize(value)?; *self = serde::Deserialize::deserialize(value)?;
Ok(()) Ok(())
@ -53,19 +49,23 @@ pub fn derive_recursive<T>(
quote! { quote! {
#[allow(clippy::extra_unused_lifetimes)] #[allow(clippy::extra_unused_lifetimes)]
impl <'de, #constrained> alacritty_config::SerdeReplace for #ident <#unconstrained> { impl <'de, #constrained> alacritty_config::SerdeReplace for #ident <#unconstrained> {
fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> { fn replace(&mut self, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> {
if key.is_empty() { match value.as_table() {
*self = serde::Deserialize::deserialize(value)?; Some(table) => {
return Ok(()); 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.as_str() {
match field { #replace_arms
#replace_arms _ => {
_ => { let error = format!("Field \"{}\" does not exist", field);
let error = format!("Field \"{}\" does not exist", field); return Err(error.into());
return Err(error.into()); },
}
}
}, },
None => *self = serde::Deserialize::deserialize(value)?,
} }
Ok(()) Ok(())
@ -95,11 +95,11 @@ fn match_arms<T>(fields: &Punctuated<Field, T>) -> TokenStream2 {
return Error::new(ident.span(), MULTIPLE_FLATTEN_ERROR).to_compile_error(); return Error::new(ident.span(), MULTIPLE_FLATTEN_ERROR).to_compile_error();
} else if flatten { } else if flatten {
flattened_arm = Some(quote! { flattened_arm = Some(quote! {
_ => alacritty_config::SerdeReplace::replace(&mut self.#ident, key, value)?, _ => alacritty_config::SerdeReplace::replace(&mut self.#ident, value)?,
}); });
} else { } else {
stream.extend(quote! { stream.extend(quote! {
#literal => alacritty_config::SerdeReplace::replace(&mut self.#ident, next_key, value)?, #literal => alacritty_config::SerdeReplace::replace(&mut self.#ident, next_value)?,
}); });
} }
} }

View File

@ -173,8 +173,8 @@ impl Log for Logger {
fn field_replacement() { fn field_replacement() {
let mut test = Test::default(); let mut test = Test::default();
let value = toml::Value::Integer(13); let value = toml::from_str("nesting.field2=13").unwrap();
test.replace("nesting.field2", value).unwrap(); test.replace(value).unwrap();
assert_eq!(test.nesting.field2, Some(13)); assert_eq!(test.nesting.field2, Some(13));
} }
@ -183,8 +183,8 @@ fn field_replacement() {
fn replace_derive() { fn replace_derive() {
let mut test = Test::default(); let mut test = Test::default();
let value = toml::Value::Integer(9); let value = toml::from_str("nesting.newtype=9").unwrap();
test.replace("nesting.newtype", value).unwrap(); test.replace(value).unwrap();
assert_eq!(test.nesting.newtype, NewType(9)); assert_eq!(test.nesting.newtype, NewType(9));
} }
@ -193,8 +193,8 @@ fn replace_derive() {
fn replace_flatten() { fn replace_flatten() {
let mut test = Test::default(); let mut test = Test::default();
let value = toml::Value::Integer(7); let value = toml::from_str("flatty=7").unwrap();
test.replace("flatty", value).unwrap(); test.replace(value).unwrap();
assert_eq!(test.flatten.flatty, 7); assert_eq!(test.flatten.flatty, 7);
} }

View File

@ -52,12 +52,9 @@ impl<'de> serde::Deserialize<'de> for CursorShapeShim {
} }
impl alacritty_config::SerdeReplace for CursorShapeShim { impl alacritty_config::SerdeReplace for CursorShapeShim {
fn replace(&mut self, key: &str, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> { fn replace(&mut self, value: toml::Value) -> Result<(), Box<dyn std::error::Error>> {
if !key.is_empty() {
return Err(format!("Fields \"{0}\" do not exist", key).into());
}
*self = serde::Deserialize::deserialize(value)?; *self = serde::Deserialize::deserialize(value)?;
Ok(()) Ok(())
} }
} }