Skip to content

Custom colour themes - #47

Open
AlexanderNZ wants to merge 8 commits into
bgreenwell:develfrom
AlexanderNZ:main
Open

Custom colour themes#47
AlexanderNZ wants to merge 8 commits into
bgreenwell:develfrom
AlexanderNZ:main

Conversation

@AlexanderNZ

@AlexanderNZ AlexanderNZ commented Jun 13, 2026

Copy link
Copy Markdown

This PR addresses #42

Included features:

  • Replaced Theme enum with NamedTheme list. Built-in themes are populated from factory methods while custom themes are appended or overridden (by name, e.g. a user defines in config Dracula that overrides the inbuilt Dracula theme).
  • Custom colour themes defined in config.toml. Users can define themes with #RRGGBB hex values or named colours (red, cyan etc).
  • Added helpers for foreground and background colours while allowing current field names to override these aliases. Foreground and background colours will be applied first, while variables ending in _fg and _bg will be applied over those aliases. Selection, search, and current-row/column/cell colors are excluded so they inherit their contrast from the parent theme. Specific fields like string_fg override aliases.
  • Added an inherits field. inherits allows custom themes to extend other themes (be they built in or custom). Themes resolve sequentially, users will have to ensure that they inherit theme data from themes defined earlier in the config file. Sequential resolution means no circular references can occur.
  • Added --theme <NAME> CLI flag. Selects a theme at launch, overriding any configured defaults. This will error out if you provide input that doesn't match an available theme name. This is very helpful for e.g. people like me who insist that every CLI tool is themed the same way. I run sketchybar with a colour picker widget. xleak can now respect my colour picker.
  • Ensured theme cycling continues to work with t. It will now cycle through built-in themes before cycling through custom themes in the order they appear in config.
  • Warns on stderr if the terminal may not support truecolor and the active theme uses RGB colors

Example Config:

[theme]
default = "tokyonight"

[[theme.custom]]
name = "tokyonight"
inherits = "Dracula"
foreground = "#c0caf5"
background = "#1a1b26"
header_fg = "#7aa2f7"
border_fg = "#565f89"

Testing:

  • 52 tests pass (11 new: color parsing, config parsing, theme resolution, alias override, inheritance, missing parent)
  • cargo clippy --all-targets -- -D warnings clean
  • cargo fmt --all -- --check clean
  • Manually tested: all 6 built-in themes, custom themes, inherits, alias+override, named colors, --theme flag, error cases (invalid color, missing parent, unknown field)

I've had a good poke around with xleak running locally and everything seems to be working well, but I am not a rust expert nor am I an expert in testing, so take that for what it's worth!

Tradeoffs:

  • deny_unknown_fields - adding this provides typo protection at the cost of configs from newer versions (where those versions add fields) will error on older binaries.

@bgreenwell - you mentioned graceful fallback for truecolour. I'll work on that in a separate PR. The rationale there is that ratatui already attempts to gracefully fall back and I wanted to land this PR first. This PR is getting to the point where I feel it is too large so I don't want to bundle more stuff in here.

@AlexanderNZ

Copy link
Copy Markdown
Author

Hope you'll forgive the repeated squashing and amending - tried to keep history clean but I found out too late that there was a cheap way of doing basic colour compatibility checking. Raised #48 to track the more robust way of doing it.

@bgreenwell

Copy link
Copy Markdown
Owner

Found a bug in the background alias before merging.

The issue: In apply_custom_fields (tui.rs), the background alias sets current_row_bg and current_cell_bg to the same value. A user who writes:

[[theme.custom]]
name = "tokyonight"
background = "#1a1b26"

loses row highlighting entirely — every row has the same background color, making navigation harder in a spreadsheet viewer.

Confirmed with a test:

#[test]
fn test_background_alias_row_highlight_distinct() {
    let custom = crate::config::CustomTheme {
        name: "NavTest".into(),
        inherits: Some("Default".into()),
        background: Some(Color::Rgb(26, 27, 38)),
        ..Default::default()
    };
    let themes = resolve_themes(&[custom]).unwrap();
    let t = &themes[6];
    assert_ne!(t.colors.current_row_bg, t.colors.current_cell_bg);
}

Output:

assertion `left != right` failed: background alias collapses current_row_bg and current_cell_bg to the same value, making row highlighting invisible
  left: Rgb(26, 27, 38)
 right: Rgb(26, 27, 38)

