Inherit theme

[TheSamsa]

This PR provides a solution to resolve #2454

[kirawi]

I think it could be simplified further by having the deserializer for Theme use the deserializer for RawTheme. That way the change here wouldn't be necessary: https://github.com/helix-editor/helix/pull/3067/files#diff-e045fef28ffa8dfddad18b6f9689de4865c7b4fa8a1a8b2768ea01912d2547d0R16-R18

[TheSamsa]

I am not familiar with serde to know how to use one Deserializer inside another one, or how to invoke the deserialization itself.

Also I guess this would not work fully if the theme has a base theme, because right now the deserialization of the RawTheme only stores the name of the inherit_from theme only as string, and does not load it automatically. (I didn't want to put a file operation inside the deserialize method)
Additionally this is rather a conversion from one type to another, not a 'real' deserialization. Thats why I used the From/Into approach. But I am okay with changing this, if I know how 😉

[archseer]

This RawTheme seems unnecessary. I'd deserialize both files to toml::Value, then use merge_toml_values like we do for the config, then deserialize the merged value into a theme. If you merge the new theme into the parent, it'll override values as needed.

[TheSamsa]

@archseer
I tested it with merge_toml_values but it does not work as I thought it would.
Example: i use this to remove a modifier in the gruvbox theme, from this:
"ui.selection" = { bg = "bg3", modifiers = ["reversed"] } to "ui.selection" = { bg = "bg3" }
But with the mechanic of merge_toml_values (https://github.com/helix-editor/helix/blob/master/helix-loader/src/lib.rs#L169-L186) it does not remove the modifier it keeps it, no matter which theme is passed as the left or right into the function.

[the-mikedavis]

You'll want to use a lower merge_depth so style maps themselves are not merged. You might need to split the palette out of the toml before merging though since it has merge_depth + 1 compared to the styles.

[TheSamsa]

@the-mikedavis thanks for the hints. I removed the RawTheme and use the toml::Value directly, although it did require some special handling because of the palette as you correctly stated.
But it works now, even with multiple stages of inherit_from definitions. Although it is probably possible to introduce circular loading of themes. Nonetheless I thought it might be the wish to be able to use a chain of theme inheritance.

[Omnikar]

@the-mikedavis Anything still left to be done here before merging? I'd really like to see this feature land.

[archseer]

HashMap::<String, Value>::deserialize(value)

Should still work since toml::Value implements Deserialize.

[TheSamsa]

It does for a Deserialize trait implementation, see the answer below.

[archseer]

Why the change? It now takes extra steps since you first call from_slice then Theme::from

[TheSamsa]

I implemented the From trait for Theme because I could not directly deserialize from a file. Since we would need to load the parent theme while deserializing. A pattern which I did not like.
But we could implement both and share the code, I'll provide a solution.