-
Notifications
You must be signed in to change notification settings - Fork 136
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
Allow for loading a single default theme #478
Comments
In bat we already load themes lazily, see https://github.com/sharkdp/bat/blob/8676bbf97f2832ad2231e102ca9c9b7b72267fda/src/assets/lazy_theme_set.rs. Maybe you can do something similar? The default theme set in syntect is deliberately not maintained, so I'm not sure it makes sense to make it more "usable". |
I feel like the performance notes I talked about are a bit of a red herring here. I like the idea of a All I really want is an enum that enumerates all of the values contained in the default theme set since right now it's up to library consumers manually checking all of the keys to figure out what themes are actually in the set
Can you elaborate on what you mean here? This addresses the main pain-point I had when dealing with the default theme set where the included themes are pretty opaque |
Motivation
I have a project where we allow users to pick one of the builtin syntect themes as the syntax highlighter. Only one theme will be used for the whole execution of the program, so to get that theme we have some code that's something along the lines of
It would be nice if syntect exposed a way to do something like the above with a
Theme::load_default(ThemeDefaults)
method that allowed for loading a single theme. I see that the current implementation compresses all of the themes together which makes sense for trying to reduce binary size. This means that loading a single theme will still have to decompress and deserialize everything, but this should still be more ergonomic when the user only wants to load a single theme (which I would assume is the common case)I'd be happy to open a PR if this seems like a reasonable addition
The text was updated successfully, but these errors were encountered: