comrak

comrak copied to clipboard

Had a lot of ideas floating around, so figured it would be good to open an issue that covers all of them

Existing issues / papercuts:

1. SyntectAdapter panics when trying to use a theme name that isn't contained in the theme_set
2. SyntectAdapter holds an entire theme set when holding the single theme being used would suffice
3. No way to use SyntectAdapter without pulling in the embedded defaults from syntect
   • I'm currently using an external syntax definitions in one of my projects, but I still wind up with ~0.8 MiB of extra data in the final binary from syntects default syntax definitions
4. No obvious way to know which themes are included in the default set
   • Really I'm talking about trishume/syntect#478

Spitballing a new API

I was thinking providing something along the lines of

struct SyntectAdapter {
    theme: Theme,
    syntax_set: SyntaxSet,
}

impl SyntectAdapter {
    fn defaults(default: DefaultTheme) -> Self { ... }
    fn empty() -> SyntectBuilder { ... }
}

struct SyntectBuilder {
    ...
}

impl SyntectBuilder {
    fn default_theme(mut self, default: DefaultTheme) -> Self { ... }
    fn custom_theme(mut self, theme: Theme) -> Self { ... }
    fn default_syntax_set(mut self) -> Self { ... }
    fn custom_syntax_set(mut self, syntax_set: SyntaxSet) -> Self { ... }
    fn finish(mut self) -> SyntectAdapter { ... }
}

#[non_exhaustive]
enum DefaultTheme {
    Base16OceanDark,
    Base16EightiesDark,
    Base16MochaDark,
    Base16OceanLight,
    InspiredGithub,
    SolarizedDark,
    SolarizedLight,
}

1) and 2) are addressed by moving getting the actual Theme to use to caller code, 3) is addressed by adding an extra syntect-no-defaults feature that would remove SyntectAdapter::defaults(), SyntectBuilder::default_theme(), and SyntectBuilder::default_syntax_set(), and 4) is addressed through adding DefaultTheme and SyntectBuilder::default_theme()

I think point 3) is nil since the linker seems smart enough to throw away the unused embedded asset, so I don't think that adding more features to control that makes sense. I don't think that changes my ideal API though