Skip to content

Let --show-themes accept --dark and --light together - #2217

Open
VXNCXNX wants to merge 1 commit into
dandavison:mainfrom
VXNCXNX:fix/show-themes-dark-and-light
Open

Let --show-themes accept --dark and --light together#2217
VXNCXNX wants to merge 1 commit into
dandavison:mainfrom
VXNCXNX:fix/show-themes-dark-and-light

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 15, 2026

Copy link
Copy Markdown

What's broken

--show-themes --dark --light is documented and does not work.

The help text for show_themes in src/cli.rs says:

To control the themes shown, use --dark or --light, or both, on the command line together with this option.

But:

$ delta --show-themes --dark --light
--light and --dark cannot be used together.

The cause

validate_light_and_dark in src/options/set.rs runs before any subcommand dispatch, so it kills --show-themes along with everything else.

The intent is still in the tree: src/subcommands/show_themes.rs:64 has a || (dark && light) branch that is currently unreachable. It was written for exactly this.

The fix

Exempt --show-themes from the mutual-exclusion check, which re-arms that branch. Nothing in show_themes.rs changes.

Two things I checked

opt.light and opt.dark feed only theme::get_color_mode, where light wins if both are set. That resolves to a valid, fully populated config, and it is only used for the outer shell: the per-theme configs inside the loop are rebuilt from --features <theme>, so the outer colour mode does not affect what is rendered.

--show-syntax-themes makes no such promise, its help text never mentions the flags, so I did not extend the fix to it. --show-syntax-themes --dark --light still exits fatally, the same before and after.

Verification

With a HOME holding a themes gitconfig containing one dark and one light theme:

--dark           -> 1 theme
--light          -> 1 theme
--dark --light   -> 2 themes

And the guard still does its job for normal use:

$ delta --dark --light
--light and --dark cannot be used together.

cargo fmt --all --check, cargo clippy -- -D warnings, cargo build --release and cargo test (438 passed) are all clean, plus tests/test_deprecated_options.

The new integration test spawns the binary with its own HOME, because GitConfig::try_create returns None under cfg(test), so a unit test finds no themes at all. Reverting set.rs with the test kept makes it fail.

One note unrelated to this change: cargo clippy --all-targets -- -D warnings reports 4 needless_borrow errors in src/utils/process.rs. I confirmed those on a clean tree. CI's gate is cargo clippy -- -D warnings without --all-targets, which passes.

There is no issue for this one, I noticed it while reading the help text.

The help text documents using both flags with --show-themes to see both
kinds, but validate_light_and_dark ran before subcommand dispatch and
killed it. The (dark && light) branch in show_themes.rs was already there,
just unreachable.
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