fix(convert): zero-pad each channel in RgbToHex
RgbToHex emitted a single hex digit for channel values below 0x10 because strconv.FormatInt produces the minimal representation. This broke the fixed-width hex format, the round-trip with HexToRgb, and the hex2basicMap fast path in Rgb2basic. Format each channel with %02x, matching RgbTo256.
This commit is contained in:
1 parent
9df61ac4fe
commit
f78554106d
3 files changed
+84
-1
No files matched your search
@@ -0,0 +1,71 @@
|
|||||||
|
# fix(convert): zero-pad each channel in RgbToHex
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`RgbToHex` (and its alias `Rgb2hex`) does not zero-pad channel values below
|
||||||
|
`0x10`, so a single hex digit is emitted for those channels:
|
||||||
|
|
||||||
|
```go
|
||||||
|
color.RgbToHex([]int{1, 2, 3}) // "123" , want "010203"
|
||||||
|
color.RgbToHex([]int{0, 0, 0}) // "000" , want "000000"
|
||||||
|
color.RgbToHex([]int{10, 11, 12}) // "0a0b0c" -> actually "abc"
|
||||||
|
```
|
||||||
|
|
||||||
|
This breaks two contracts:
|
||||||
|
|
||||||
|
1. The round-trip with `HexToRgb` is no longer an identity for colors with any
|
||||||
|
channel below 16:
|
||||||
|
|
||||||
|
```go
|
||||||
|
color.RgbToHex(color.HexToRgb("010203")) // "123", not "010203"
|
||||||
|
```
|
||||||
|
|
||||||
|
2. `Rgb2basic` looks a color up in `hex2basicMap`, whose keys are all
|
||||||
|
fixed-width 6-digit hex strings (e.g. "000000"). Because `RgbToHex(0,0,0)`
|
||||||
|
returns "000", pure black never matches the fast-path map and silently falls
|
||||||
|
through to the approximate `RgbToAnsi`.
|
||||||
|
|
||||||
|
The sibling converter `RgbTo256` already builds its hex key with
|
||||||
|
`fmt.Sprintf("%02x%02x%02x", ...)`, so this is an inconsistency within the same
|
||||||
|
file.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`convert.go`, `RgbToHex`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
for _, v := range rgb {
|
||||||
|
hexNodes = append(hexNodes, strconv.FormatInt(int64(v), 16))
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`strconv.FormatInt(_, 16)` produces the minimal representation, i.e. one digit
|
||||||
|
for values 0-15.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Format each channel as a fixed two-digit hex value:
|
||||||
|
|
||||||
|
```go
|
||||||
|
hexNodes = append(hexNodes, fmt.Sprintf("%02x", v))
|
||||||
|
```
|
||||||
|
|
||||||
|
`fmt` is already imported in the file. Existing values >= 16 are unchanged.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Extended `TestRgbToHex` in `utils_test.go` with small-channel cases
|
||||||
|
(`{0,0,0}`, `{1,2,3}`, `{10,11,12}`) and an explicit
|
||||||
|
`RgbToHex(HexToRgb(hex)) == hex` round-trip check.
|
||||||
|
|
||||||
|
- Gate: `go test ./` -> ok (PASS)
|
||||||
|
- `go vet ./...` -> exit 0
|
||||||
|
- `go build ./...` -> exit 0
|
||||||
|
|
||||||
|
Counterfactual: reverting `RgbToHex` to `strconv.FormatInt(int64(v), 16)` makes
|
||||||
|
the new test fail with `expect: "abc"` vs `actual: "0a0b0c"`.
|
||||||
|
|
||||||
|
Note: `gofmt -l convert.go` reports the file, but this is pre-existing (a doc
|
||||||
|
comment near line 866, unrelated to this change and present on the base commit).
|
||||||
|
The changed region is gofmt-clean, so unrelated lines were intentionally left
|
||||||
|
untouched.
|
||||||
+4
-1
@@ -483,7 +483,10 @@ func RgbToHex(rgb []int) string {
|
|||||||
hexNodes := make([]string, 0, len(rgb))
|
hexNodes := make([]string, 0, len(rgb))
|
||||||
|
|
||||||
for _, v := range rgb {
|
for _, v := range rgb {
|
||||||
hexNodes = append(hexNodes, strconv.FormatInt(int64(v), 16))
|
// each channel must be two hex digits, otherwise channels below 0x10
|
||||||
|
// are emitted as a single digit and break both the fixed-width hex
|
||||||
|
// format and the round-trip with HexToRgb.
|
||||||
|
hexNodes = append(hexNodes, fmt.Sprintf("%02x", v))
|
||||||
}
|
}
|
||||||
return strings.Join(hexNodes, "")
|
return strings.Join(hexNodes, "")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -119,12 +119,21 @@ func TestRgbToHex(t *testing.T) {
|
|||||||
{"cccccc", []int{204, 204, 204}},
|
{"cccccc", []int{204, 204, 204}},
|
||||||
{"aabbcc", []int{170, 187, 204}},
|
{"aabbcc", []int{170, 187, 204}},
|
||||||
{"aa99cd", []int{170, 153, 205}},
|
{"aa99cd", []int{170, 153, 205}},
|
||||||
|
// channels below 0x10 must be zero-padded to two hex digits
|
||||||
|
{"000000", []int{0, 0, 0}},
|
||||||
|
{"010203", []int{1, 2, 3}},
|
||||||
|
{"0a0b0c", []int{10, 11, 12}},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, item := range tests {
|
for _, item := range tests {
|
||||||
assert.Equal(t, RgbToHex(item.given), item.want)
|
assert.Equal(t, RgbToHex(item.given), item.want)
|
||||||
assert.Equal(t, Rgb2hex(item.given), item.want)
|
assert.Equal(t, Rgb2hex(item.given), item.want)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RgbToHex must be the inverse of HexToRgb, including for small channels.
|
||||||
|
for _, hex := range []string{"010203", "000000", "0a0b0c", "aabbcc"} {
|
||||||
|
assert.Equal(t, RgbToHex(HexToRgb(hex)), hex)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRgbToAnsi(t *testing.T) {
|
func TestRgbToAnsi(t *testing.T) {
|
||||||
|
|||||||
Reference in new issue
Block a user