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 107c872 commit 8668a4b
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 12 deletions.
51 changes: 47 additions & 4 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
5 changes: 5 additions & 0 deletions alacritty_config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ rust-version = "1.70.0"
log = { version = "0.4.17", features = ["serde"] }
serde = "1.0.163"
toml = "0.8.2"
winit = { version = "0.29.7", features = ["serde"] }

[dev-dependencies]
alacritty_config_derive = { path = "../alacritty_config_derive" }
serde = { version = "1.0.163", features = ["derive"] }
39 changes: 37 additions & 2 deletions alacritty_config/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
use std::collections::HashMap;
use std::error::Error;
use std::path::PathBuf;

use log::LevelFilter;
use winit::platform::wayland::Theme;
use serde::Deserialize;
use toml::Value;

pub trait SerdeReplace {
fn replace(&mut self, value: Value) -> Result<(), Box<dyn Error>>;
}

#[macro_export]
macro_rules! impl_replace {
($($ty:ty),*$(,)*) => {
$(
Expand All @@ -29,7 +32,9 @@ impl_replace!(
bool,
char,
String,
PathBuf,
LevelFilter,
Theme,
);

fn replace_simple<'de, D>(data: &mut D, value: Value) -> Result<(), Box<dyn Error>>
Expand All @@ -47,9 +52,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 +66,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 }));
}
}
23 changes: 23 additions & 0 deletions alacritty_config_derive/tests/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct Test {
enom_error: TestEnum,
#[config(removed = "it's gone")]
gone: bool,
option_struct: Option<Test4>,
}

impl Default for Test {
Expand All @@ -53,6 +54,7 @@ impl Default for Test {
enom_big: TestEnum::default(),
enom_error: TestEnum::default(),
gone: false,
option_struct: None,
}
}
}
Expand All @@ -76,6 +78,12 @@ struct Test3 {
#[derive(SerdeReplace, Deserialize, Default, PartialEq, Eq, Debug)]
struct NewType(usize);

#[derive(ConfigDeserialize, Default)]
struct Test4 {
a: usize,
b: usize,
}

#[test]
fn config_deserialize() {
let logger = unsafe {
Expand Down Expand Up @@ -198,3 +206,18 @@ fn replace_flatten() {

assert_eq!(test.flatten.flatty, 7);
}

#[test]
fn replace_option_multiple() {
let mut test = Test::default();

let value = toml::from_str("option_struct.a=1").unwrap();
test.replace(value).unwrap();

let value = toml::from_str("option_struct.b=2").unwrap();
test.replace(value).unwrap();

let option_struct = test.option_struct.as_ref().unwrap();
assert_eq!(option_struct.a, 1);
assert_eq!(option_struct.b, 2);
}

0 comments on commit 8668a4b

Please sign in to comment.