Repository: alacritty/alacritty (language: rust)

Issue:
Setting window position from CLI works only for one axis
Upgrading from alacritty 0.12 I found myself unable to set *both* `x` and `y` positions from CLI, but I can set either *one*. Examples:

```
# This command applies only `x=444` option and ignores `y=333` option:
alacritty -o window.position.y=333 window.position.x=444

# This command applies only `y=333` option and ignores `x=444` option:
alacritty -o window.position.x=444 window.position.y=333
```
It feels like a latter position drops the former one.

Interestingly, if I set both `x` and `y` in the alacritty.toml and run alacritty with neither `x` nor `y` specified from the CLI, then both positions get applied correctly.

https://github.com/alacritty/alacritty/issues/7267 is possibly related.

### System

OS: Debian
Version: alacritty 0.13.0
Linux: X11, BSPWM



Discussion / hints:
Can you try `"window.position={ x = 333, y = 444 }"`?
It works fine that way, thank you.

Since position is a `Delta` it's deserialized directly and doesn't benefit from our abstractions which allow overriding. Not a huge deal, but might be nice to look into a workaround. Since it's defined in Alacritty it should be fixable without too many issues.
~~I'm afraid I was too fast to report it working. The method you suggested works fine when I run it in a shell indeed, but when I try to run it via my hotkey daemon (`sxhkd`, namely), neither `x`, nor `y` position don't work anymore (no matter if set both at once, or each separately).~~
Sorry, it was mistake on my side (had to escape the `{}` symbols), the way you suggested works fine.

>I'm afraid I was too fast to report it working. The method you suggested works fine when I run it in a shell indeed, but when I try to run it via my hotkey daemon (sxhkd, namely), neither x, nor y position don't work anymore (no matter if set both at once, or each separately).

that's different issue though if they don't work at all.
>Since position is a Delta it's deserialized directly and doesn't benefit from our abstractions which allow overriding. Not a huge deal, but might be nice to look into a workaround. Since it's defined in Alacritty it should be fixable without too many issues.

maybe we can accumulate cli options and then apply the end union of all that was passed at once?
> maybe we can accumulate cli options and then apply the end union of all that was passed at once?

That would probably work as a backup. But even incrementally these shouldn't override each other. Otherwise we might not support overriding just one when they're defined in the config.
I mean, union is what it's all about? because same options later should purge previous, but different will just merge.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 49dddcce206..5c443c42ff1 100644
--- a/CHANGELOG.md
+++ b/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
 
diff --git a/Cargo.lock b/Cargo.lock
index adfbb411e7f..5c094025f69 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -70,6 +70,7 @@ dependencies = [
 name = "alacritty_config"
 version = "0.1.3-dev"
 dependencies = [
+ "alacritty_config_derive",
  "log",
  "serde",
  "toml",
diff --git a/alacritty/src/config/debug.rs b/alacritty/src/config/debug.rs
index a8be77d9de9..15c06454397 100644
--- a/alacritty/src/config/debug.rs
+++ b/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,
 }
diff --git a/alacritty/src/config/ui_config.rs b/alacritty/src/config/ui_config.rs
index f4f67cb6360..21059734f55 100644
--- a/alacritty/src/config/ui_config.rs
+++ b/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;
diff --git a/alacritty/src/config/window.rs b/alacritty/src/config/window.rs
index 3ae4e29e0cd..ed84622ef99 100644
--- a/alacritty/src/config/window.rs
+++ b/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,
+        }
+    }
+}
diff --git a/alacritty/src/display/window.rs b/alacritty/src/display/window.rs
index f36d05e972d..e4bfa2cb845 100644
--- a/alacritty/src/display/window.rs
+++ b/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)
diff --git a/alacritty/src/window_context.rs b/alacritty/src/window_context.rs
index c87c22108ed..3f3e6807d6d 100644
--- a/alacritty/src/window_context.rs
+++ b/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;
diff --git a/alacritty_config/Cargo.toml b/alacritty_config/Cargo.toml
index 985da0e3c22..28875c3b950 100644
--- a/alacritty_config/Cargo.toml
+++ b/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"] }
diff --git a/alacritty_config/src/lib.rs b/alacritty_config/src/lib.rs
index c5a43b87250..81e43bb8066 100644
--- a/alacritty_config/src/lib.rs
+++ b/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 }));
+    }
+}


Test patch: