fix: color names ignore case, and bright requires a standard color, like in Git. - #2863
Merged
Sebastian Thiel (Byron) merged 2 commits intoAug 1, 2026
Conversation
… like in Git.
`Name::from_str()` compared color names with `==` against lowercase literals and
stripped the `bright` prefix with a case-sensitive `strip_prefix()`, so every
spelling Git accepts but that is not all-lowercase was rejected. Measured against
`git config --file <f> --get-color color.test` with git 2.50.1 (Apple Git-155):
input git gix before gix after
RED ESC[31m error red
NoRmAl (empty, i.e. normal) error normal
DEFAULT ESC[39m error default
BrightRed ESC[91m error brightred
brightRED ESC[91m error brightred
RED brightBLUE bold ESC[1;31;104m error red brightblue bold
The same function stripped `bright` and then fell through to the ANSI and hex
fallbacks without consulting the flag again, so a `bright` prefix on anything
that is not one of the eight standard colors was silently dropped:
input git gix before gix after
bright0 invalid color value 0 error
bright1 invalid color value 1 error
bright255 invalid color value 255 error
bright#ff0010 invalid color value #ff0010 error
The eight standard colors are matched from a table with `eq_ignore_ascii_case()`,
mirroring `color_names[]` in Git's `color.c`, and a `bright` prefix that did not
resolve there is now an error instead of falling through. `normal`, `default` and
`-1` keep their existing meaning; attributes stay case-sensitive, which is what
Git does too (`git config --get-color` rejects `BOLD`, `NOBOLD` and `NO-BOLD`).
Errors now report the whole input rather than the remainder after `bright` was
stripped.
Over 67 probed inputs, gix disagreed with git on 28 before and disagrees on 3
after; the three are `RESET`, `Reset` and `red RESET`, because Git also matches
the `reset` attribute case-insensitively. That is left alone here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
Sebastian Thiel (Byron)
enabled auto-merge
August 1, 2026 16:44
Member
|
Thanks a lot, much appreciated! |
Shuvam Kumar (shuvamk)
deleted the
fix/color-name-case-and-bright-prefix
branch
August 1, 2026 17:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written by an AI agent (Claude) operating through the
shuvamkaccount, per the agent impersonation policy.What's wrong (AI)
gix_config_value::color::Name::from_str()compares color names with==against lowercaseliterals and strips the
brightprefix with a case-sensitivestrip_prefix("bright"). Gitcompares them with
strncasecmp(match_word()incolor.c), so every spelling Git acceptsthat is not all-lowercase is rejected by
gix.Measured with
git version 2.50.1 (Apple Git-155)againstgixat77dc1ff87, usingprintf '[color]\n\ttest = "%s"\n' "$input" > f; git config --file f --get-color color.teston one side and
gix_config_value::Color::try_from(input)on the other:git --get-colorColor::try_fromonmainREDESC[31mredrEdESC[31mredNoRmAlnormalDEFAULTESC[39mdefaultBLACKESC[30mblackBRIGHTREDESC[91mbrightredBrightRedESC[91mbrightredbrightREDESC[91mbrightredRED brightBLUE boldESC[1;31;104mred brightblue boldSecond,
from_str()stripsbrightand then falls through to the ANSI and hex fallbackswithout consulting the flag again, so a
brightprefix on anything that is not one of theeight standard colors is silently dropped instead of being an error:
git --get-colorColor::try_fromonmainbright0error: invalid color value: bright00bright1error: invalid color value: bright11bright255error: invalid color value: bright255255bright#ff0010error: invalid color value: bright#ff0010#ff0010Both have been there since the code was moved into this crate in
edb1162e28(#450) —git blameshows every line of the match block still at that commit — and a search of the issuetracker for colour parsing turned up nothing that chooses this behaviour deliberately.
Over the 67 inputs I probed,
gixdisagrees withgiton 28 before this change and on3 after — see the last section for the three.
What this does
Name::from_str()now matches the eight standard colors from a table witheq_ignore_ascii_case(), mirroringcolor_names[]in Git'scolor.c, and abrightprefixthat does not resolve against that table is an error rather than falling through to the ANSI
and hex parsers.
normalanddefaultare matched case-insensitively too, and-1keeps itsmeaning as an alias for
normal. The prefix test usessplit_at_checked(), so an input whosemulti-byte character straddles byte 6 still cannot panic (
f9d566f82) — I checkedbrighét,brigh€xandbrighté, all of which return an error.What deliberately does not change: attributes stay case-sensitive, with one exception
documented below, because Git's
parse_attr()compares them withmemcmp—git config --get-colorrejectsBOLD,NOBOLDandNO-BOLD, and so doesgix, before and after.One incidental improvement: an error now reports the whole input rather than the remainder
after
brightwas stripped, sobrightfooreportsbrightfooinstead offoo.eq_ignore_ascii_case()is already how the siblingboolean.rsin this crate matchesyes/on/true.Alternative you might prefer
A
matchon string literals cannot be made case-insensitive, so the other way to write this isto keep all 24 match arms and lowercase the input once up front with
s.to_ascii_lowercase().That is a smaller textual diff, at the cost of one allocation on every
Name::from_str()call;I chose the table because it allocates nothing and
AGENTS.mdasks plumbing crates to avoidavoidable copies. Say the word and I will switch it over.
Tests
Three new tests in
gix-config-value/tests/value/color.rs, and all three fail without thesource change. Reverting only
gix-config-value/src/color.rsand keeping the tests:With the source restored:
test result: ok. 25 passed; 0 failed. No existing assertion wasmodified.
What I ran
justandcargo-nextestare not installed here, so these are the rawcargoinvocations,on
rustc 1.95.0(workspace MSRV is 1.85):cargo test -p gix-config-valuecargo test -p gix-config -p gixcargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warnings -A unknown-lints -A unfulfilled_lint_expectationscargo doc -p gix-config-value --no-depscargo test --workspace --no-fail-faston the branch: 181 suites, 3673 passed, 2 failed, 22ignored. The two failures are
gix-date'sparse::fuzz::artifact_inputs_can_be_parsed_without_panickingand
gix-imara-diff'spackaged_files_have_matching_provenance_and_modified_files_have_notices,both
Os { code: 2, kind: NotFound }on files that are present in the checkout; both pass whentheir crate is run on its own, and
cargo tree -e normal,build,devshows no path from eithercrate to
gix-config-value. The same run on unmodifiedmain@77dc1ff87failed 9 testsof the same shape, so this is noise from my machine rather than anything on either side of the
diff.
-A unfulfilled_lint_expectationsis needed because cleanmainreports three of thosein
gix-ref/src/lib.rson 1.95.0.One divergence I did not fix
Git also matches the
resetattribute case-insensitively — it is special-cased withstrncasecmpwhile every other attribute goes throughmemcmp— andgixstill rejects it:git --get-colorgix, before and after this PRRESETESC[mResetESC[mred RESETESC[;31mThose are the 3 remaining mismatches out of 67. I left them out because they live in
Attribute::from_str()rather than in the name parser and would look like an arbitraryexception among the case-sensitive attributes. Happy to fold a one-line fix for it into this
PR, or send it separately — whichever you prefer.