Repository navigation
fix(color): return a stable name for colors with aliases - #1218
ChandraChaan wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
|
I think we should switch to
|
gdamore
left a comment
There was a problem hiding this comment.
Seems good. I wonder if we should not just build this as a map for the reverse lookup -- although the reality is that I would not expect this interface to be heavily used.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1218 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 49 49
Lines 6145 6148 +3
=======================================
+ Hits 5656 5659 +3
Misses 472 472
Partials 17 17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for the review. A reverse map would make |
|
Hi @gdamore, a gentle ping on this one: it's approved and all checks pass. Is it good to merge with the scan as is, or would you like me to switch |
Color.Name()(and soString()) loops over theNamesmap and returns the first match. Go randomises map iteration, and seven colors have two names (gray/grey, darkgray/darkgrey, darkslategray/darkslategrey, dimgray/dimgrey, lightgray/lightgrey, lightslategray/lightslategrey, slategray/slategrey), so the same color can print as "gray" or "grey" from one call or run to the next.This picks the alphabetically first matching name, so those colors always come back with the "gray" spelling.
I added
TestColorNameAliases, which checksName()andString()for all seven colors 200 times each.Verified in CI on my fork, since I did not run Go locally:
color_test.go:255: iteration 0: Name() = "grey", want "gray"andFAIL github.com/gdamore/tcell/v3/color. Runs: https://github.com/ChandraChaan/tcell/actions/runs/41581599873 (linux), https://github.com/ChandraChaan/tcell/actions/runs/41581599845 (macos), https://github.com/ChandraChaan/tcell/actions/runs/41581599824 (windows).Summary by CodeRabbit