fix: pad short rows in SetRows to the number of columns
SetRows stored rows with fewer cells than the header unchanged, so short rows rendered fmt's %!s(MISSING) verb and an empty row combined with a first-column formatter panicked on vals[0]. Pad short rows to the column count, mirroring AddRow's documented behavior of rendering missing cells as empty.
This commit is contained in:
1 parent
c86829d169
commit
dc9063405a
3 files changed
+119
-2
No files matched your search
@@ -0,0 +1,83 @@
|
|||||||
|
# fix: pad short rows in SetRows to the number of columns
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`SetRows` does not pad rows that have fewer cells than the table has columns.
|
||||||
|
As a result, short rows render fmt's missing-argument verb, and an empty row
|
||||||
|
combined with a first-column formatter panics:
|
||||||
|
|
||||||
|
```go
|
||||||
|
tbl := table.New("A", "B", "C").WithPadding(1)
|
||||||
|
tbl.SetRows([][]string{{"x", "y"}})
|
||||||
|
tbl.Print()
|
||||||
|
// A B C
|
||||||
|
// x y %!s(MISSING)
|
||||||
|
```
|
||||||
|
|
||||||
|
```go
|
||||||
|
tbl := table.New("A", "B").
|
||||||
|
WithFirstColumnFormatter(func(f string, v ...interface{}) string { return fmt.Sprintf(f, v...) })
|
||||||
|
tbl.SetRows([][]string{{}})
|
||||||
|
tbl.Print() // panic: runtime error: index out of range [0] with length 0
|
||||||
|
```
|
||||||
|
|
||||||
|
`AddRow` already handles this: it allocates `make([]string, len(t.header))` per
|
||||||
|
row, so short input renders the remaining columns empty. The documented
|
||||||
|
behaviour ("Rows can have less cells than the total number of columns;
|
||||||
|
subsequent cells will be rendered empty.") is not honored by `SetRows`.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
`table.go`, `SetRows`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
for _, row := range rows {
|
||||||
|
if len(row) > headerLength {
|
||||||
|
t.rows = append(t.rows, row[:headerLength])
|
||||||
|
} else {
|
||||||
|
t.rows = append(t.rows, row) // short rows stored as-is
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Short rows are stored unchanged. Later, `printRow` builds one value per cell of
|
||||||
|
the row and passes them to `fmt.Fprintf` with a format string containing
|
||||||
|
`len(t.header)` verbs, so missing cells become `%!s(MISSING)`. With a
|
||||||
|
first-column formatter, `printRow` also indexes `vals[0]`, which panics for an
|
||||||
|
empty row.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Pad short rows to the number of columns, mirroring `AddRow`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
switch {
|
||||||
|
case len(row) > headerLength:
|
||||||
|
t.rows = append(t.rows, row[:headerLength])
|
||||||
|
case len(row) < headerLength:
|
||||||
|
// pad short rows so every column is rendered, matching AddRow
|
||||||
|
padded := make([]string, headerLength)
|
||||||
|
copy(padded, row)
|
||||||
|
t.rows = append(t.rows, padded)
|
||||||
|
default:
|
||||||
|
t.rows = append(t.rows, row)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added `TestSetRowsShortRowsPadded` (asserts no `MISSING` verb and the exact
|
||||||
|
padded output) and `TestSetRowsEmptyRowWithFirstColumnFormatter` (asserts no
|
||||||
|
panic).
|
||||||
|
|
||||||
|
- Gate (`GOTOOLCHAIN=go1.25.10`): `go test ./...` -> ok (PASS, exit 0)
|
||||||
|
- `go vet ./...` -> exit 0
|
||||||
|
- `go build ./...` -> exit 0
|
||||||
|
|
||||||
|
Counterfactual: reverting `SetRows` makes `TestSetRowsShortRowsPadded` fail with
|
||||||
|
`output contains missing-arg verb: "A B C \nx y %!s(MISSING)\n"`.
|
||||||
|
|
||||||
|
Note: `gofmt -l table.go` reports the file, but this is pre-existing (the
|
||||||
|
`Print()` block near line 258 is space-indented on the base commit) and
|
||||||
|
unrelated to this change; the modified region is gofmt-clean, so unrelated lines
|
||||||
|
were intentionally left untouched.
|
||||||
@@ -238,9 +238,15 @@ func (t *table) SetRows(rows [][]string) Table {
|
|||||||
headerLength := len(t.header)
|
headerLength := len(t.header)
|
||||||
|
|
||||||
for _, row := range rows {
|
for _, row := range rows {
|
||||||
if len(row) > headerLength {
|
switch {
|
||||||
|
case len(row) > headerLength:
|
||||||
t.rows = append(t.rows, row[:headerLength])
|
t.rows = append(t.rows, row[:headerLength])
|
||||||
} else {
|
case len(row) < headerLength:
|
||||||
|
// pad short rows so every column is rendered, matching AddRow
|
||||||
|
padded := make([]string, headerLength)
|
||||||
|
copy(padded, row)
|
||||||
|
t.rows = append(t.rows, padded)
|
||||||
|
default:
|
||||||
t.rows = append(t.rows, row)
|
t.rows = append(t.rows, row)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -309,3 +309,31 @@ func TestTable_WithWidthFunc(t *testing.T) {
|
|||||||
assert.Contains(t, actual, "请求 alpha")
|
assert.Contains(t, actual, "请求 alpha")
|
||||||
assert.Contains(t, actual, "abc beta")
|
assert.Contains(t, actual, "abc beta")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSetRowsShortRowsPadded(t *testing.T) {
|
||||||
|
// SetRows must render every column for short rows (like AddRow does),
|
||||||
|
// instead of emitting fmt's %!s(MISSING) verb.
|
||||||
|
buf := &bytes.Buffer{}
|
||||||
|
tbl := New("A", "B", "C").WithWriter(buf).WithPadding(1)
|
||||||
|
tbl.SetRows([][]string{{"x", "y"}})
|
||||||
|
tbl.Print()
|
||||||
|
|
||||||
|
out := buf.String()
|
||||||
|
if strings.Contains(out, "MISSING") {
|
||||||
|
t.Fatalf("output contains missing-arg verb: %q", out)
|
||||||
|
}
|
||||||
|
assert.Equal(t, "A B C \nx y \n", out)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSetRowsEmptyRowWithFirstColumnFormatter(t *testing.T) {
|
||||||
|
// An empty row must not panic when a first-column formatter is set.
|
||||||
|
buf := &bytes.Buffer{}
|
||||||
|
tbl := New("A", "B").WithWriter(buf).WithPadding(1).
|
||||||
|
WithFirstColumnFormatter(func(f string, v ...interface{}) string {
|
||||||
|
return fmt.Sprintf(f, v...)
|
||||||
|
})
|
||||||
|
assert.NotPanics(t, func() {
|
||||||
|
tbl.SetRows([][]string{{}})
|
||||||
|
tbl.Print()
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in new issue
Block a user