Skip to content

cli: use the terminal palette for low-confidence groups - #1538

Open
shivamtiwari3 wants to merge 1 commit into
google:mainfrom
shivamtiwari3:cli-palette-low-confidence-colors
Open

shivamtiwari3 wants to merge 1 commit into
google:mainfrom
shivamtiwari3:cli-palette-low-confidence-colors

Conversation

@shivamtiwari3

Copy link
Copy Markdown

Fixes #1243.

Summary

The Rust CLI's fallback color for content type groups without an explicit mapping (including the common "text" and "unknown", plus "binary", "database", "font", "geometry", "gis", "model", "scientific", "inode" and "undefined") was a fixed light gray, #cccccc:

_ => result.bold().truecolor(0xcc, 0xcc, 0xcc),

On a light-background terminal this is barely visible, which is what #1243 reports ("The very light gray used by low-quality detection reports works very poorly there").

Fix

Use the terminal palette for the fallback instead of a fixed truecolor:

_ => result.bold().bright_black(),

bright_black maps to the terminal palette (ANSI bright black), so it renders as a dark gray on light backgrounds and as a light gray on dark backgrounds. It adapts to the user's theme, so no --light-mode flag or extra configuration is needed. This follows the preference already expressed on #1243 by @ia0 that using the palette is the safest choice.

The eight highlighted groups keep their deliberate Tailwind truecolors; only the low-confidence fallback changes.

Testing

  • rust/cli/test.sh: added a regression test that runs the CLI with --colors on a plain .txt file (group "text"), asserts the output contains the palette code (\x1b[1;90m) and does not contain the old #cccccc truecolor (\x1b[1;38;2;204;204;204m).
  • cargo build, cargo clippy -- --deny=warnings and cargo fmt -- --check are clean.
  • Manually verified: magika --colors text.txt now emits \x1b[1;90m...\x1b[0m.

Notes

This is the Rust CLI counterpart of the now-closed #1317 (which targeted the removed Python fallback client). Opened as requested by @ebursztein on #1317, where #1243 was left open for the Rust CLI. Happy to switch to a --light-mode flag or a --color-file option (both suggested on #1243) if you prefer that direction.

…#1243)

The fallback color for content type groups without an explicit truecolor
mapping (including "text", "unknown", "binary", "font" and "inode") was a
fixed light gray (#cccccc). On light-background terminals this is barely
readable, as reported in google#1243.

Use the terminal palette instead: bright black renders as a dark gray on
light backgrounds and as a light gray on dark backgrounds, so it adapts to
the user's theme without needing a new flag. This follows the preference
noted in google#1243 that the palette is the safest choice.

Add a regression test that checks the "text" group output uses the palette
code and not the previous #cccccc truecolor.
@shivamtiwari3

Copy link
Copy Markdown
Author

@ia0 since you own the Rust code, could you take a look when you have a moment? It's a small change in rust/cli/src/main.rs (one line) plus a test in rust/cli/test.sh.

The Rust CI job hasn't run yet — it may be waiting on a maintainer to approve workflows for this fork PR. Locally I get clean cargo build, cargo clippy -- -D warnings, and cargo fmt --check.

Heads up that #1531 also touches rust/cli/src/main.rs; if it lands first I'll rebase. I'm also happy to switch this to a --light-mode flag or --color-file option if that's the direction you'd prefer over the palette.

This branch has not been deployed

No deployments
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.

Light gray on white is no good

1 participant