Fix: remove current_row_bg from the background alias block. It'll inherit from the parent theme (which already has a contrasting value), and users who want to override it can set current_row_bg explicitly.

Everything else looks great — happy to approve once this is addressed.

@AlexanderNZ

Copy link
Copy Markdown
Author

Argh! Completely missed that. Worse - it actually extends further than just that one case. It looks like foreground and background aliases were both affected.

I decided that aliases should only cover elements that are meant to look uniform. Elements whose purpose are to provide contrast should inherit from parent even when foreground and background aliases have been added. This means that elements that are intended to look different will inherit contrasting colours from the parent where they were designed in.

If users want to override that, they still can, they just have to explicitly set the additional values.

I've also generalised the test you added to catch the broader class of issues and have refactored parts of the code that failed that test.

@bgreenwell

Copy link
Copy Markdown
Owner

Hey @AlexanderNZ, heads up: I just merged #49, which was a big refactor. src/tui.rs is now a tui/ module with the theme code living in src/tui/theme.rs, so this branch won't merge cleanly anymore. Sorry for the churn!

The good news is the new structure should make this feature easier to land, since themes now have their own module instead of being buried in a 2k line file. If you're still up for it, could you rebase onto main and move the custom theme logic into src/tui/theme.rs? Happy to answer questions about the new layout. If you don't have the time, no worries, just let me know and I'll keep #42 open as the tracking issue for someone to pick up.

AlexanderNZ and others added 5 commits July 26, 2026 15:59
Replace the fixed Theme enum with a ThemeSet built at startup from the
built-ins plus any [[theme.custom]] entries in config.toml, so built-in and
user-defined themes are the same thing and theme cycling treats them alike.

Custom themes accept #RRGGBB hex or the 16 named ANSI colors. `inherits`
lets a theme extend another and only restate what differs. Customs resolve
in config order, so a theme can only inherit from one defined before it —
that ordering requirement is what makes circular chains unrepresentable
rather than something to detect. A custom sharing a built-in's name replaces
it in place, keeping cycle order stable.

`foreground` and `background` are broad-brush aliases, applied before the
per-field overrides so specific fields still win. They deliberately skip
every element whose job is to stand out — the cursor cell, current row and
column, and search highlights — which keep the contrast their parent theme
designed in. Setting `background` used to flatten current_row_bg onto it and
make the cursor row invisible; the regression test covers that whole class,
not just the one field.

deny_unknown_fields catches typos like `forground`. The tradeoff is that a
config using a field from a newer xleak fails on an older binary instead of
degrading.

Theme resolution happens in run_tui before the terminal is reconfigured, so
an unresolvable `inherits` fails with a readable message and warnings aren't
swallowed by the alternate screen.

Co-Authored-By: Claude <noreply@anthropic.com>
Group the four display flags (horizontal_scroll, no_header,
no_column_id, no_row_id) into a TuiOptions struct in tui::mod,
removing the #[allow(clippy::too_many_arguments)] on TuiState::new.

Pure refactor — deliberately ordered after the feature commit so
bgreenwell can drop it without unpicking any custom-theme logic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add --theme <NAME> (long-only; -t is taken by --table) to select the
startup theme from the command line. Unknown names are a hard error
listing available themes, while an unknown config default still falls
back gracefully with a warning.

Theme resolution moves from run_tui to main.rs so errors and warnings
surface identically in interactive and non-interactive (--export) mode.

Also fixes the help text ("6 built-in themes" → "available themes")
now that custom themes join the cycle.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three review findings addressed:

Finding 3 — inherit-by-name: when `inherits` is absent, resolve_base
now falls back to an existing theme with the same normalized name
before defaulting to Default. `name = "Dracula"` + one field now
inherits Dracula's palette rather than silently resetting 19 fields.

Finding 4 — scoped truecolor warning: NamedTheme gains a `custom`
flag so the RGB-without-truecolor warning only fires for user-defined
themes. Every non-Default built-in uses Color::Rgb, so warning
unconditionally would nag users with no custom config. Reports the
actual COLORTERM value instead of assuming "not set".

Finding 5 — shared normalization: promote the private `normalized()`
to `utils::normalize_name` and use it from both `theme.rs` and
`config.rs::parse_color`. Now `inherits = "solarized-dark"` resolves
the same way the color name `light-yellow` always did.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Custom Themes section to README with config syntax, inheritance
behaviour, and alias semantics. Update config.toml.example with the
full list of per-field overrides and a commented-out example. Add
the feature, --theme flag, and truecolor warning to CHANGELOG.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…es_rgb

CustomTheme's 20 color fields, apply_custom_fields, and uses_rgb were
three hand-maintained parallel lists — adding a ColorScheme field in
one silently missed the others (exactly the class of bug behind the
original alias review finding).

A single color_field_table! macro in theme.rs now defines every
customizable field with its kind (Color vs Option<Color>) and alias
membership (fg/bg/none). Three consumer macros generate the struct
fields, the apply logic, and the RGB check from that one table.

Tradeoff: CustomTheme is now macro-generated, so it stops being
greppable and drops out of rustdoc. Kept as the last commit so it
can be dropped independently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@AlexanderNZ

AlexanderNZ commented Jul 26, 2026

Copy link
Copy Markdown
Author

Nice refactor. Makes this work a bit cleaner.

I've made four separate commits where it probably could have been one or two, I wanted to break out the refactor of your code from the feature changes so you can drop anything that doesn't sit right without unpicking the rest.

The true colour warning now only nags on custom themes. I didn't want to spam people who hadn't actually touched any of their colour config.

I've added a sixth commit that collapses CustomTheme / apply_custom_fields / uses_rgb into a single macro_rules! table. After my additions those three lists need to stay in sync across two files and it felt like a maintenance trap waiting to happen. But you just refactored that code, so I've broken it out as the last commit in case you'd prefer to drop it.

@bgreenwell
bgreenwell changed the base branch from main to devel August 9, 2026 00:42
@bgreenwell

Copy link
Copy Markdown
Owner

Hey @AlexanderNZ, thanks for the rebase and for splitting the commits the way
you did. Being able to read the macro change on its own made this much easier
to review. Sorry it took a while to get back to you.

Heads up on one thing I changed: I retargeted this PR to devel. That's the
default/integration branch now, and main just tracks releases. Nothing for
you to do, the diff was unaffected.

I went through the whole thing. Short version: this is good, and I want to
land it.

The alias fix. You went further than the bug I reported, and I checked
that it holds by construction rather than just by the test passing. Every
field whose job is contrast (current_cell_fg/bg, current_row_bg,
current_col_fg, and the four search colors) is tagged none in the table,
so an alias physically can't reach them. The generalized test is a superset
of my original assertion. Nice.

inherits. I poked at the cases you didn't call out: forward references,
self-inheritance, and a custom named after a built-in inheriting itself. They
all behave the way I'd want, and rejecting forward references so cycles can't
be expressed is the right trade. Good call not writing a cycle detector.

The macro commit: keeping it. I know you offered to drop it since I'd just
touched that code. It removes about 46 lines net and folds three lists that
had to stay in sync across two files into one table. That sync problem is what
produced the alias bug in the first place, so this is the change that stops it
recurring. Worth the small hit to grep-ability.

deny_unknown_fields: keeping that too. It only covers [[theme.custom]]
rather than the whole config, and with 20 similarly-named color fields the typo
protection is worth more than the forward-compat cost.

One change before I merge:

truecolor_warning is called in main.rs before the if cli.interactive
branch, so xleak book.xlsx --export csv > out.csv can emit a terminal-color
warning on stderr for output that has no colors in it. Could you gate that one
call on cli.interactive? To be clear, I only mean the warning. Resolving
themes eagerly so a bad inherits fails in both modes is right, keep that.

And one wording nit, no code change needed. The warning returns early unless
the active theme is custom, but as your own all_builtins_use_rgb test shows,
every built-in uses RGB. So someone on Terminal.app running built-in Nord, who
is basically the person #48 is about, won't see it. I think the anti-nag
scoping is the right behavior, but the CHANGELOG entry crediting #48 promises
more than it delivers. Mind softening that line to describe just the custom
theme case?

Test coverage is solid, and I appreciate that the tests look themes up by name
instead of hardcoding cycle positions.

Once those two are in I'll merge. You shouldn't need another rebase, #69
touches CHANGELOG.md and README.md too but I'm landing yours first, so that
conflict is theirs to resolve, not yours.

Let me know if you run into issues!

@AlexanderNZ

Copy link
Copy Markdown
Author

Sweet! Made those two changes just now :)

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.

2 participants