Skip to content

Commit

Permalink
Fix replacing optional fields
Browse files Browse the repository at this point in the history
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 alacritty#7518.
  • Loading branch information
chrisduerr committed Jan 2, 2024
1 parent 8f57367 commit 5685ce8
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions alacritty/src/config/debug.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use log::LevelFilter;

use serde::Deserialize;

use alacritty_config_derive::ConfigDeserialize;

/// Debugging options.
Expand Down Expand Up @@ -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,
}
10 changes: 10 additions & 0 deletions alacritty/src/config/ui_config.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 24 additions & 4 deletions alacritty/src/config/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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,

Expand Down Expand Up @@ -62,6 +59,9 @@ pub struct WindowConfig {

/// Initial dimensions.
dimensions: Dimensions,

/// System decorations theme variant.
decorations_theme_variant: Option<Theme>,
}

impl Default for WindowConfig {
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
}
}
}
2 changes: 1 addition & 1 deletion alacritty/src/display/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion alacritty/src/window_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions alacritty_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
37 changes: 35 additions & 2 deletions alacritty_config/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::error::Error;
use std::path::PathBuf;

use log::LevelFilter;
use serde::Deserialize;
Expand All @@ -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),*$(,)*) => {
$(
Expand All @@ -29,6 +31,7 @@ impl_replace!(
bool,
char,
String,
PathBuf,
LevelFilter,
);

Expand All @@ -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),
}
}
}

Expand All @@ -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 }));
}
}

0 comments on commit 5685ce8

Please sign in to comment.