helix
helix copied to clipboard
The ability to define global theme overrides
Currently, making changes to a theme is only possible by inheritance or editing the theme directly. Both of these options require a per-theme approach. But overrides such as "using the terminal's background" usually are meant for any theme. This makes it annoying to change themes after doing some alterations to the previous one.
~~This PR add the option to create a theme_overrides.toml file in the config directory and apply the changes to any theme currently active.~~
This PR adds the option to use a theme table in config.toml that has theme.overrides option. The keys specified here will override the active theme.
- The changes are backwards compatible.
- The overrides will persist while changing themes with
:theme. theme.overridestable is internally the same as a theme so any subkeys a theme can have, overrides can also have.
Example:
[theme]
name = "github_dark"
[theme.overrides]
"ui.background" = {}
"ui.cursor" = { fg = "dark", bg = "light" }
"ui.statusline" = { bg = {} }
"ui.statusline.normal" = { fg = "dark", bg = "light" }
"ui.statusline.insert" = { fg = "dark", bg = "light" }
"ui.statusline.select" = { fg = "dark", bg = "light" }
"ui.statusline.separator" = { fg = "light", bg = "dark" }
[theme.overrides.palette]
dark = "#000000"
light = "#E9E0C9"
Instead of a seperate config file could this not be subkeys in config.toml, for example:
[theme.overwrite]
# Theme overwrites here
Exactly my thoughts as well, but theme would then be a toml map and I think we lose the ability to do theme = "onedark".
Instead of a seperate config file could this not be subkeys in
config.toml, for example:[theme.overwrite] # Theme overwrites here
@pascalkuthe wouldn't it add unnecessary overhead to the configuration file if a user wants to make too many overrides? That was my thinking.
Exactly my thoughts as well, but
themewould then be a toml map and I think we lose the ability to dotheme = "onedark".
Ad damm I always forget about that. I hate that aspect of toml even if that makes it a lot easier to deal with. You could do soo many cool things like this.
Maybe we could just allow a hybrid. Either allow the current theme = "onedark" or make it a map:
[theme]
name = "onedark"
[theme.overwrite]
# Theme overwrites here
Might be confusing tough so perhaps just adding a separate theme_overwrite map might be better.
[theme_overwrite]
# Theme overwrites here
Not sure which of those two options are better.
Instead of a seperate config file could this not be subkeys in
config.toml, for example:[theme.overwrite] # Theme overwrites here@pascalkuthe wouldn't it add unnecessary overhead to the configuration file if a user wants to make too many overrides? That was my thinking.
I think a global overwrite like this is only useful for a few themekeys like the background and maybe the cursor. Most other overwrites would be theme specifc (it would look if all themes used the same color for certain syntax highlights) so I don't think that would be a problem.
I would be very cautious about adding new config files.
I think a global overwrite like this is only useful for a few themekeys like the background and maybe the cursor. Most other overwrites would be theme specifc (it would look if all themes used the same color for certain syntax highlights) so I don't think that would be a problem.
That is correct and my use case as well.
I would be very cautious about adding new config files.
I think including this in the config is the better choice anyway but for my reference, what is the potential issue with adding new config files?
Regarding how to define in the config, first option seems more extendable and structured. However, wouldn't it break existing configs?
I think including this in the config is the better choice anyway but for my reference, what is the potential issue with adding new config files?
Primarily, If we keep adding new config files, the config becomes more spread-out and harder to find what you are looking for. There is also an effort to add per-workspace configs (#5748) and each new file needs to be discovered with multiple fallback paths etc.
Regarding how to define in the config, first option seems more extendable and structured. However, wouldn't it break existing configs?
As I said we would allow both options for backwards compatibility.
We check if theme is a string and treat it as we do now or we check if theme is a table and interpret as I showed above.
We have avoided breaking changes to the config multiple times with schemes like this.
It's a bit confusing that you would need to change the way the theme name is specified if you want to overwrite something that's why I also offered the second alternative but we do stuff like this already for other settings too so it's probably fine.
As I said we would allow both options for backwards compatibility. We check if
themeis a string and treat it as we do now or we check ifthemeis a table and interpret as I showed above. We have avoided breaking changes to the config multiple times with schemes like this.
This is quite common, and serde even has a doc page on the subject: https://serde.rs/string-or-struct.html.
@pascalkuthe these changes capture what I meant to do