Skip to content

fix: make -h/--human-readable actually apply - #1233

Open
VXNCXNX wants to merge 1 commit into
lsd-rs:mainfrom
VXNCXNX:fix/human-readable-flag
Open

fix: make -h/--human-readable actually apply#1233
VXNCXNX wants to merge 1 commit into
lsd-rs:mainfrom
VXNCXNX:fix/human-readable-flag

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes #880.

The problem

-h / --human-readable is parsed and then never used:

$ grep -rn human_readable src/
src/app.rs:61:    human_readable: bool,

One hit, the declaration. Nothing reads it. So the flag has no effect at all:

$ lsd -l --size bytes -h
5000                 <- raw bytes, -h ignored

The fix

Make -h and --size override each other, so the later flag wins:

lsd -l --size bytes -h            ->  4.9 KB     (was 5000)
lsd -l -h --size bytes            ->  5000       (--size wins, as it should)
lsd -l --size bytes -h --size short -> 4.9M      (last of three wins)

This uses clap's overrides_with, which the repo already relies on for all/almost_all in the same file, and args_override_self = true is already set on the struct, so it's the existing idiom rather than manual index comparison.

--classic still takes precedence over both, unchanged. Config-file size: still works and the CLI still beats it:

config size: bytes                 ->  5000
config size: bytes  +  CLI -h      ->  4.9 KB

One change that needs a sentence

human_readable had to become pub so flags/size.rs can read it, which left help: () as the struct's only private field, and that newly trips clippy's manual_non_exhaustive. I made help pub too, matching every other field.

I checked whether this is a public API concern: it isn't. lsd has no lib target (only [[bin]] in Cargo.toml) and mod app; is private in main.rs, so Cli is unreachable from outside the crate. The alternative clippy suggests, #[non_exhaustive], would be wrong here since clap needs the field.

Tests

Four tests added to flags/size.rs's existing test module, covering both orderings, --classic precedence, and the config interaction.

cargo build:         clean
cargo test:          395 passed (unit), 44 passed (integration)
cargo fmt --check:   clean
cargo clippy --all-targets: no new warnings in the changed files

One integration test, test_date_custom_format_supports_nanos_with_length, fails in my environment on a locale decimal separator. It fails identically on a clean checkout, so it's unrelated to this change.


TODO

  • Use cargo fmt (cargo fmt --check clean)
  • Add necessary tests (four in flags/size.rs)
  • Update README (if applicable) (not applicable: the README already documents -h/--human-readable; this makes the documented behaviour true rather than changing it)
  • Update config sample file in doc/samples (if applicable) (not applicable: no config key is added or changed, the size: key is unchanged)
  • Update icon sample file in doc/samples (if applicable) (not applicable: no icon change)
  • Update color sample file in doc/samples (if applicable) (not applicable: no color change)
  • Update man page at lsd/doc/lsd.md (if applicable) (not applicable: the man page already documents -h/--human-readable)

Disclosure: written with AI assistance (Claude Code). I reproduced the issue, ran the change and the verification myself.

the flag was parsed and then referenced nowhere, so lsd -l --size bytes -h still printed raw bytes. Make -h and --size override each other so the later flag wins, which is what the rest of the CLI does. Closes lsd-rs#880.
@VXNCXNX
VXNCXNX requested a review from zwpaper as a code owner August 14, 2026 19:55
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.

Feature request: -h should override previous --size

1 participant