From 5685ce8bf8cb919f454518f1206b7ebc52636378 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Tue, 2 Jan 2024 14:34:57 +0100 Subject: 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 | 1 + Cargo.lock | 1 + alacritty/src/config/debug.rs | 10 ++++------ alacritty/src/config/ui_config.rs | 10 ++++++++++ alacritty/src/config/window.rs | 28 ++++++++++++++++++++++++---- alacritty/src/display/window.rs | 2 +- alacritty/src/window_context.rs | 2 +- alacritty_config/Cargo.toml | 4 ++++ alacritty_config/src/lib.rs | 37 +++++++++++++++++++++++++++++++++++-- 9 files changed, 81 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49dddcce..5c443c42 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 adfbb411..5c094025 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 a8be77d9..15c06454 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 f4f67cb6..21059734 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 for Shell { } } +impl SerdeReplace for Program { + fn replace(&mut self, value: toml::Value) -> Result<(), Box> { + *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 3ae4e29e..ed84622e 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, - /// System decorations theme variant. - pub decorations_theme_variant: Option, - /// 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, } impl Default for WindowConfig { @@ -149,6 +149,10 @@ impl WindowConfig { OptionAsAlt::None => WinitOptionAsAlt::None, } } + + pub fn theme(&self) -> Option { + 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 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 f36d05e9..e4bfa2cb 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 c87c2210..3f3e6807 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 985da0e3..28875c3b 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 c5a43b87..81e43bb8 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>; } +#[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 { } } -impl<'de, T: Deserialize<'de>> SerdeReplace for Option { +impl<'de, T: SerdeReplace + Deserialize<'de>> SerdeReplace for Option { fn replace(&mut self, value: Value) -> Result<(), Box> { - 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 { 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 = 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 })); + } +} -- cgit v1.2.3-54-g00ecf