Skip to content

feat(ui): expose Shiki syntax themes#450

Open
benvinegar wants to merge 1 commit into
mainfrom
feat/shiki-syntax-theme
Open

feat(ui): expose Shiki syntax themes#450
benvinegar wants to merge 1 commit into
mainfrom
feat/shiki-syntax-theme

Conversation

@benvinegar

@benvinegar benvinegar commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Add syntax_theme config and --syntax-theme CLI support so UI themes and Shiki syntax themes can be chosen independently.
  • Thread the active syntax theme through Pierre/Shiki highlighter loading and highlight caches.
  • Source built-in Catppuccin code highlighting from Shiki's bundled Catppuccin themes by default.
  • Replace the t theme-cycle shortcut with an opencode-style selector modal for UI themes and bundled Shiki syntax themes (↑/↓, Enter, Esc).
  • Document the new option and add regression coverage for config/CLI parsing, Catppuccin Shiki token colors, and the selector shortcut.

Refs #415.

Testing

  • bun run format
  • bun run typecheck
  • bun run lint
  • bun test src/ui/AppHost.interactions.test.tsx src/ui/components/ui-components.test.tsx src/ui/lib/ui-lib.test.ts src/ui/diff/pierre.test.ts src/core/config.test.ts src/core/cli.test.ts src/ui/themes.test.ts
  • bun test — 1058 pass, 14 skip, 0 fail

This PR description was generated by Pi using OpenAI GPT-5 Codex (2026-06-16)

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a syntax_theme config key and --syntax-theme CLI flag, letting users choose a Shiki syntax color theme independently from Hunk's UI theme. Catppuccin UI themes now automatically wire up their matching Shiki Catppuccin bundle as the default syntax theme.

  • Config/CLI plumbing: syntax_theme is parsed via normalizeString, propagated through mergeOptions/resolveConfiguredCliInput, and exposed as --syntax-theme in the CLI. Priority order (CLI > repo config > global config) is respected by the existing merge logic.
  • Highlighting pipeline: loadHighlightedDiff/loadHighlightedSourceLines now accept a full AppTheme (via the HighlightThemeInput union) so the Shiki theme name is resolved from theme.syntaxTheme ?? pierreThemeName(theme.appearance); highlight caches in useHighlightedDiff and useHighlightedSource now key on theme.id:syntaxTheme:file_id:fingerprint.
  • Catppuccin default: createCatppuccinTheme sets syntaxTheme: \\catppuccin-${flavor}\`` so Catppuccin UI themes source code colors from Shiki's bundled Catppuccin palettes out of the box; buildCustomTheme clears `syntaxTheme` when explicit per-role syntax overrides are present to avoid the Shiki theme silently overriding hand-tuned semantic colors.

Confidence Score: 4/5

Safe to merge. The change is well-scoped and thoroughly tested; the main behavior gap is the catch block dropping the custom syntax theme when a language grammar fails.

The plumbing from CLI/config through to the Shiki highlighter and both highlight caches is consistent and the Catppuccin default wiring is verified end-to-end by the new pierre.test.ts assertions. The two notes are a fallback path that silently loses the configured syntax theme on language-grammar failures, and a redundant hook dependency that could cause harmless extra effect runs — neither affects correctness in the common case.

src/ui/diff/pierre.ts (catch block fallback behavior) and src/ui/diff/useHighlightedDiff.ts / src/ui/diff/useHighlightedSource.ts (redundant theme dependency).

Important Files Changed

