on master help sign in
Refactor toml::Value->Theme conversion

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.

commit70887b7378d642226ad5102fbe601fecb225225c authorMichael Davis <mcarsondavis@gmail.com> dateTue 24 Jan 2023, 11:07 (4 years ago) parentb3e9f62
1 file changed+39−39
helix-view/src/theme.rs+39−39
helix-view/src/theme.rs+39−39
@@ -4,7 +4,7 @@ use std::{
44 str,
55};
66
7−use anyhow::{anyhow, Context, Result};
7+use anyhow::{anyhow, Result};
88use helix_core::hashmap;
99use helix_loader::merge_toml_values;
1010use log::warn;
@@ -209,16 +209,18 @@ pub struct Theme {
209209
210210impl From<Value> for Theme {
211211 fn from(value: Value) -> Self {
212− let values: Result<HashMap<String, Value>> =
213− toml::from_str(&value.to_string()).context("Failed to load theme");
214−
215− let (styles, scopes, highlights) = build_theme_values(values);
216−
217− Self {
218− styles,
219− scopes,
220− highlights,
221− ..Default::default()
212+ if let Value::Table(table) = value {
213+ let (styles, scopes, highlights) = build_theme_values(table);
214+
215+ Self {
216+ styles,
217+ scopes,
218+ highlights,
219+ ..Default::default()
220+ }
221+ } else {
222+ warn!("Expected theme TOML value to be a table, found {:?}", value);
223+ Default::default()
222224 }
223225 }
224226}
@@ -228,9 +230,9 @@ impl<'de> Deserialize<'de> for Theme {
228230 where
229231 D: Deserializer<'de>,
230232 {
231− let values = HashMap::<String, Value>::deserialize(deserializer)?;
233+ let values = Map::<String, Value>::deserialize(deserializer)?;
232234
233− let (styles, scopes, highlights) = build_theme_values(Ok(values));
235+ let (styles, scopes, highlights) = build_theme_values(values);
234236
235237 Ok(Self {
236238 styles,
@@ -242,39 +244,37 @@ impl<'de> Deserialize<'de> for Theme {
242244}
243245
244246fn build_theme_values(
245− values: Result<HashMap<String, Value>>,
247+ mut values: Map<String, Value>,
246248) -> (HashMap<String, Style>, Vec<String>, Vec<Style>) {
247249 let mut styles = HashMap::new();
248250 let mut scopes = Vec::new();
249251 let mut highlights = Vec::new();
250252
251− if let Ok(mut colors) = values {
252− // TODO: alert user of parsing failures in editor
253− let palette = colors
254− .remove("palette")
255− .map(|value| {
256− ThemePalette::try_from(value).unwrap_or_else(|err| {
257− warn!("{}", err);
258− ThemePalette::default()
259− })
260− })
261− .unwrap_or_default();
262− // remove inherits from value to prevent errors
263− let _ = colors.remove("inherits");
264− styles.reserve(colors.len());
265− scopes.reserve(colors.len());
266− highlights.reserve(colors.len());
267− for (name, style_value) in colors {
268− let mut style = Style::default();
269− if let Err(err) = palette.parse_style(&mut style, style_value) {
253+ // TODO: alert user of parsing failures in editor
254+ let palette = values
255+ .remove("palette")
256+ .map(|value| {
257+ ThemePalette::try_from(value).unwrap_or_else(|err| {
270258 warn!("{}", err);
271− }
272−
273− // these are used both as UI and as highlights
274− styles.insert(name.clone(), style);
275− scopes.push(name);
276− highlights.push(style);
259+ ThemePalette::default()
260+ })
261+ })
262+ .unwrap_or_default();
263+ // remove inherits from value to prevent errors
264+ let _ = values.remove("inherits");
265+ styles.reserve(values.len());
266+ scopes.reserve(values.len());
267+ highlights.reserve(values.len());
268+ for (name, style_value) in values {
269+ let mut style = Style::default();
270+ if let Err(err) = palette.parse_style(&mut style, style_value) {
271+ warn!("{}", err);
277272 }
273+
274+ // these are used both as UI and as highlights
275+ styles.insert(name.clone(), style);
276+ scopes.push(name);
277+ highlights.push(style);
278278 }
279279
280280 (styles, scopes, highlights)