diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..e7fb995 --- /dev/null +++ b/PR.md @@ -0,0 +1,73 @@ +# fix(strutil): center-aligned Resize/Align overflows the requested length + +## Problem + +`strutil.Resize` / `strutil.Align` (and their utf8 variants) with +`comdef.Center` alignment produce strings longer than the requested length for +almost every non-empty input: + +```go +strutil.Resize("ab", 6, comdef.Center) // " ab " (len 7), want " ab " (len 6) +strutil.Resize("ab", 4, comdef.Center) // " ab " (len 5), want " ab " (len 4) +strutil.Resize("abc", 7, comdef.Center) // " abc "(len 8), want " abc "(len 7) +``` + +The extra space that centers an odd amount of padding is added unconditionally +whenever the input is non-empty, not only when the padding is actually odd. + +## Root cause + +`strutil/padding.go`, `resize`: + +```go +if align == comdef.Center { + padLn := (wantLen - sLen) / 2 + if diff = wantLen - padLn*2; diff > 0 { // wrong: missing "- sLen" + s += " " + } + ... +} +``` + +The condition is meant to detect an odd amount of total padding +(`wantLen - sLen`). Simplifying the buggy expression: + +``` +wantLen - 2*padLn = wantLen - 2*floor((wantLen-sLen)/2) + = sLen + ((wantLen - sLen) mod 2) +``` + +so `diff > 0` is true whenever `sLen > 0`, regardless of the padding parity. +The result is one space too long for every non-empty string with even padding. + +The existing `TestResize("title", 20, PosMiddle)` case did not catch this +because its padding (15) is odd, where the extra space is legitimately needed. + +## Fix + +Compare against the actual leftover padding by subtracting `sLen`: + +```go +if diff = wantLen - sLen - padLn*2; diff > 0 { + s += " " +} +``` + +This adds the extra space only when `(wantLen - sLen)` is odd, keeping the odd +case (e.g. the "title" test) unchanged. + +## Test + +Added `TestResize_center_length` in `strutil/padding_test.go` asserting both the +exact result and `len(result) == wantLen` for even- and odd-padding cases. + +- Gate: `go test ./strutil/ -run TestResize` -> ok (PASS) +- `go vet ./strutil/` -> exit 0 +- `go build ./...` -> exit 0 + +Counterfactual: reverting the change makes the new test fail with +`actual: int(7)` / `actual: " ab "` vs the expected length 6 / `" ab "`. + +Note: `TestDateSNv3/dateLen4_base62` in `strutil/gensn_test.go` is flaky and +already fails on the unmodified base commit (it asserts uniqueness of randomly +generated serial numbers); it is unrelated to this change. diff --git a/strutil/padding.go b/strutil/padding.go index cfb01d1..e18b23b 100644 --- a/strutil/padding.go +++ b/strutil/padding.go @@ -164,7 +164,9 @@ func resize(s string, sLen, wantLen int, align comdef.Align, cutOverflow bool) s if align == comdef.Center { padLn := (wantLen - sLen) / 2 - if diff = wantLen - padLn*2; diff > 0 { + // add a single extra space only when the total padding is odd, + // otherwise the result overflows the requested length. + if diff = wantLen - sLen - padLn*2; diff > 0 { s += " " } padStr := string(RepeatBytes(' ', padLn)) diff --git a/strutil/padding_test.go b/strutil/padding_test.go index 3d2351d..fc028b9 100644 --- a/strutil/padding_test.go +++ b/strutil/padding_test.go @@ -38,6 +38,29 @@ func TestPadding(t *testing.T) { assert.Eq(t, "hi,你好000", strutil.Utf8Padding(s, "0", 10, strutil.PosRight)) } +func TestResize_center_length(t *testing.T) { + // Center alignment must always produce a string of exactly the requested + // length; the extra padding space is only needed when the total padding + // (wantLen - len(s)) is odd. + tests := []struct { + give string + len int + want string + }{ + {"ab", 6, " ab "}, // even padding (4): 2 + 2 + {"ab", 4, " ab "}, // even padding (2): 1 + 1 + {"ab", 5, " ab "}, // odd padding (3): 1 + 2 + {"abc", 7, " abc "}, // even padding (4): 2 + 2 + {"a", 4, " a "}, // odd padding (3): 1 + 2 + } + + for _, tt := range tests { + got := strutil.Resize(tt.give, tt.len, comdef.Center) + assert.Eq(t, tt.len, len(got), "Resize(%q,%d,Center)=%q", tt.give, tt.len, got) + assert.Eq(t, tt.want, got) + } +} + func TestResize(t *testing.T) { tests := []struct { want, give string