Upstream gookit/goutil state of the touched files before 71dcf88743
This commit is contained in:
1 parent
fd69f1aadd
commit
1ffc0f390b
3 files changed
-85
No files matched your search
@@ -1,73 +0,0 @@
|
||||
# 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.
|
||||
@@ -166,12 +166,6 @@ func Substr(s string, pos, length int) string {
|
||||
stopIdx = strLn + length
|
||||
}
|
||||
|
||||
// A negative length can move stopIdx before pos; clamp to avoid an
|
||||
// out-of-range slice.
|
||||
if stopIdx < pos {
|
||||
stopIdx = pos
|
||||
}
|
||||
|
||||
return string(runes[pos:stopIdx])
|
||||
}
|
||||
|
||||
|
||||
@@ -120,12 +120,6 @@ func TestSubstr(t *testing.T) {
|
||||
assert.Eq(t, "", strutil.Substr("abcDEF", 23, 5))
|
||||
assert.Eq(t, "cDEF12", strutil.Substr("abcDEF123", 2, -1))
|
||||
assert.Eq(t, "cDEF", strutil.Substr("abcDEF123", 2, -3))
|
||||
|
||||
// negative length trimming more than the remaining substring must not
|
||||
// panic; it should yield an empty string.
|
||||
assert.Eq(t, "", strutil.Substr("hello", 4, -3))
|
||||
assert.Eq(t, "", strutil.Substr("hello", 3, -5))
|
||||
assert.Eq(t, "", strutil.Substr("abcDEF123", 8, -3))
|
||||
}
|
||||
|
||||
func TestSplitInlineComment(t *testing.T) {
|
||||
|
||||
Reference in new issue
Block a user