Skip to content

Commit

Permalink
Refactor toml::Value->Theme conversion
Browse files Browse the repository at this point in the history
The `From<Value>` implementation for `Theme` converted the Value to a
string and re-parsed the string to convert it to
`HashMap<String, Value>` which feels a bit wasteful. This change uses
the underlying `toml::map::Map` directly when the value is a table and
warns about the unexpected `Value` shape otherwise.

This is necessary because toml 0.6.0 changes the Display implementation
for Value::Table so that the `to_string` no longer encodes the value as
a Document, just a Value. So the parse of the Value fails to be decoded
as a HashMap.

The behavior for returning `Default::default` matches the previous
code's behavior except that it did not warn when the input Value was
failed to parse.
  • Loading branch information
the-mikedavis committed Jan 24, 2023
1 parent b3e9f62 commit 70887b7
Showing 1 changed file with 39 additions and 39 deletions.
78 changes: 39 additions & 39 deletions helix-view/src/theme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
str,
};

use anyhow::{anyhow, Context, Result};
use anyhow::{anyhow, Result};
use helix_core::hashmap;
use helix_loader::merge_toml_values;
use log::warn;
Expand Down Expand Up @@ -209,16 +209,18 @@ pub struct Theme {

impl From<Value> for Theme {
fn from(value: Value) -> Self {
let values: Result<HashMap<String, Value>> =
toml::from_str(&value.to_string()).context("Failed to load theme");

let (styles, scopes, highlights) = build_theme_values(values);

Self {
styles,
scopes,
highlights,
..Default::default()
if let Value::Table(table) = value {
let (styles, scopes, highlights) = build_theme_values(table);

Self {
styles,
scopes,
highlights,
..Default::default()
}
} else {
warn!("Expected theme TOML value to be a table, found {:?}", value);
Default::default()
}
}
}
Expand All @@ -228,9 +230,9 @@ impl<'de> Deserialize<'de> for Theme {
where
D: Deserializer<'de>,
{
let values = HashMap::<String, Value>::deserialize(deserializer)?;
let values = Map::<String, Value>::deserialize(deserializer)?;

let (styles, scopes, highlights) = build_theme_values(Ok(values));
let (styles, scopes, highlights) = build_theme_values(values);

Ok(Self {
styles,
Expand All @@ -242,39 +244,37 @@ impl<'de> Deserialize<'de> for Theme {
}

fn build_theme_values(
values: Result<HashMap<String, Value>>,
mut values: Map<String, Value>,
) -> (HashMap<String, Style>, Vec<String>, Vec<Style>) {
let mut styles = HashMap::new();
let mut scopes = Vec::new();
let mut highlights = Vec::new();

if let Ok(mut colors) = values {
// TODO: alert user of parsing failures in editor
let palette = colors
.remove("palette")
.map(|value| {
ThemePalette::try_from(value).unwrap_or_else(|err| {
warn!("{}", err);
ThemePalette::default()
})
})
.unwrap_or_default();
// remove inherits from value to prevent errors
let _ = colors.remove("inherits");
styles.reserve(colors.len());
scopes.reserve(colors.len());
highlights.reserve(colors.len());
for (name, style_value) in colors {
let mut style = Style::default();
if let Err(err) = palette.parse_style(&mut style, style_value) {
// TODO: alert user of parsing failures in editor
let palette = values
.remove("palette")
.map(|value| {
ThemePalette::try_from(value).unwrap_or_else(|err| {
warn!("{}", err);
}

// these are used both as UI and as highlights
styles.insert(name.clone(), style);
scopes.push(name);
highlights.push(style);
ThemePalette::default()
})
})
.unwrap_or_default();
// remove inherits from value to prevent errors
let _ = values.remove("inherits");
styles.reserve(values.len());
scopes.reserve(values.len());
highlights.reserve(values.len());
for (name, style_value) in values {
let mut style = Style::default();
if let Err(err) = palette.parse_style(&mut style, style_value) {
warn!("{}", err);
}

// these are used both as UI and as highlights
styles.insert(name.clone(), style);
scopes.push(name);
highlights.push(style);
}

(styles, scopes, highlights)
Expand Down

0 comments on commit 70887b7

Please sign in to comment.