Add ability to derive from a base theme

[poliorcetics]

Describe your feature request

Currently if I want to experiment with changes to a theme, I need to copy the original theme file and modify this copy. If the original is updated from upstream, I don't get those updates. If I haven't made a copy and directly modified the original, it's either overwritten, losing my changes, or causes a merge conflict if I have it in a local branch of helix.

I would like the ability to have something like [fallback] name = "gruvbox" (or [origin], [source], etc, name is bikeshedable) in theme files, so that I can modify only the keys that interest me while keeping the rest.

This could work by merging the TOMLs, like for the user config and the default one. Merging the TOMLs would also let me use colors declared in the original or override them.

It could even be done recursively so that I can have gv2 -> gv1 -> gruvbox if necessary, but that's just a nice-to-have.

[TheSamsa]

I want to so this, but would need some pointers where to begin ;)

[CptPotato]

Another thing to consider:

Should the derived theme be able to override or use palette variables from the base one, or are the palettes only valid within the same theme?

[poliorcetics]

Merging the TOMLs would also let me use colors declared in the original or override them.

I didn’t have the term « palette » but that’s what I wanted to say there

If palette are not merged or overridable, we end up in pretty much the same situation as now, because colors will go out of sync again

[kirawi]

I want to so this, but would need some pointers where to begin ;)

You could add an inherits field to

helix/helix-view/src/theme.rs

Lines 92 to 98 in af387e6

	pub struct Theme {
	// UI styles are stored in a HashMap
	styles: HashMap<String, Style>,
	// tree-sitter highlight styles are stored in a Vec to optimize lookups
	scopes: Vec<String>,
	highlights: Vec<Style>,
	}

as a String which would be equal to the name of a theme.

Then you could look for the theme within Loader and recursively call Loader::load(&theme.inherits) , and merge them within

helix/helix-view/src/theme.rs

Lines 38 to 56 in af387e6

	pub fn load(&self, name: &str) -> Result<Theme, anyhow::Error> {
	if name == "default" {
	return Ok(self.default());
	}
	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 data = std::fs::read(&path)?;
	toml::from_slice(data.as_slice()).context("Failed to deserialize theme")
	}

There is already a function to merge Toml values.

helix/helix-loader/src/lib.rs

Lines 114 to 168 in 2c7f770

	pub fn merge_toml_values(
	left: toml::Value,
	right: toml::Value,
	merge_toplevel_arrays: bool,
	) -> toml::Value {
	use toml::Value;
	fn get_name(v: &Value) -> Option<&str> {
	v.get("name").and_then(Value::as_str)
	}
	match (left, right) {
	(Value::Array(mut left_items), Value::Array(right_items)) => {
	// The top-level arrays should be merged but nested arrays should
	// act as overrides. For the `languages.toml` config, this means
	// that you can specify a sub-set of languages in an overriding
	// `languages.toml` but that nested arrays like Language Server
	// arguments are replaced instead of merged.
	if merge_toplevel_arrays {
	left_items.reserve(right_items.len());
	for rvalue in right_items {
	let lvalue = get_name(&rvalue)
	.and_then(|rname| {
	left_items.iter().position(|v| get_name(v) == Some(rname))
	})
	.map(|lpos| left_items.remove(lpos));
	let mvalue = match lvalue {
	Some(lvalue) => merge_toml_values(lvalue, rvalue, false),
	None => rvalue,
	};
	left_items.push(mvalue);
	}
	Value::Array(left_items)
	} else {
	Value::Array(right_items)
	}
	}
	(Value::Table(mut left_map), Value::Table(right_map)) => {
	for (rname, rvalue) in right_map {
	match left_map.remove(&rname) {
	Some(lvalue) => {
	let merged_value = merge_toml_values(lvalue, rvalue, merge_toplevel_arrays);
	left_map.insert(rname, merged_value);
	}
	None => {
	left_map.insert(rname, rvalue);
	}
	}
	}
	Value::Table(left_map)
	}
	// Catch everything else we didn't handle, and use the right value
	(_, value) => value,
	}
	}

Palette colors may need special handling so the palette doesn't completely overwrite the inherited palette.

[TheSamsa]

@kirawi hey thank you a lot, this will help me out!

Also I was thinking this approach will only allow to completely overwrite some properties. But wont work if one only wants to remove ie. underlined style.
But I guess for the first iteration this'll work.

[TheSamsa]

So I played around a bit and decided to provide a solution which uses an intermediate RawTheme for the deserialization.
This allows to merge the ThemePalette, so one can for example type this into his own theme

inherit_from = "gruvbox"
[palette]
bg0 = "#ffffff"

and his background will be white.

[CptPotato]

This looks pretty good, would this work as well?

inherit_from = "gruvbox"
ui.background = "#ffffff"

[TheSamsa]

@CptPotato
If you use ui.background = { bg = "#ffffff" } it does.

Also, I will update this, because right now your custom theme needs to have another name than the theme you are inheriting from. But I like to name my theme gruvbox and first use the user dir, and for the inherits_from the default dir.

[TheSamsa]

It should now be possible to name the custom theme in user dir the same as the default dir one.

[getreu]

My use case would be to inherit Autumn_night from Autumn, which would ease maintenance a lot. I suppose other themes will profit as well.

Will it be possible to inherit from multiple files? One could have different files for foreground, background and ruler themes and combine them, or :

What about just overwriting variables by allowing to load more than one theme in a sequence? E.g.:

:theme Autumn_night

would become:

:theme Autumn Autumn_night

[TheSamsa]

@getreu
The proposed solution in the PR does only allow to inherit from one base theme. Because inheriting from multiple themes can be quite tricky in which order the values are overwritten. Which will make it quite complex.
It could be blown up and introduce a complex syntax for inheriting themes and specifying what values to overwrite and which not, but I don't think this will be worth it.
I assume most use cases are dealt with this approach.

[TheSamsa]

@kirawi would you mind to review the proposed solution?