fix: washed-out yellow on light-background terminals#1740
Open
joshazmy wants to merge 1 commit into
Open
Conversation
FullTerminalAnsiOutputCodes.YELLOW used plain SGR 33 ("dark yellow"),
which renders as a washed-out, barely-visible color on light/white
terminal backgrounds. This color was already fixed once, to the
brighter 256-color gold1 (\033[00;38;5;220m), in 0b43361 ("Improved
visibility on white terminal backgrounds"), but got silently
overwritten back to plain SGR 33 by 21f9b8a, which was actually only
trying to match a *different* yellow use (the hook-ignored-hook hint)
to git's own advice color.
Restore the 256-color gold1 for full-fledged terminals. 8-color
terminals (BasicTerminalAnsiOutputCodes) can't render the 256-color
escape, so they keep plain SGR 33 explicitly rather than inheriting
the 256-color value.
Fixes VirtusLab#380
PawelLipski
reviewed
Jul 6, 2026
| UNDERLINE = '\033[4m' | ||
| GREEN = '\033[32m' | ||
| YELLOW = '\033[33m' | ||
| # Plain SGR 33 ("dark yellow") renders as a washed-out, barely-visible color on light/white-background |
Collaborator
There was a problem hiding this comment.
The "SGR" acronym isn't well-known, pls expand (at least in one place)
PawelLipski
reviewed
Jul 6, 2026
Comment on lines
+91
to
+93
| # backgrounds; it was already used for exactly this reason back in 0b43361 ("Improved visibility on white | ||
| # terminal backgrounds"), but got silently overwritten by 21f9b8a while that commit was actually only | ||
| # trying to match a *different* yellow use (the hook-ignored-hook hint below) to git's own advice color. |
Collaborator
There was a problem hiding this comment.
Pls do not describe the historical state in the comments, just the current one (to reduce confusion/reduce mental load)
Collaborator
|
Thanks for the contribution - could you actually attach the screens of before and after this change on your terminal? |
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
2 participants
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.
Summary
FullTerminalAnsiOutputCodes.YELLOWused plain SGR 33 ("dark yellow"), which renders as a washed-out, barely-visible color on light/white terminal backgrounds. This color was already fixed once, to the brighter 256-color gold1 (\033[00;38;5;220m), in 0b43361 ("Improved visibility on white terminal backgrounds"), but got silently overwritten back to plain SGR 33 by 21f9b8a, which was actually only trying to match a different yellow use (the hook-ignored-hook hint) to git's own advice color.This restores the 256-color gold1 for full-fledged terminals. 8-color terminals (
BasicTerminalAnsiOutputCodes) can't render the 256-color escape, so they keep plain SGR 33 explicitly rather than inheriting the 256-color value.Fixes #380
Test plan
test_yellow_is_not_the_washed_out_dark_yellowregression test intests/test_utils.pytests/test_utils.py— 8 passed, 2 skipped (platform-specific, unrelated)test_status.py,test_discover.py,test_advance.py,test_fork_point.py,test_go_interactive.py,test_traverse.py) — all pass except 18 pre-existing failures unrelated to this change (verified: identical failures reproduce against unmodifieddevelop, caused byshutil.copytreefailing on a.git/fsmonitor--daemon.ipcunix socket in this sandbox, not by this fix)