From 33daeafb77278f965eeeed73890930893bb6ae3d Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Thu, 14 Jul 2022 12:31:13 +0200
Subject: [PATCH 1/8] Add RawTheme to handle inheritance with theme palette

---
 helix-view/src/theme.rs | 99 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 91 insertions(+), 8 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index fa5fa702804b..4711c17af1bf 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -42,17 +42,21 @@ impl Loader {
         if name == "base16_default" {
             return Ok(self.base16_default());
         }
-        let filename = format!("{}.toml", name);
 
-        let user_path = self.user_dir.join(&filename);
-        let path = if user_path.exists() {
-            user_path
-        } else {
-            self.default_dir.join(filename)
-        };
 
+        // let mut raw_theme: RawTheme = self.load_raw(name)?;
+
+        // if let Some(parent_theme_name) = &raw_theme.inherits_from {
+        //     //let parent_theme_name = "bogster";
+        //     let parent_raw_theme = self.load_raw(parent_theme_name)?;
+
+        //     raw_theme.inherit(parent_raw_theme);
+        // }
+
+        let path = self.path(name);
         let data = std::fs::read(&path)?;
-        toml::from_slice(data.as_slice()).context("Failed to deserialize theme")
+
+        toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
     }
 
     pub fn read_names(path: &Path) -> Vec<String> {
@@ -70,6 +74,28 @@ impl Loader {
             .unwrap_or_default()
     }
 
+    // Loads the raw theme data first from the user_dir then in default_dir
+    fn load_raw(&self, name: &str) -> Result<RawTheme, anyhow::Error> {
+        let path = self.path(name);
+        let data = std::fs::read(&path)?;
+
+        toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
+    }
+
+    // Returns the path to the theme name
+    fn path(&self, name: &str) -> PathBuf {
+        let filename = format!("{}.toml", name);
+
+        let user_path = self.user_dir.join(&filename);
+        let path = if user_path.exists() {
+            user_path
+        } else {
+            self.default_dir.join(filename)
+        };
+
+        path
+    }
+
     /// Lists all theme names available in default and user directory
     pub fn names(&self) -> Vec<String> {
         let mut names = Self::read_names(&self.user_dir);
@@ -96,6 +122,63 @@ impl Loader {
     }
 }
 
+struct RawTheme {
+    // Raw toml values
+    values: HashMap<String, Value>,
+    palette: ThemePalette,
+    inherits_from: Option<String>,
+}
+
+impl RawTheme {
+    fn inherit(&mut self, parent_theme: RawTheme) {
+        let palette = ThemePalette::new(
+            parent_theme
+                .palette
+                .palette
+                .into_iter()
+                .chain(self.palette.palette.clone())
+                .collect(),
+        );
+        self.palette = palette;
+
+        let values = parent_theme
+            .values
+            .into_iter()
+            .chain(self.values.clone())
+            .collect();
+        self.values = values;
+    }
+}
+
+impl<'de> Deserialize<'de> for RawTheme {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let mut values = HashMap::<String, Value>::deserialize(deserializer)?;
+
+        let palette = values
+            .remove("palette")
+            .map(|value| {
+                ThemePalette::try_from(value).unwrap_or_else(|err| {
+                    warn!("{}", err);
+                    ThemePalette::default()
+                })
+            })
+            .unwrap_or_default();
+
+        let inherits_from = values
+            .remove("inherits_from")
+            .map(|value| value.to_string().replace("\"", ""));
+
+        Ok(Self {
+            values,
+            palette,
+            inherits_from,
+        })
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct Theme {
     // UI styles are stored in a HashMap

From 3370de826a4182400552f388c17a8e0c57fdab8a Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Thu, 14 Jul 2022 14:19:52 +0200
Subject: [PATCH 2/8] Add a intermediate step in theme loading

it uses RawTheme struct to load the original ThemePalette, so we can merge it with the inherited one.
---
 helix-view/src/theme.rs | 46 +++++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index 4711c17af1bf..b49ebf65ac5a 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -43,20 +43,21 @@ impl Loader {
             return Ok(self.base16_default());
         }
 
+        let mut raw_theme: RawTheme = self.load_raw(name)?;
 
-        // let mut raw_theme: RawTheme = self.load_raw(name)?;
+        if let Some(parent_theme_name) = &raw_theme.inherits_from {
+            //let parent_theme_name = "bogster";
+            let parent_raw_theme = self.load_raw(parent_theme_name)?;
 
-        // if let Some(parent_theme_name) = &raw_theme.inherits_from {
-        //     //let parent_theme_name = "bogster";
-        //     let parent_raw_theme = self.load_raw(parent_theme_name)?;
+            raw_theme.inherit(parent_raw_theme);
+        }
 
-        //     raw_theme.inherit(parent_raw_theme);
-        // }
+        Ok(Theme::from(raw_theme))
 
-        let path = self.path(name);
-        let data = std::fs::read(&path)?;
+        // let path = self.path(name);
+        // let data = std::fs::read(&path)?;
 
-        toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
+        // toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
     }
 
     pub fn read_names(path: &Path) -> Vec<String> {
@@ -157,6 +158,7 @@ impl<'de> Deserialize<'de> for RawTheme {
     {
         let mut values = HashMap::<String, Value>::deserialize(deserializer)?;
 
+        // TODO: alert user of parsing failures in editor
         let palette = values
             .remove("palette")
             .map(|value| {
@@ -188,6 +190,32 @@ pub struct Theme {
     highlights: Vec<Style>,
 }
 
+impl From<RawTheme> for Theme {
+    fn from(raw_theme: RawTheme) -> Self {
+        let mut styles = HashMap::new();
+        let mut scopes = Vec::new();
+        let mut highlights = Vec::new();
+
+        for (name, style_value) in raw_theme.values {
+            let mut style = Style::default();
+            if let Err(err) = raw_theme.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);
+        }
+
+        Self {
+            styles,
+            scopes,
+            highlights,
+        }
+    }
+}
+
 impl<'de> Deserialize<'de> for Theme {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where

From cbbe8ae51d2b5fee32c919bb499a8c1909009c84 Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Thu, 14 Jul 2022 14:31:13 +0200
Subject: [PATCH 3/8] Load default themes via RawThemes, remove Theme
 deserialization

---
 helix-view/src/theme.rs | 68 ++++++-----------------------------------
 1 file changed, 9 insertions(+), 59 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index b49ebf65ac5a..3314e036c856 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -13,11 +13,14 @@ use toml::Value;
 pub use crate::graphics::{Color, Modifier, Style};
 
 pub static DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    toml::from_slice(include_bytes!("../../theme.toml")).expect("Failed to parse default theme")
+    let raw_theme: RawTheme = toml::from_slice(include_bytes!("../../theme.toml"))
+        .expect("Failed to parse default theme");
+    Theme::from(raw_theme)
 });
 pub static BASE16_DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    toml::from_slice(include_bytes!("../../base16_theme.toml"))
-        .expect("Failed to parse base 16 default theme")
+    let raw_theme: RawTheme = toml::from_slice(include_bytes!("../../base16_theme.toml"))
+        .expect("Failed to parse base 16 default theme");
+    Theme::from(raw_theme)
 });
 
 #[derive(Clone, Debug)]
@@ -53,11 +56,6 @@ impl Loader {
         }
 
         Ok(Theme::from(raw_theme))
-
-        // let path = self.path(name);
-        // let data = std::fs::read(&path)?;
-
-        // toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
     }
 
     pub fn read_names(path: &Path) -> Vec<String> {
@@ -88,13 +86,11 @@ impl Loader {
         let filename = format!("{}.toml", name);
 
         let user_path = self.user_dir.join(&filename);
-        let path = if user_path.exists() {
+        if user_path.exists() {
             user_path
         } else {
             self.default_dir.join(filename)
-        };
-
-        path
+        }
     }
 
     /// Lists all theme names available in default and user directory
@@ -171,7 +167,7 @@ impl<'de> Deserialize<'de> for RawTheme {
 
         let inherits_from = values
             .remove("inherits_from")
-            .map(|value| value.to_string().replace("\"", ""));
+            .map(|value| value.to_string().replace('\"', ""));
 
         Ok(Self {
             values,
@@ -216,52 +212,6 @@ impl From<RawTheme> for Theme {
     }
 }
 
-impl<'de> Deserialize<'de> for Theme {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let mut styles = HashMap::new();
-        let mut scopes = Vec::new();
-        let mut highlights = Vec::new();
-
-        if let Ok(mut colors) = HashMap::<String, Value>::deserialize(deserializer) {
-            // 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();
-
-            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) {
-                    warn!("{}", err);
-                }
-
-                // these are used both as UI and as highlights
-                styles.insert(name.clone(), style);
-                scopes.push(name);
-                highlights.push(style);
-            }
-        }
-
-        Ok(Self {
-            scopes,
-            styles,
-            highlights,
-        })
-    }
-}
-
 impl Theme {
     #[inline]
     pub fn highlight(&self, index: usize) -> Style {

From 07f6cf279c343bb311cc45c506bea18c25e12ba7 Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Thu, 14 Jul 2022 17:22:45 +0200
Subject: [PATCH 4/8] Allow naming custom theme same as inherited one

---
 helix-view/src/theme.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index 3314e036c856..6774eb582738 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -46,11 +46,12 @@ impl Loader {
             return Ok(self.base16_default());
         }
 
-        let mut raw_theme: RawTheme = self.load_raw(name)?;
+        let path = self.path(name, false);
+        let mut raw_theme: RawTheme = self.load_raw(path)?;
 
         if let Some(parent_theme_name) = &raw_theme.inherits_from {
-            //let parent_theme_name = "bogster";
-            let parent_raw_theme = self.load_raw(parent_theme_name)?;
+            let path = self.path(parent_theme_name, parent_theme_name == name);
+            let parent_raw_theme = self.load_raw(path)?;
 
             raw_theme.inherit(parent_raw_theme);
         }
@@ -74,19 +75,18 @@ impl Loader {
     }
 
     // Loads the raw theme data first from the user_dir then in default_dir
-    fn load_raw(&self, name: &str) -> Result<RawTheme, anyhow::Error> {
-        let path = self.path(name);
+    fn load_raw(&self, path: PathBuf) -> Result<RawTheme, anyhow::Error> {
         let data = std::fs::read(&path)?;
 
         toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
     }
 
     // Returns the path to the theme name
-    fn path(&self, name: &str) -> PathBuf {
+    fn path(&self, name: &str, only_default_dir: bool) -> PathBuf {
         let filename = format!("{}.toml", name);
 
         let user_path = self.user_dir.join(&filename);
-        if user_path.exists() {
+        if !only_default_dir && user_path.exists() {
             user_path
         } else {
             self.default_dir.join(filename)

From 0ddd6d7b8099d7e856864a273516b81c6d25fdd4 Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Tue, 9 Aug 2022 12:56:12 +0200
Subject: [PATCH 5/8] Remove RawTheme and use toml::Value directly

---
 Cargo.lock              |   1 +
 helix-view/Cargo.toml   |   1 +
 helix-view/src/theme.rs | 191 ++++++++++++++++++++++------------------
 3 files changed, 108 insertions(+), 85 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b02f2d99d62e..44bd8a7dfbf1 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -504,6 +504,7 @@ dependencies = [
  "futures-util",
  "helix-core",
  "helix-dap",
+ "helix-loader",
  "helix-lsp",
  "helix-tui",
  "log",
diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml
index b612d2d44087..acbfc5c89c90 100644
--- a/helix-view/Cargo.toml
+++ b/helix-view/Cargo.toml
@@ -17,6 +17,7 @@ term = ["crossterm"]
 bitflags = "1.3"
 anyhow = "1"
 helix-core = { version = "0.6", path = "../helix-core" }
+helix-loader = { version = "0.6", path = "../helix-loader" }
 helix-lsp = { version = "0.6", path = "../helix-lsp" }
 helix-dap = { version = "0.6", path = "../helix-dap" }
 crossterm = { version = "0.24", optional = true }
diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index 6774eb582738..b71c6da75fe6 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -5,20 +5,20 @@ use std::{
 
 use anyhow::Context;
 use helix_core::hashmap;
+use helix_loader::merge_toml_values;
 use log::warn;
 use once_cell::sync::Lazy;
-use serde::{Deserialize, Deserializer};
-use toml::Value;
+use toml::{map::Map, Value};
 
 pub use crate::graphics::{Color, Modifier, Style};
 
 pub static DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    let raw_theme: RawTheme = toml::from_slice(include_bytes!("../../theme.toml"))
+    let raw_theme: Value = toml::from_slice(include_bytes!("../../theme.toml"))
         .expect("Failed to parse default theme");
     Theme::from(raw_theme)
 });
 pub static BASE16_DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    let raw_theme: RawTheme = toml::from_slice(include_bytes!("../../base16_theme.toml"))
+    let raw_theme: Value = toml::from_slice(include_bytes!("../../base16_theme.toml"))
         .expect("Failed to parse base 16 default theme");
     Theme::from(raw_theme)
 });
@@ -46,17 +46,40 @@ impl Loader {
             return Ok(self.base16_default());
         }
 
-        let path = self.path(name, false);
-        let mut raw_theme: RawTheme = self.load_raw(path)?;
+        let theme_toml = self.load_theme(name, name, false)?;
 
-        if let Some(parent_theme_name) = &raw_theme.inherits_from {
-            let path = self.path(parent_theme_name, parent_theme_name == name);
-            let parent_raw_theme = self.load_raw(path)?;
+        Ok(Theme::from(theme_toml))
+    }
 
-            raw_theme.inherit(parent_raw_theme);
-        }
+    // load the theme and its parent recursively and merge them
+    // `base_theme_name` is the theme from the config.toml,
+    // used to prevent some circular loading scenarios
+    fn load_theme(
+        &self,
+        name: &str,
+        base_them_name: &str,
+        only_default_dir: bool,
+    ) -> Result<Value, anyhow::Error> {
+        let path = self.path(name, only_default_dir);
+        let theme_toml = self.load_toml(path)?;
+
+        let inherits_from = theme_toml.get("inherits_from");
+
+        let theme_toml = if let Some(parent_theme_name) = inherits_from {
+            let parent_theme_name = parent_theme_name.to_string().replace('\"', "");
+
+            let parent_theme_toml = self.load_theme(
+                &parent_theme_name,
+                base_them_name,
+                base_them_name == parent_theme_name,
+            )?;
+
+            self.merge_themes(parent_theme_toml, theme_toml)
+        } else {
+            theme_toml
+        };
 
-        Ok(Theme::from(raw_theme))
+        Ok(theme_toml)
     }
 
     pub fn read_names(path: &Path) -> Vec<String> {
@@ -74,14 +97,47 @@ impl Loader {
             .unwrap_or_default()
     }
 
-    // Loads the raw theme data first from the user_dir then in default_dir
-    fn load_raw(&self, path: PathBuf) -> Result<RawTheme, anyhow::Error> {
+    // merge one theme into the parent theme
+    fn merge_themes(&self, parent_theme_toml: Value, theme_toml: Value) -> Value {
+        let parent_palette = parent_theme_toml.get("palette");
+        let palette = theme_toml.get("palette");
+
+        // handle the table seperately since it needs a `merge_depth` of 2
+        // this would conflict with the rest of the theme merge strategy
+        let palette_values = if let Some(parent_palette) = parent_palette {
+            if let Some(palette) = palette {
+                merge_toml_values(parent_palette.clone(), palette.clone(), 2)
+            } else {
+                parent_palette.clone()
+            }
+        } else {
+            if let Some(palette) = palette {
+                palette.clone()
+            } else {
+                Map::new().into()
+            }
+        };
+
+        // add the palette correctly as nested table
+        let mut palette = Map::new();
+        palette.insert(String::from("palette"), palette_values);
+
+        // merge the theme into the parent theme
+        let theme = merge_toml_values(parent_theme_toml, theme_toml, 1);
+        // merge the before specially handled palette into the theme
+        merge_toml_values(theme, palette.into(), 1)
+    }
+
+    // Loads the theme data as `toml::Value` first from the user_dir then in default_dir
+    fn load_toml(&self, path: PathBuf) -> Result<toml::Value, anyhow::Error> {
         let data = std::fs::read(&path)?;
 
-        toml::from_slice(data.as_slice()).context("Faled to deserialize theme")
+        toml::from_slice(data.as_slice()).context("Failed to deserialize theme")
     }
 
-    // Returns the path to the theme name
+    // Returns the path to the theme with the name
+    // With `only_default_dir` as false the path will first search for the user path
+    // disabled it ignores the user path and returns only the default path
     fn path(&self, name: &str, only_default_dir: bool) -> PathBuf {
         let filename = format!("{}.toml", name);
 
@@ -119,64 +175,6 @@ impl Loader {
     }
 }
 
-struct RawTheme {
-    // Raw toml values
-    values: HashMap<String, Value>,
-    palette: ThemePalette,
-    inherits_from: Option<String>,
-}
-
-impl RawTheme {
-    fn inherit(&mut self, parent_theme: RawTheme) {
-        let palette = ThemePalette::new(
-            parent_theme
-                .palette
-                .palette
-                .into_iter()
-                .chain(self.palette.palette.clone())
-                .collect(),
-        );
-        self.palette = palette;
-
-        let values = parent_theme
-            .values
-            .into_iter()
-            .chain(self.values.clone())
-            .collect();
-        self.values = values;
-    }
-}
-
-impl<'de> Deserialize<'de> for RawTheme {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let mut values = HashMap::<String, Value>::deserialize(deserializer)?;
-
-        // 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);
-                    ThemePalette::default()
-                })
-            })
-            .unwrap_or_default();
-
-        let inherits_from = values
-            .remove("inherits_from")
-            .map(|value| value.to_string().replace('\"', ""));
-
-        Ok(Self {
-            values,
-            palette,
-            inherits_from,
-        })
-    }
-}
-
 #[derive(Clone, Debug)]
 pub struct Theme {
     // UI styles are stored in a HashMap
@@ -186,22 +184,45 @@ pub struct Theme {
     highlights: Vec<Style>,
 }
 
-impl From<RawTheme> for Theme {
-    fn from(raw_theme: RawTheme) -> Self {
+impl From<Value> for Theme {
+    fn from(value: Value) -> Self {
         let mut styles = HashMap::new();
         let mut scopes = Vec::new();
         let mut highlights = Vec::new();
 
-        for (name, style_value) in raw_theme.values {
-            let mut style = Style::default();
-            if let Err(err) = raw_theme.palette.parse_style(&mut style, style_value) {
-                warn!("{}", err);
-            }
+        let theme_values: Result<HashMap<String, Value>, anyhow::Error> =
+            toml::from_str(&value.to_string()).context("Failed to load theme");
+
+        if let Ok(mut colors) = theme_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_from");
+
+            styles.reserve(colors.len());
+            scopes.reserve(colors.len());
+            highlights.reserve(colors.len());
 
-            // these are used both as UI and as highlights
-            styles.insert(name.clone(), style);
-            scopes.push(name);
-            highlights.push(style);
+            for (name, style_value) in colors {
+                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);
+            }
         }
 
         Self {

From b5e267d7c87598edf2310d7565ae4310d85b15c3 Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Wed, 10 Aug 2022 08:44:03 +0200
Subject: [PATCH 6/8] Resolve all review changes resulting in a cleaner code

---
 helix-view/src/theme.rs | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index b71c6da75fe6..9530e79499bb 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -3,7 +3,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use anyhow::Context;
+use anyhow::{anyhow, Context, Result};
 use helix_core::hashmap;
 use helix_loader::merge_toml_values;
 use log::warn;
@@ -38,7 +38,7 @@ impl Loader {
     }
 
     /// Loads a theme first looking in the `user_dir` then in `default_dir`
-    pub fn load(&self, name: &str) -> Result<Theme, anyhow::Error> {
+    pub fn load(&self, name: &str) -> Result<Theme> {
         if name == "default" {
             return Ok(self.default());
         }
@@ -59,17 +59,22 @@ impl Loader {
         name: &str,
         base_them_name: &str,
         only_default_dir: bool,
-    ) -> Result<Value, anyhow::Error> {
+    ) -> Result<Value> {
         let path = self.path(name, only_default_dir);
         let theme_toml = self.load_toml(path)?;
 
-        let inherits_from = theme_toml.get("inherits_from");
+        let inherits = theme_toml.get("inherits");
 
-        let theme_toml = if let Some(parent_theme_name) = inherits_from {
-            let parent_theme_name = parent_theme_name.to_string().replace('\"', "");
+        let theme_toml = if let Some(parent_theme_name) = inherits {
+            let parent_theme_name = parent_theme_name.as_str().ok_or_else(|| {
+                anyhow!(
+                    "Theme: expected 'inherits' to be a string: {}",
+                    parent_theme_name
+                )
+            })?;
 
             let parent_theme_toml = self.load_theme(
-                &parent_theme_name,
+                parent_theme_name,
                 base_them_name,
                 base_them_name == parent_theme_name,
             )?;
@@ -104,18 +109,13 @@ impl Loader {
 
         // handle the table seperately since it needs a `merge_depth` of 2
         // this would conflict with the rest of the theme merge strategy
-        let palette_values = if let Some(parent_palette) = parent_palette {
-            if let Some(palette) = palette {
+        let palette_values = match (parent_palette, palette) {
+            (Some(parent_palette), Some(palette)) => {
                 merge_toml_values(parent_palette.clone(), palette.clone(), 2)
-            } else {
-                parent_palette.clone()
-            }
-        } else {
-            if let Some(palette) = palette {
-                palette.clone()
-            } else {
-                Map::new().into()
             }
+            (Some(parent_palette), None) => parent_palette.clone(),
+            (None, Some(palette)) => palette.clone(),
+            (None, None) => Map::new().into(),
         };
 
         // add the palette correctly as nested table
@@ -129,7 +129,7 @@ impl Loader {
     }
 
     // Loads the theme data as `toml::Value` first from the user_dir then in default_dir
-    fn load_toml(&self, path: PathBuf) -> Result<toml::Value, anyhow::Error> {
+    fn load_toml(&self, path: PathBuf) -> Result<Value> {
         let data = std::fs::read(&path)?;
 
         toml::from_slice(data.as_slice()).context("Failed to deserialize theme")
@@ -190,7 +190,7 @@ impl From<Value> for Theme {
         let mut scopes = Vec::new();
         let mut highlights = Vec::new();
 
-        let theme_values: Result<HashMap<String, Value>, anyhow::Error> =
+        let theme_values: Result<HashMap<String, Value>> =
             toml::from_str(&value.to_string()).context("Failed to load theme");
 
         if let Ok(mut colors) = theme_values {
@@ -206,7 +206,7 @@ impl From<Value> for Theme {
                 .unwrap_or_default();
 
             // remove inherits from value to prevent errors
-            let _ = colors.remove("inherits_from");
+            let _ = colors.remove("inherits");
 
             styles.reserve(colors.len());
             scopes.reserve(colors.len());

From 43e7cd47226f7f8be51776427a84e1e1c9e6500a Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Tue, 6 Sep 2022 18:48:49 +0200
Subject: [PATCH 7/8] Simplify return for Loader::load

---
 helix-view/src/theme.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index 9530e79499bb..c9d6e81fedde 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -46,9 +46,7 @@ impl Loader {
             return Ok(self.base16_default());
         }
 
-        let theme_toml = self.load_theme(name, name, false)?;
-
-        Ok(Theme::from(theme_toml))
+        self.load_theme(name, name, false).map(Theme::from)
     }
 
     // load the theme and its parent recursively and merge them

From 0bc852b216dece8c704cb561647d858454dbb699 Mon Sep 17 00:00:00 2001
From: Christoph Schmidler <c.schmidler@gmail.com>
Date: Thu, 8 Sep 2022 21:12:36 +0200
Subject: [PATCH 8/8] Add  implementation to avoid extra step for loading of
 base themes

---
 helix-view/src/theme.rs | 108 +++++++++++++++++++++++++---------------
 1 file changed, 68 insertions(+), 40 deletions(-)

diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs
index c9d6e81fedde..85f5cc131974 100644
--- a/helix-view/src/theme.rs
+++ b/helix-view/src/theme.rs
@@ -8,19 +8,25 @@ use helix_core::hashmap;
 use helix_loader::merge_toml_values;
 use log::warn;
 use once_cell::sync::Lazy;
+use serde::{Deserialize, Deserializer};
 use toml::{map::Map, Value};
 
 pub use crate::graphics::{Color, Modifier, Style};
 
 pub static DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    let raw_theme: Value = toml::from_slice(include_bytes!("../../theme.toml"))
-        .expect("Failed to parse default theme");
-    Theme::from(raw_theme)
+    //    let raw_theme: Value = toml::from_slice(include_bytes!("../../theme.toml"))
+    //        .expect("Failed to parse default theme");
+    //    Theme::from(raw_theme)
+
+    toml::from_slice(include_bytes!("../../theme.toml")).expect("Failed to parse default theme")
 });
 pub static BASE16_DEFAULT_THEME: Lazy<Theme> = Lazy::new(|| {
-    let raw_theme: Value = toml::from_slice(include_bytes!("../../base16_theme.toml"))
-        .expect("Failed to parse base 16 default theme");
-    Theme::from(raw_theme)
+    //    let raw_theme: Value = toml::from_slice(include_bytes!("../../base16_theme.toml"))
+    //        .expect("Failed to parse base 16 default theme");
+    //    Theme::from(raw_theme)
+
+    toml::from_slice(include_bytes!("../../base16_theme.toml"))
+        .expect("Failed to parse base 16 default theme")
 });
 
 #[derive(Clone, Debug)]
@@ -184,51 +190,73 @@ pub struct Theme {
 
 impl From<Value> for Theme {
     fn from(value: Value) -> Self {
-        let mut styles = HashMap::new();
-        let mut scopes = Vec::new();
-        let mut highlights = Vec::new();
-
-        let theme_values: Result<HashMap<String, Value>> =
+        let values: Result<HashMap<String, Value>> =
             toml::from_str(&value.to_string()).context("Failed to load theme");
 
-        if let Ok(mut colors) = theme_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");
+        let (styles, scopes, highlights) = build_theme_values(values);
 
-            styles.reserve(colors.len());
-            scopes.reserve(colors.len());
-            highlights.reserve(colors.len());
+        Self {
+            styles,
+            scopes,
+            highlights,
+        }
+    }
+}
 
-            for (name, style_value) in colors {
-                let mut style = Style::default();
-                if let Err(err) = palette.parse_style(&mut style, style_value) {
-                    warn!("{}", err);
-                }
+impl<'de> Deserialize<'de> for Theme {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let values = HashMap::<String, Value>::deserialize(deserializer)?;
 
-                // these are used both as UI and as highlights
-                styles.insert(name.clone(), style);
-                scopes.push(name);
-                highlights.push(style);
-            }
-        }
+        let (styles, scopes, highlights) = build_theme_values(Ok(values));
 
-        Self {
+        Ok(Self {
             styles,
             scopes,
             highlights,
+        })
+    }
+}
+
+fn build_theme_values(
+    values: Result<HashMap<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) {
+                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)
 }
 
 impl Theme {