Filename Overview
src/ui/diff/pierre.ts Refactored to accept full AppTheme for syntax highlighting; introduces HighlightThemeInput union for backward compat. Fallback catch blocks drop the configured syntaxTheme unconditionally.
src/ui/diff/useHighlightedDiff.ts Cache key updated to include theme.id and syntaxTheme. Redundant theme in the useLayoutEffect dep array alongside appearanceCacheKey.
src/ui/themes.ts Adds withSyntaxTheme helper; buildCustomTheme correctly clears syntaxTheme when explicit syntax color overrides are present.
src/ui/themes/catppuccin.ts Sets syntaxTheme to the matching Shiki Catppuccin bundle name for each flavor — straightforward and verified by the new pierre.test.ts assertions.
src/core/config.ts Reads syntax_theme from TOML via normalizeString and threads it through mergeOptions/resolveConfiguredCliInput without issues.
src/core/cli.ts Adds --syntax-theme CLI flag cleanly via applyCommonOptions; correctly propagates through buildCommonOptions.
src/ui/App.tsx activeTheme memoization updated to apply withSyntaxTheme before withTransparentBackground; dependencies are correct and stable.
src/ui/diff/useHighlightedSource.ts Mirrors useHighlightedDiff changes; same redundant theme dep alongside cacheKey in the useLayoutEffect.
src/ui/staticDiffPager.ts Correctly applies withSyntaxTheme after resolveTheme and passes full AppTheme to loadHighlightedDiff.
src/ui/themes/types.ts Adds optional syntaxTheme field to AppTheme and ThemeBase; clean addition.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CLI --syntax-theme / config syntax_theme"] --> B["resolveConfiguredCliInput\n(syntaxTheme in CommonOptions)"]
    B --> C["App.tsx: withSyntaxTheme(baseTheme, syntaxTheme)\n→ activeTheme.syntaxTheme"]
    C --> D{syntaxTheme set?}
    D -- yes --> E["highlighterThemeName → theme.syntaxTheme\ne.g. 'catppuccin-mocha' / 'dracula'"]
    D -- no --> F["highlighterThemeName → pierreThemeName(appearance)\n'pierre-dark' / 'pierre-light'"]
    E --> G["prepareHighlighter(language, theme)\ncacheKey: syntaxThemeName:language"]
    F --> G
    G --> H{language found?}
    H -- yes --> I["renderDiffWithHighlighter\nwith Shiki theme colors"]
    H -- no --> J["catch: fallbackTheme = appearance string\nprepareHighlighter('text', 'dark'/'light')\n⚠ syntaxTheme dropped"]
    I --> K["SHARED_HIGHLIGHTED_DIFF_CACHE\nkey: theme.id:syntaxTheme:file_id:fp"]
    J --> K
    C --> L["Catppuccin themes\nsyntaxTheme = 'catppuccin-{flavor}'"]
    L --> D
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["CLI --syntax-theme / config syntax_theme"] --> B["resolveConfiguredCliInput\n(syntaxTheme in CommonOptions)"]
    B --> C["App.tsx: withSyntaxTheme(baseTheme, syntaxTheme)\n→ activeTheme.syntaxTheme"]
    C --> D{syntaxTheme set?}
    D -- yes --> E["highlighterThemeName → theme.syntaxTheme\ne.g. 'catppuccin-mocha' / 'dracula'"]
    D -- no --> F["highlighterThemeName → pierreThemeName(appearance)\n'pierre-dark' / 'pierre-light'"]
    E --> G["prepareHighlighter(language, theme)\ncacheKey: syntaxThemeName:language"]
    F --> G
    G --> H{language found?}
    H -- yes --> I["renderDiffWithHighlighter\nwith Shiki theme colors"]
    H -- no --> J["catch: fallbackTheme = appearance string\nprepareHighlighter('text', 'dark'/'light')\n⚠ syntaxTheme dropped"]
    I --> K["SHARED_HIGHLIGHTED_DIFF_CACHE\nkey: theme.id:syntaxTheme:file_id:fp"]
    J --> K
    C --> L["Catppuccin themes\nsyntaxTheme = 'catppuccin-{flavor}'"]
    L --> D
Loading

Comments Outside Diff (1)

  1. src/ui/diff/pierre.ts, line 663-677 (link)

    P2 Fallback drops syntax theme even on language-grammar failure

    Both catch blocks in loadHighlightedDiff and the matching block in loadHighlightedSourceLines call highlightThemeAppearance(theme) unconditionally, reducing the active theme to just "light" or "dark" (i.e. Pierre's default). When the primary failure is a missing language grammar (e.g. COBOL or Brainfuck) rather than an invalid Shiki theme, the fallback unnecessarily discards the user's configured syntax_theme. A user who sets syntax_theme = "nord" and opens an unsupported-language file will see Pierre-dark colors in the fallback instead of Nord, with no indication that the theme was dropped.

    A two-stage fallback — first retry with "text" grammar but the same theme, then fall back to Pierre on a second catch — would preserve the configured syntax theme when only the language is at fault.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/ui/diff/pierre.ts
    Line: 663-677
    
    Comment:
    **Fallback drops syntax theme even on language-grammar failure**
    
    Both catch blocks in `loadHighlightedDiff` and the matching block in `loadHighlightedSourceLines` call `highlightThemeAppearance(theme)` unconditionally, reducing the active theme to just `"light"` or `"dark"` (i.e. Pierre's default). When the primary failure is a missing language grammar (e.g. COBOL or Brainfuck) rather than an invalid Shiki theme, the fallback unnecessarily discards the user's configured `syntax_theme`. A user who sets `syntax_theme = "nord"` and opens an unsupported-language file will see Pierre-dark colors in the fallback instead of Nord, with no indication that the theme was dropped.
    
    A two-stage fallback — first retry with `"text"` grammar but the same `theme`, then fall back to Pierre on a second catch — would preserve the configured syntax theme when only the language is at fault.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/ui/diff/pierre.ts:663-677
**Fallback drops syntax theme even on language-grammar failure**

Both catch blocks in `loadHighlightedDiff` and the matching block in `loadHighlightedSourceLines` call `highlightThemeAppearance(theme)` unconditionally, reducing the active theme to just `"light"` or `"dark"` (i.e. Pierre's default). When the primary failure is a missing language grammar (e.g. COBOL or Brainfuck) rather than an invalid Shiki theme, the fallback unnecessarily discards the user's configured `syntax_theme`. A user who sets `syntax_theme = "nord"` and opens an unsupported-language file will see Pierre-dark colors in the fallback instead of Nord, with no indication that the theme was dropped.

A two-stage fallback — first retry with `"text"` grammar but the same `theme`, then fall back to Pierre on a second catch — would preserve the configured syntax theme when only the language is at fault.

### Issue 2 of 2
src/ui/diff/useHighlightedDiff.ts:214-217
**Redundant `theme` dependency in `useLayoutEffect`**

`appearanceCacheKey` is derived from `buildCacheKey(theme, file)`, which already encodes `theme.id`, `theme.syntaxTheme`, and `theme.appearance` — every property of the theme that drives a different highlight result. Listing `theme` separately means the effect also re-runs whenever non-highlight fields change (UI colors, border colors, etc.) without affecting the cache key, causing a redundant cache lookup and no-op. The same applies to `useHighlightedSource.ts` line 71 where `theme` is listed alongside `cacheKey`. Removing `theme` from both dependency arrays would rely solely on the cache key, which is the intended discriminator.

Reviews (1): Last reviewed commit: "feat(ui): expose Shiki syntax themes" | Re-trigger Greptile

Comment on lines 214 to +217
return () => {
cancelled = true;
};
}, [appearance, appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight]);
}, [appearanceCacheKey, file, highlightedCacheKey, shouldLoadHighlight, theme]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant theme dependency in useLayoutEffect

appearanceCacheKey is derived from buildCacheKey(theme, file), which already encodes theme.id, theme.syntaxTheme, and theme.appearance — every property of the theme that drives a different highlight result. Listing theme separately means the effect also re-runs whenever non-highlight fields change (UI colors, border colors, etc.) without affecting the cache key, causing a redundant cache lookup and no-op. The same applies to useHighlightedSource.ts line 71 where theme is listed alongside cacheKey. Removing theme from both dependency arrays would rely solely on the cache key, which is the intended discriminator.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/diff/useHighlightedDiff.ts
Line: 214-217

Comment:
**Redundant `theme` dependency in `useLayoutEffect`**

`appearanceCacheKey` is derived from `buildCacheKey(theme, file)`, which already encodes `theme.id`, `theme.syntaxTheme`, and `theme.appearance` — every property of the theme that drives a different highlight result. Listing `theme` separately means the effect also re-runs whenever non-highlight fields change (UI colors, border colors, etc.) without affecting the cache key, causing a redundant cache lookup and no-op. The same applies to `useHighlightedSource.ts` line 71 where `theme` is listed alongside `cacheKey`. Removing `theme` from both dependency arrays would rely solely on the cache key, which is the intended discriminator.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar benvinegar force-pushed the feat/shiki-syntax-theme branch from ee3a22e to 0fad3b1 Compare June 17, 2026 03:49
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