Skip to content

Themes: Accept a dark/light theme table - #985

Open
masonmcelvain wants to merge 9 commits into
modem-dev:mainfrom
masonmcelvain:feat/adaptive-theme-config
Open

Themes: Accept a dark/light theme table#985
masonmcelvain wants to merge 9 commits into
modem-dev:mainfrom
masonmcelvain:feat/adaptive-theme-config

Conversation

@masonmcelvain

Copy link
Copy Markdown
Contributor

Support a [theme] table in config.toml to set dark/light themes:

[theme]
dark = "catppuccin-mocha"
light = "catppuccin-latte"
fallback = "github-dark-default"  # optional

fallback covers sessions where terminal background is unknown, defaulting to dark.

Writing the pair back exposed that the config writer was rewriting more of config.toml than it should, so that is fixed here too.

Changes

Theme selection

  • Add src/core/theme/selection.ts: a ThemeSelection is either a theme id or a { dark, light, fallback? } pair, and the module owns reading, comparing, and resolving one against a terminal background.
  • Probe the terminal background for a pair the same way auto does.
  • --theme <id> overrides a configured pair for one run.
  • Picking a theme in the app replaces the pair with that single id. The save-on-quit prompt renders the collapse before anything is written. - Keep the committed selection separate from the id it resolves to: persistence and in-session refresh carry the pair, while extension theme_changed events keep receiving a concrete id.
  • Reject a table that sets only one background, names an unknown key, or gives a non-string id, naming the offending file and key path. An unknown id is still tolerated, since an extension may contribute it later.

Config writing

  • Rewrite an existing theme in place instead of appending a second definition, including quoted ("theme" = …) and dotted (theme.dark = …) key forms, and drop duplicate definitions rather than leaving them behind.
  • Preserve trailing comments on a rewritten line and comments inside a [theme] table; keep a comment attached to the table it documents.
  • Collapse a [theme] table to a top-level theme = "…" when a single id replaces the pair, leaving one blank line between the neighbours it separated.
  • Write a pair as an inline table when the file has no [theme] section.

QA

Themes used below are interchangeable; any two built-in ids work.

  1. A pair follows the terminal. Put a [theme] table in ~/.config/hunk/config.toml with distinguishable dark/light ids, then run hunk diff in a dark terminal and again in a light one. Each should draw its own side.
  2. Fallback. Add fallback and run through a host that does not answer the background query (a script-allocated pty, or LazyGit as a pager). The fallback theme should render; remove fallback and dark should.
  3. Flag wins. hunk diff --theme github-dark-default with the table still in config should ignore the pair.
  4. Collapse on pick. With the table configured, press t, choose a theme, then q. The prompt should show - theme = { dark = "…", light = "…" } / + theme = "…", and saving should leave a top-level theme = "…" with no [theme] table.
  5. Nothing else in the file moves. Before step 4, add comments, a [custom_theme] table, and an [extensions] table around the [theme] table. After saving, they should be untouched, with no doubled blank lines, and the file should still parse.
  6. Refresh. With a pair configured, press r, then t. One side of the pair should still be active, and quitting should not offer to save a theme change.
  7. Bad tables. A table missing light, or with a typo'd key, should exit 1 naming the file and the problem.

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@vercel

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown

@masonmcelvain is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@benvinegar

Copy link
Copy Markdown
Member

Really like the selection vs. resolved theme split here, and the overall behavior makes sense. I found a few things I think we should address before merging:

  • This currently conflicts with main, including the packages/hunk move. After resolving the obvious conflicts locally, typechecking still fails because the new ThemeSelection type needs to be carried through the recently added history surfaces.
  • The config writer can corrupt valid TOML. For example, ["theme"] is accepted when reading, but saving adds a second top-level theme, leaving the file unparsable. A [theme] example inside a multiline extension string can also be mistaken for the real table and partially deleted.
  • Collapsing [theme] still drops some comments, including comments on the table header and inside the table.
  • Terminal probing happens before extension-backed repo discovery finalizes the effective config. If that second config resolution introduces an adaptive pair, we can end up using the dark/fallback side in a light terminal.
  • Saving a selection over [diff.theme] writes a top-level theme but leaves the command-scoped table in place, so the saved choice is overridden again on the next run.

I'd strongly suggest validating the generated TOML before replacing the config file, and avoiding rewriting theme at all when its semantic value hasn't changed. Tests around quoted headers, multiline strings, command-scoped themes, and extension-discovered repo config would help here.

The feature and core model look good, but I don't think the config-writing path is safe to merge as-is.

