Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep all theme keys documented in one place #3649

Open
poliorcetics opened this issue Sep 2, 2022 · 1 comment
Open

Keep all theme keys documented in one place #3649

poliorcetics opened this issue Sep 2, 2022 · 1 comment
Labels
A-documentation Area: Documentation improvements C-enhancement Category: Improvements

Comments

@poliorcetics
Copy link
Contributor

poliorcetics commented Sep 2, 2022

Currently the theme keys are all over the place in the code and keeping track of them is only possible by looking at all the commits to find when new ones are added, making it hard for themes to know what they are missing and what has been removed since.

Fo example: #2759 added ui.bufferline.active but did not add it to the documentation, only the config option.

While not the only way to do so, here is what I propose:

Create a new crate only dealing with the ThemeKey type and its documentation.

The theme keys are a single enum like this:

#[derive(Debug, PartialEq, PartialOrd, Ord, Eq, Hash)] // PartialOrd, Ord for storage in a BTreeMap
pub enum ThemeKey {
    // Sort them alphabetically for easier maintenance
    UiBackground,
    UiOther,
    UiSomethingElse,
    // ...
}

impl FromStr for ThemeKey {
    // ...
}

impl Display for ThemeKey {
    // opposite of FromStr
}

// other impls as needed

Then add a binary to this crate that can be used to

  1. generate the doc for all these variants
  2. check the validity of a theme file (with varying levels of strictness, like "all keys and nothing more", "all keys but additional keys are okay", "not all keys but additionals are not okay", "not all keys and additionals are okay"), with checking for unused colors in the [palette] too

Run 1) in CI and check there are no diffs from the current docs, that way it always stays up to date for the helix doc pages, or even generate the page in CI

I'll try to work on this soon because it has really been bothering me 😅

What are the advantages

  1. Smaller theme keys (an enum variant is not a string literal so the hashmap/btreemap will be smaller)
  2. Easy to find references to usage points
  3. autocompletion
  4. no typos are possible
  5. theme checking
@poliorcetics poliorcetics added the C-enhancement Category: Improvements label Sep 2, 2022
@the-mikedavis
Copy link
Member

Theme keys have important fallback behavior: keyword.function falls back to keyword, markup.raw.inline falls back to markup.raw and markup, ui.bufferline.active falls back to ui.bufferline and ui. So an enum alone can't be use as the internal representation - it needs to be a trie.

Also, not all accesses of the theme can be done statically: tree-sitter query scopes use theme scopes directly. We could analyze the queries and look at their captures but this is not totally straightforward: some highlight queries use "throwaway captures" like @__name__ for matching a node the query doesn't want to highlight (example). Though we could use a convention that scopes starting with and underscore are not put into the generated theme key documentation. Or only automatically generate documentation for interface scopes (not syntax highlights).

There's a PR for checking themes #3234. It's also not straightforward to check a theme: not all themes must define all theme keys since they can use the fallback behavior or may want to not theme a particular element (like base16_transparent).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Documentation improvements C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants