trishume / syntect

Rust library for syntax highlighting using Sublime Text syntax definitions.
https://docs.rs/syntect
MIT License
1.93k stars 139 forks source link

Allow for loading a single default theme #478

Open CosmicHorrorDev opened 1 year ago

CosmicHorrorDev commented 1 year ago

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

use syntect::highlighting::{Theme as SyntectTheme, ThemeSet as SyntectThemeSet};

/// These represent the theme that the user selects
pub enum ThemeDefaults {
    Base16OceanDark,
    Base16EightiesDark,
    Base16MochaDark,
    Base16OceanLight,
    InspiredGithub,
    SolarizedDark,
    SolarizedLight,
}

impl ThemeDefaults {
    pub fn as_syntect_name(self) -> &'static str {
        match self {
            Self::Base16OceanDark => "base16-ocean.dark",
            Self::Base16EightiesDark => "base16-eighties.dark",
            Self::Base16MochaDark => "base16-mocha.dark",
            Self::Base16OceanLight => "base16-ocean.light",
            Self::InspiredGithub => "InspiredGitHub",
            Self::SolarizedDark => "Solarized (dark)",
            Self::SolarizedLight => "Solarized (light)",
        }
    }
}

impl From<ThemeDefaults> for SyntectTheme {
    fn from(default: ThemeDefaults) -> Self {
        let mut default_themes = SyntectThemeSet::load_defaults();
        default_themes
            .themes
            .remove(default.as_syntect_name())
            .expect("Included with defaults")
    }
}

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

Enselic commented 1 year ago

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".

CosmicHorrorDev commented 1 year ago

I feel like the performance notes I talked about are a bit of a red herring here. I like the idea of a LazyThemeSet and would be glad if syntect adopted it officially (since I already have roughly the same implementation copied in another one of my projects), but this issue is more about API ergonomics. I mentioned performance to note that this wasn't for a performance improvement

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

The default theme set in syntect is deliberately not maintained, so I'm not sure it makes sense to make it more "usable".

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