masonmcelvain and others added 4 commits September 7, 2026 08:10
`theme = "auto"` only ever chose between github-light-default and
github-dark-default, so a reader who wanted one-light and one-dark-pro
had no way to follow their terminal. Accept a `[theme]` table naming both
sides instead, with an optional `fallback` for terminals that never
answer the background probe, following Helix's config shape.

The committed preference now stays whatever config asked for until
someone picks a theme in the selector, so quitting without touching
themes no longer rewrites `auto` — or a pair — into the one id it
happened to resolve to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Y3vNR6iJEH4epjQo8JARe
The theme writer only recognized the `theme = ` and `[theme]` spellings,
while the reader takes whatever Bun.TOML.parse produced. A config written
as `theme.dark = "nord"` read fine, then gained a second `theme = "..."`
assignment on the next preference save, after which Hunk refused to start
with `BuildMessage: Cannot redefine key 'theme'` — an error naming neither
the file nor the key. A quoted key inside `[theme]` failed the same way.
Match every spelling TOML accepts for a key, and drop the extra lines a
dotted key spreads a value over.

Saves also deleted comments. The `[theme]` range ran to the next section
header, so collapsing the table took the blank lines and comments that
introduce whatever follows, and every rewritten key lost its trailing
comment. Since one save rewrites all nine preferences, toggling
`wrap_lines` stripped comments out of an untouched `[theme]` table.
Comments now stay attached to the key or section they introduce, which is
also why a collapsed table is written over its own header rather than
appended — but only while `[theme]` is the first table, since a top-level
key at any later position would scope into the table above it.

Theme errors named `theme` whichever section or file held the bad table.
That matters more now that a checked-in `.hunk/config.toml` can hard-fail
startup for everyone in the repo, so they name both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BLvy5LioGNhwJHzDPb7xxy
Saving view preferences could leave a config file slightly messier than it
found it. Collapsing a [theme] table that sat between two other tables left
the blank line from each side, and a file with no tables at all had new keys
inserted above its trailing comment, because the backtrack that keeps a
comment attached to the table it documents could not tell a real table index
from the clamp used when no table exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJVQRku6dLXkxiACCy177U
An in-session refresh rebuilt its reload input from the theme id the
selection currently resolved to, so an adaptive pair arrived at the reload as
whichever side the terminal happened to be on. Nothing collapses today
because every refresh path keeps the mounted App, but the descriptor is the
wrong thing to freeze. Carry the committed selection instead, and leave the
resolved id to extension events, which want a concrete theme.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJVQRku6dLXkxiACCy177U
@masonmcelvain
masonmcelvain force-pushed the feat/adaptive-theme-config branch from c66827f to 904a4d4 Compare September 7, 2026 14:28
masonmcelvain and others added 5 commits September 7, 2026 08:37
Keep the selection beside the input in the history surface, resolve
it per terminal in the interactive surface, and let static output fall
through to the pair's fallback side since it never probes the terminal.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6
The line-based writer matched `[theme]` and `theme =` by regex, so it
missed quoted headers like `["theme"]` and added a second root key,
mistook a `[theme]` example inside a multiline string for the real
table, and dropped comments on the header and inside the table when a
single id replaced a pair. It also rewrote every preference on every
save, so a mode-only change could still corrupt an untouched theme.

Replace the regex matching with a small line scanner that tracks
multiline strings and bracketed values and compares headers and keys by
parsed path, hoist a collapsed table's comments onto the key, and only
rewrite keys that differ from the loaded baseline. Before replacing the
file, parse the candidate and require it to equal the original document
with just the edited keys changed; otherwise leave the file alone and
say what to set by hand. Writes go through a sibling temp file so a
crash cannot truncate the config.

Saves also learn the command and pager tables that shaped the session,
so a key those tables define is written back there instead of to a root
key the next run would override.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6
Carry the resolved command and pager scope from config resolution into
the review and history bootstraps and on to the quit prompt, so a theme
picked under a `[diff.theme]` or `[pager]` override lands in that table
rather than in a root key the override would shadow on the next run.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6
Startup asked the terminal for its background right after the first
config resolution, but an extension VCS backend can settle a repo root
the bundled catalog could not, and that repo's config is only read in
the second resolution. A `[theme]` pair introduced there never triggered
the probe, so a light terminal drew the dark or fallback side.

Move the probe after extension-backed resolution so it sees the final
selection.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6
@masonmcelvain
masonmcelvain force-pushed the feat/adaptive-theme-config branch from 904a4d4 to d2a509b Compare September 7, 2026 16:32
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.

Extend config file to set dark/light themes

2 participants