Fix replacing optional fields

This fixes an issue with the default `SerdeReplace` implementation where
it would never recurse through options but always replace the entire
option with the new value.

Closes #7518.

CHANGELOG.md

@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - IME input lagging behind on X11
 - xdotool modifiers input not working correctly on X11
 - Parsing numbers fails for mouse bindings
+- Some config options overriding each other in CLI/IPC
 
 ## 0.13.0
 

Cargo.lock

@@ -70,6 +70,7 @@ dependencies = [
 name = "alacritty_config"
 version = "0.1.2"
 dependencies = [
+ "alacritty_config_derive",
  "log",
  "serde",
  "toml",

alacritty/src/config/debug.rs

@@ -1,7 +1,5 @@
 use log::LevelFilter;
 
-use serde::Deserialize;
-
 use alacritty_config_derive::ConfigDeserialize;
 
 /// Debugging options.
@@ -47,17 +45,17 @@ impl Default for Debug {
 }
 
 /// The renderer configuration options.
-#[derive(Deserialize, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(ConfigDeserialize, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub enum RendererPreference {
     /// OpenGL 3.3 renderer.
-    #[serde(rename = "glsl3")]
+    #[config(rename = "glsl3")]
     Glsl3,
 
     /// GLES 2 renderer, with optional extensions like dual source blending.
-    #[serde(rename = "gles2")]
+    #[config(rename = "gles2")]
     Gles2,
 
     /// Pure GLES 2 renderer.
-    #[serde(rename = "gles2_pure")]
+    #[config(rename = "gles2_pure")]
     Gles2Pure,
 }

alacritty/src/config/ui_config.rs

@@ -1,9 +1,11 @@
 use std::cell::RefCell;
 use std::collections::HashMap;
+use std::error::Error;
 use std::fmt::{self, Formatter};
 use std::path::PathBuf;
 use std::rc::Rc;
 
+use alacritty_config::SerdeReplace;
 use alacritty_terminal::term::Config as TermConfig;
 use alacritty_terminal::tty::{Options as PtyOptions, Shell};
 use log::{error, warn};
@@ -656,6 +658,14 @@ impl From<Program> for Shell {
     }
 }
 
+impl SerdeReplace for Program {
+    fn replace(&mut self, value: toml::Value) -> Result<(), Box<dyn Error>> {
+        *self = Self::deserialize(value)?;
+
+        Ok(())
+    }
+}
+
 pub(crate) struct StringVisitor;
 impl<'de> serde::de::Visitor<'de> for StringVisitor {
     type Value = String;

alacritty/src/config/window.rs

@@ -3,10 +3,10 @@ use std::fmt::{self, Formatter};
 use log::{error, warn};
 use serde::de::{self, MapAccess, Visitor};
 use serde::{Deserialize, Deserializer, Serialize};
-use winit::window::{Fullscreen, Theme};
 
 #[cfg(target_os = "macos")]
 use winit::platform::macos::OptionAsAlt as WinitOptionAsAlt;
+use winit::window::{Fullscreen, Theme as WinitTheme};
 
 use alacritty_config_derive::{ConfigDeserialize, SerdeReplace};
 
@@ -31,9 +31,6 @@ pub struct WindowConfig {
     #[config(skip)]
     pub embed: Option<u32>,
 
-    /// System decorations theme variant.
-    pub decorations_theme_variant: Option<Theme>,
-
     /// Spread out additional padding evenly.
     pub dynamic_padding: bool,
 
@@ -62,6 +59,9 @@ pub struct WindowConfig {
 
     /// Initial dimensions.
     dimensions: Dimensions,
+
+    /// System decorations theme variant.
+    decorations_theme_variant: Option<Theme>,
 }
 
 impl Default for WindowConfig {
@@ -149,6 +149,10 @@ impl WindowConfig {
             OptionAsAlt::None => WinitOptionAsAlt::None,
         }
     }
+
+    pub fn theme(&self) -> Option<WinitTheme> {
+        self.decorations_theme_variant.map(WinitTheme::from)
+    }
 }
 
 #[derive(ConfigDeserialize, Debug, Clone, PartialEq, Eq)]
@@ -292,3 +296,19 @@ pub enum OptionAsAlt {
     #[default]
     None,
 }
+
+/// System decorations theme variant.
+#[derive(ConfigDeserialize, Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Theme {
+    Light,
+    Dark,
+}
+
+impl From<Theme> for WinitTheme {
+    fn from(theme: Theme) -> Self {
+        match theme {
+            Theme::Light => WinitTheme::Light,
+            Theme::Dark => WinitTheme::Dark,
+        }
+    }
+}

alacritty/src/display/window.rs

@@ -165,7 +165,7 @@ impl Window {
 
         let window = window_builder
             .with_title(&identity.title)
-            .with_theme(config.window.decorations_theme_variant)
+            .with_theme(config.window.theme())
             .with_visible(false)
             .with_transparent(true)
             .with_blur(config.window.blur)

alacritty/src/window_context.rs

@@ -289,7 +289,7 @@ impl WindowContext {
         }
 
         // Always reload the theme to account for auto-theme switching.
-        self.display.window.set_theme(self.config.window.decorations_theme_variant);
+        self.display.window.set_theme(self.config.window.theme());
 
         // Update display if either padding options or resize increments were changed.
         let window_config = &old_config.window;

alacritty_config/Cargo.toml

@@ -12,3 +12,7 @@ rust-version = "1.70.0"
 log = { version = "0.4.17", features = ["serde"] }
 serde = "1.0.163"
 toml = "0.8.2"
+
+[dev-dependencies]
+alacritty_config_derive = { path = "../alacritty_config_derive" }
+serde = { version = "1.0.163", features = ["derive"] }

alacritty_config/src/lib.rs

@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 use std::error::Error;
+use std::path::PathBuf;
 
 use log::LevelFilter;
 use serde::Deserialize;
@@ -9,6 +10,7 @@ pub trait SerdeReplace {
     fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>>;
 }
 
+#[macro_export]
 macro_rules! impl_replace {
     ($($ty:ty),*$(,)*) => {
         $(
@@ -29,6 +31,7 @@ impl_replace!(
     bool,
     char,
     String,
+    PathBuf,
     LevelFilter,
 );
 
@@ -47,9 +50,12 @@ impl<'de, T: Deserialize<'de>> SerdeReplace for Vec<T> {
     }
 }
 
-impl<'de, T: Deserialize<'de>> SerdeReplace for Option<T> {
+impl<'de, T: SerdeReplace + Deserialize<'de>> SerdeReplace for Option<T> {
     fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>> {
-        replace_simple(self, value)
+        match self {
+            Some(inner) => inner.replace(value),
+            None => replace_simple(self, value),
+        }
     }
 }
 
@@ -58,3 +64,30 @@ impl<'de, T: Deserialize<'de>> SerdeReplace for HashMap<String, T> {
         replace_simple(self, value)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate as alacritty_config;
+    use alacritty_config_derive::ConfigDeserialize;
+
+    #[test]
+    fn replace_option() {
+        #[derive(ConfigDeserialize, Default, PartialEq, Eq, Debug)]
+        struct ReplaceOption {
+            a: usize,
+            b: usize,
+        }
+
+        let mut subject: Option<ReplaceOption> = None;
+
+        let value: Value = toml::from_str("a=1").unwrap();
+        SerdeReplace::replace(&mut subject, value).unwrap();
+
+        let value: Value = toml::from_str("b=2").unwrap();
+        SerdeReplace::replace(&mut subject, value).unwrap();
+
+        assert_eq!(subject, Some(ReplaceOption { a: 1, b: 2 }));
+    }
+}