Theme overrides discussion

[ktprograms]

Original comment (as I can't seem to be able to convert my issue to a discussion):
I personally like the vulcan theme, except that the KeywordConstant token uses a grey background.

The ability to override specific parts of a theme in the config file would be nice.

There is a map of TokenType to string in chroma, which can be used in combination with a chroma.StyleBuilder to override specific parts.

For the configuration, I propose a subkey to the theme:, like overrides:. For example:

theme: "vulcan"
  overrides:
    - "kc": "#CF5967"
    - "nf": "#FFFFFF bold"

The quick.Highlight function only supports the chroma style name, so it can't be used with customized styles, but the function itself is only about 28 lines of code (less if we hardcode lexer and formatter), so it's feasible to copy it and allow it to take a chroma.Style directly (the change is also just removing 3 lines).

---

Having implemented it, I have a few new things to discuss.

I'm not sure the usefulness of specifying the input file in the config. I can understand how it was simple to implement with using cmd.Flags().VisitAll, but for having the theme overrides it'd be easier to not have to handle the input file from both flags and config (as VisitAll can't be used for the theme as it's using subkeys).

What I've currently implemented is:

• Still use StringVarP for the input file flag
• In the RunE func, get the theme name from the YAML (now stored as theme.name), and put it in a configTheme variable.
• Add correct logic to decide if the theme flag (stored in flags.theme variable) should be overwritten (only if the flag wasn't specified).
• As long as the configTheme and flags.theme are equal (to not use theme overrides on a different theme), parse the overrides (stored in YAML as theme.overrides) and apply them to the style.

I'm not really sure how to ensure the checking to not override should be done, because when the YAML key doesn't exist and the flag isn't passed, the default theme is used, but the overrides are still applied (as the "" config value is equal to the "" flag value). Should this just be considered "user error" and not have special handling?

This is what the config file will look like with my idea:

theme:
  name: vulcan
  overrides:
    kc: "#CF5967"

[noahgorstein]

Hi @ktprograms. Thanks for bringing this up and apologies for the wait on this. I do like this idea and think it'll be a nice option for those who want to customize the themes further. The short answer for why the file is supported via config file is just cause it was easy to do and at the time I didn't image a scenario of having config options with nested key/values. I thought there would only be a small number of flags such as "interactive" which would just take a value of true/false. That kind of thing. I like where you're headed with this. Do you have an in progress PR you want to put up?

[ktprograms]

Understandable about not expecting more complicated config flags.

I'll send a PR soon (I'll need to finalize it) that removes the file: config option and adds the theme name and overrides options.