Skip to content

theme: make ThemeStyle and StatusColors fields constructable#2322

Draft
glani wants to merge 1 commit into
longbridge:mainfrom
glani:heretic/theme-style-public-fields
Draft

theme: make ThemeStyle and StatusColors fields constructable#2322
glani wants to merge 1 commit into
longbridge:mainfrom
glani:heretic/theme-style-public-fields

Conversation

@glani
Copy link
Copy Markdown
Contributor

@glani glani commented Apr 30, 2026

Summary

Widens four field-level privacies in crates/ui/src/highlighter/registry.rs:

  • ThemeStyle.color, ThemeStyle.font_style, ThemeStyle.font_weightpub
  • StatusColors.errorpub

No semantic change. No signature change. No test change. 4 lines.

Why

HighlightThemeStyle exposes its syntax: SyntaxColors and status: StatusColors as pub fields, and SyntaxColors's per-category fields are pub Option<ThemeStyle>. A downstream consumer can read theme.highlight_theme.style.syntax.keyword and even assign a Some(_) to it — but cannot construct the ThemeStyle value to put inside the Some. The fields are private, no constructor exists, no Default impl exists, only Serialize/Deserialize derives.

The result: programmatic construction of a ThemeStyle from outside the crate is impossible without a JSON round-trip through serde. That's brittle (no compile-time check on field names — a future field rename silently breaks downstream at runtime) and pulls a serde_json dep into consumers who otherwise wouldn't need it.

Two consistency arguments for widening:

  1. The wasm_stub.rs counterpart already declares ThemeStyle with all pub fields (crates/ui/src/highlighter/wasm_stub.rs:118-123). Native registry.rs is the outlier — same logical type, different visibility surface depending on target.
  2. The encapsulation isn't load-bearing. No validation, no normalization, no invariant. The fields are plain Option<...> config slots that get populated by deserialization and read by From<ThemeStyle> for HighlightStyle. Widening privacy doesn't bypass anything.

Concrete downstream use case

A theme-bridge that maps an app-defined htheme format's syntax.* block onto upstream's SyntaxColors:

let mut highlight = HighlightTheme::clone(&theme.highlight_theme);
let syntax = &mut highlight.style.syntax;

if let Some(s) = htheme.syntax.get("keyword") {
    syntax.keyword = Some(ThemeStyle {
        color: Some(s.fg),
        font_weight: s.bold.then_some(FontWeightContent::Bold),
        font_style: s.italic.then_some(FontStyle::Italic),
    });
}
// ... map remaining categories ...

highlight.style.status.error = Some(htheme.error_color);
theme.highlight_theme = Arc::new(highlight);

Without this PR the same code path requires serde_json::from_value(...) on a hand-built JSON object, which is a workable workaround but is strictly worse on every axis.

Scope deliberately small

The remaining StatusColors fields (warning, info, success, hint and their .background / .border variants) stay private for now. They have the same encapsulation defect, but I've narrowed this PR to exactly the fields a real consumer needs to write today. Happy to widen the scope if you'd prefer all 15 in one go — just say the word.

Status: draft

Marking draft until the downstream consumer (a heretic theme bridge in another workspace) verifies the patch enables the intended use case end-to-end. Will move to ready-for-review once that lands.

Both ThemeStyle (the value type stored in SyntaxColors::keyword,
.string, .comment, etc.) and StatusColors::error keep their fields
private with no constructor, no public builder, and (for ThemeStyle)
no Default impl. The container type HighlightThemeStyle exposes its
syntax: SyntaxColors and status: StatusColors as pub fields, and
SyntaxColors's per-category fields are pub Option<ThemeStyle>, so a
downstream consumer can read theme.highlight_theme.style.syntax.keyword
and assign a Some(_) to it, but cannot construct the ThemeStyle value
to put inside the Some. The struct is reachable only via
Serialize/Deserialize round-trip through JSON.

Two consistency points argue for widening privacy:

1. The wasm_stub.rs counterpart already declares ThemeStyle with all
   pub fields (highlighter/wasm_stub.rs:118-123). Native registry.rs
   has been the outlier.
2. The encapsulation isn't load-bearing: there's no validation,
   normalization, or invariant being protected. The fields are plain
   Option<...> config slots.

This change widens privacy on the four fields a downstream theme
bridge needs to write programmatically: ThemeStyle.{color, font_style,
font_weight} and StatusColors.error. No semantic change, no signature
change, no test change. The remaining StatusColors fields (warning,
info, success, hint and their .background/.border variants) stay
private for now; widen as needed in a future PR.

Concrete downstream use case: a theme-bridge that maps an app-defined
htheme format's syntax.* block onto upstream's SyntaxColors:

    let mut highlight = HighlightTheme::clone(&theme.highlight_theme);
    let syntax = &mut highlight.style.syntax;
    if let Some(s) = htheme.get("keyword") {
        syntax.keyword = Some(ThemeStyle {
            color: Some(s.fg),
            font_weight: s.bold.then_some(FontWeightContent::Bold),
            font_style: s.italic.then_some(FontStyle::Italic),
        });
    }
    highlight.style.status.error = Some(htheme.error_color);
    theme.highlight_theme = Arc::new(highlight);

Without this change the same code requires a JSON round-trip through
serde, which is brittle (no compile-time check on field names) and
pulls a serde_json dep into the consumer for what is structurally a
trivial value-construction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant