Skip to content

Respect NO_COLOR values and explicit color settings - #1240

Open
fzlzjerry wants to merge 1 commit into
lsd-rs:mainfrom
fzlzjerry:fix/1060-no-color-semantics
Open

fzlzjerry wants to merge 1 commit into
lsd-rs:mainfrom
fzlzjerry:fix/1060-no-color-semantics

Conversation

@fzlzjerry

Copy link
Copy Markdown

Addresses #1060 and the configuration/default interaction discussed in #544.

NO_COLOR is already supported, but the environment check treats an empty value as active and ignores non-Unicode values on Unix. Explicit color.when: always also loses to the environment, contrary to the configuration override guidance.

This change:

  • Reads NO_COLOR as an OS string and only treats nonempty values as active.
  • Lets explicit always/never color configuration override the environment. auto, including the built-in default, still honors NO_COLOR, so the default configuration does not accidentally turn the variable off.
  • Preserves command-line overrides and classic-mode behavior. Theme definitions and the generic precedence for other flags are unchanged.
  • Adds unit/CLI coverage and documents the behavior in the README, man page and config samples.

Validation on Linux:

  • Full Cargo-built test binaries pass with Rust 1.85.0 and 1.98.0, default and no-default features, with NO_COLOR both unset and set. Each default run has 443 tests; each no-default run has 439. Runs use non-root private namespaces, including the permission-sensitive tests.
  • Four color unit cases and the new CLI regression fail on the original implementation and pass with the change.
  • 17 real pseudo-terminal cases pass for each of the four version/feature binaries, covering empty/nonempty/non-Unicode values, no user config, explicit config and CLI overrides.
  • The CI-pinned Rust 1.85.0 passes formatting, cargo clippy --locked --tests -- -D warnings and the clap/deprecated check. Rust 1.98.0's stricter Clippy reports the same 10 existing diagnostics on the clean base and this patch, all in unchanged files; its formatting and clap/deprecated check pass.
  • The man page builds with the project's Pandoc command. Dependencies and the lockfile are unchanged.

TODO

  • Use cargo fmt
  • Add necessary tests
  • Update README
  • Update config sample file in doc/samples
  • Update man page at doc/lsd.md

Read NO_COLOR as an OS string and ignore empty values. Nonempty values, including non-Unicode values on Unix, disable automatic color.

Let explicit always/never color configuration override the environment while keeping auto responsive to NO_COLOR. Command-line resolution remains first, and the generic ordering for other flags is unchanged.

Add unit and CLI coverage for environment values, built-in defaults, and overrides, and document the behavior in the README, man page, and config samples. Full test binaries pass with Rust 1.85 and 1.98 under default/no-default features, with NO_COLOR both unset and set.

Refs: lsd-rs#1060
@fzlzjerry
fzlzjerry requested a review from zwpaper as a code owner September 8, 2026 21:06
Copilot AI lite review requested due to automatic review settings September 8, 2026 21:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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