Skip to content

Commit ffba8a2

Browse files
author
David Noble
committed
feat(result): one delimited formatter with three presets, and one table (#740, #741)
make check passes. DelimitedFormatter carries three attributes -- Separator, SuppressHeadings, Raw -- and ships three presets, which is gcloud's shape: its csv and value are one renderer differing in separator, heading, and quoting, with value documented as "CSV with no heading and <TAB> separator instead of <COMMA>". csv , heading RFC 4180 quoting spreadsheets tsv tab no heading RFC 4180 quoting cut -f2, awk value tab no heading raw text --jq already built Quoting is what separates the last from the middle. A machine-parseable format must quote a field containing its own delimiter or the row loses a boundary; a raw renderer must not, or text a caller composed comes back wearing quotes it did not write. The formatter now accepts every shape rather than only rows-with-fields. A non-sequence is one row -- `--jq '.count'` produces exactly that, and refusing it made the filter stage incomplete. A slice of scalars is one column with no header, since a scalar has no field to name. A byte slice is one value rather than a sequence of numbers. Two tests asserted refusals that are now supported shapes, and are rewritten as Accepts with the reason recorded rather than deleted. That folding fixed tsv as a side effect: `-o tsv` on a scalar said "expected slice or array, got int64" before, because value was a separate implementation rather than a preset of the same one. The redundancy was in the implementation, not the name. It also simplified what it touched. With scalars no longer an error case, csvHeadersFromElements and csvRowFromElement always returned nil errors; lint caught it, and both lost their error returns along with the handling at three call sites. TableFormatter shares that column inference -- HasHeaders, csv:"name" overrides, csv:"-" skips, the map-key union -- so one value renders the same columns as table, csv, or tsv, and only the presentation differs. A test pins it, because divergent column logic is how one rendering ends up showing a field the other hides. Alignment is text/tabwriter's, which measures in RUNES. That is half of #741 fixed at the shared formatter rather than at each call site: a hand-rolled %-30s counts bytes, so a row with a multi-byte character is short by the difference and every column after it shifts. The test uses a 7-rune, 21-byte name and asserts the next column starts at the same rune offset on both rows. The set is csv, json, none, table, template=BODY, tsv, value, yaml. Files renamed: csv.go and its tests hold three presets now, not one format.
1 parent c946d7a commit ffba8a2

8 files changed

Lines changed: 378 additions & 204 deletions

File tree

pkg/result/csv.go renamed to pkg/result/delimited.go

Lines changed: 107 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -9,55 +9,78 @@ import (
99
"io"
1010
"reflect"
1111
"sort"
12+
"strings"
1213
)
1314

14-
// CSVFormatter renders a slice of rows as delimiter-separated values.
15+
// DelimitedFormatter renders values as delimiter-separated lines.
1516
//
16-
// One formatter serves two jobs that disagree about two things at once, so each gets a preset rather than a
17-
// flag. A spreadsheet wants a comma and a header row; `awk '{print $2}'` wants a tab and no header row to
18-
// skip. gcloud reached the same shape -- its `value` format is documented as "CSV with no heading and <TAB>
19-
// separator instead of <COMMA>" -- and exposes the separator as an attribute of one implementation.
17+
// One formatter, three attributes, three presets. gcloud reached the same shape: its `csv` and `value` are
18+
// one renderer differing in separator, heading, and quoting, with `value` documented as "CSV with no heading
19+
// and <TAB> separator instead of <COMMA>".
2020
//
21-
// - [NewCSVFormatter] -- comma, with headings. The spreadsheet preset.
22-
// - [NewTSVFormatter] -- tab, no headings. The pipeline preset.
21+
// - [NewCSVFormatter] -- comma, heading, quoted. RFC 4180, for a spreadsheet.
22+
// - [NewTSVFormatter] -- tab, no heading, quoted. For `cut -f2` and `awk '{print $2}'`.
23+
// - [NewValueFormatter] -- tab, no heading, raw. For text `--jq` already built.
2324
//
24-
// The shape of value drives header inference:
25+
// Quoting is what separates the last from the middle. A machine-parseable format must quote a field
26+
// containing its own delimiter or the row loses a boundary; a raw renderer must not, or the text a caller
27+
// composed comes back wearing quotes it did not write.
28+
//
29+
// Shape drives column inference:
2530
//
2631
// - If value implements [HasHeaders], its Headers() result is the column order.
2732
// - Otherwise, if value is a slice/array of structs, the struct fields (in declaration order, with
2833
// `csv:"name"` overrides and `csv:"-"` skips) are the columns.
2934
// - Otherwise, if value is a slice/array of maps, the union of map keys (sorted alphabetically) is
3035
// the columns.
31-
// - Otherwise, [Format] returns an error.
36+
// - Otherwise the elements are scalars: one column, no header, since a scalar has no field to name.
37+
//
38+
// A value that is not a slice or array at all is one row. That is the shape `--jq '.count'` produces, and
39+
// erroring on it would make the filter stage incomplete.
3240
//
33-
// Cell values are rendered via fmt.Sprint, which honors fmt.Stringer for custom types and produces
34-
// reasonable defaults for numbers, bools, time.Time, and nil. Quoting is handled by encoding/csv per
35-
// RFC 4180. Empty input renders no bytes (no header row).
36-
type CSVFormatter struct {
41+
// Cell values render via fmt.Sprint, which honors fmt.Stringer and produces reasonable defaults for numbers,
42+
// bools, time.Time, and nil. Empty input renders no bytes.
43+
type DelimitedFormatter struct {
3744

38-
// Separator is the field delimiter. The zero value is a comma, so the zero CSVFormatter is RFC 4180.
45+
// Separator is the field delimiter. The zero value is a comma.
3946
Separator rune
4047

41-
// SuppressHeadings omits the header row. A spreadsheet wants the row; a shell pipeline does not, since
42-
// `awk` and `cut` would each have to skip it.
48+
// SuppressHeadings omits the header row. A spreadsheet wants it; a shell pipeline does not, since awk
49+
// and cut would each have to skip it.
4350
SuppressHeadings bool
51+
52+
// Raw disables RFC 4180 quoting. Set for a renderer whose job is to print what it was given.
53+
Raw bool
4454
}
4555

46-
// NewCSVFormatter returns the spreadsheet preset: comma-separated, with a header row.
56+
// NewCSVFormatter returns the spreadsheet preset: comma, heading, quoted.
4757
//
4858
// Returns:
49-
// - `CSVFormatter`: the RFC 4180 formatter.
50-
func NewCSVFormatter() CSVFormatter { return CSVFormatter{Separator: ','} }
59+
// - `DelimitedFormatter`: the RFC 4180 formatter.
60+
func NewCSVFormatter() DelimitedFormatter { return DelimitedFormatter{Separator: ','} }
5161

52-
// NewTSVFormatter returns the pipeline preset: tab-separated, with no header row.
62+
// NewTSVFormatter returns the pipeline preset: tab, no heading, quoted.
5363
//
54-
// Tabs need no quoting for the values a row normally carries, so a line survives `cut -f2` and
55-
// `awk '{print $2}'` without a parser. Suppressing the header is half of that: a header row is one more
56-
// line every consumer has to skip.
64+
// Tabs need no quoting for the values a row normally carries, so a line survives `cut -f2` without a parser.
65+
// Quoting stays on so a field that does contain a tab cannot silently split the row.
5766
//
5867
// Returns:
59-
// - `CSVFormatter`: the tab-separated formatter.
60-
func NewTSVFormatter() CSVFormatter { return CSVFormatter{Separator: '\t', SuppressHeadings: true} }
68+
// - `DelimitedFormatter`: the tab-separated formatter.
69+
func NewTSVFormatter() DelimitedFormatter {
70+
return DelimitedFormatter{Separator: '\t', SuppressHeadings: true}
71+
}
72+
73+
// NewValueFormatter returns the raw preset: tab, no heading, no quoting.
74+
//
75+
// This is what completes the filter stage. `--jq` can build any line; every other format then imposes a
76+
// syntax on it -- json quotes and escapes, yaml applies scalar rules, csv adds a header. This one prints it.
77+
// `aws` ships the same rendering as `text` and `gcloud` as `value`.
78+
//
79+
// Returns:
80+
// - `DelimitedFormatter`: the raw formatter.
81+
func NewValueFormatter() DelimitedFormatter {
82+
return DelimitedFormatter{Separator: '\t', SuppressHeadings: true, Raw: true}
83+
}
6184

6285
// HasHeaders is the opt-in interface that overrides automatic header inference. Implementations are
6386
// typically named-slice types whose row shape doesn't lend itself to reflection (e.g., heterogeneous
@@ -69,30 +92,34 @@ type HasHeaders interface {
6992
}
7093

7194
// Compile-time interface guard.
72-
var _ Formatter = CSVFormatter{}
95+
var _ Formatter = DelimitedFormatter{}
7396

7497
// region Formatter
7598

7699
// Format renders value as delimiter-separated values to w.
77100
//
78101
// Returns an error if value is not a slice/array of structs or maps and does not implement
79102
// [HasHeaders].
80-
func (f CSVFormatter) Format(value any, w io.Writer) error {
103+
func (f DelimitedFormatter) Format(value any, w io.Writer) error {
81104

82105
if value == nil {
83106
return nil
84107
}
85108

86-
rv := reflect.ValueOf(value)
87-
if rv.Kind() == reflect.Pointer {
88-
if rv.IsNil() {
89-
return nil
90-
}
91-
rv = rv.Elem()
109+
rv := indirect(reflect.ValueOf(value))
110+
if !rv.IsValid() {
111+
return nil
92112
}
93113

114+
// A value that is not a sequence is one row. `--jq '.count'` produces this, and refusing it would make
115+
// the filter stage incomplete.
94116
if rv.Kind() != reflect.Slice && rv.Kind() != reflect.Array {
95-
return fmt.Errorf("result.CSVFormatter: expected slice or array, got %T", value)
117+
return f.emit(w, nil, [][]string{{csvCellValue(rv)}})
118+
}
119+
120+
// A byte slice is one value, not a sequence of numbers.
121+
if rv.Type().Elem().Kind() == reflect.Uint8 {
122+
return f.emit(w, nil, [][]string{{string(rv.Bytes())}})
96123
}
97124

98125
if rv.Len() == 0 {
@@ -101,29 +128,52 @@ func (f CSVFormatter) Format(value any, w io.Writer) error {
101128

102129
headers, headersFromValue := csvHeadersFromValue(value)
103130
if !headersFromValue {
104-
var err error
105-
headers, err = csvHeadersFromElements(rv)
106-
if err != nil {
107-
return err
131+
headers = csvHeadersFromElements(rv)
132+
}
133+
134+
rows := make([][]string, 0, rv.Len())
135+
for i := range rv.Len() {
136+
rows = append(rows, csvRowFromElement(rv.Index(i), headers))
137+
}
138+
139+
return f.emit(w, headers, rows)
140+
}
141+
142+
// emit writes the header and rows, quoted or raw.
143+
//
144+
// Parameters:
145+
// - `w`: the destination.
146+
// - `headers`: the column names, or nil when the rows are scalars.
147+
// - `rows`: the rendered cells.
148+
//
149+
// Returns:
150+
// - `error`: any write error.
151+
func (f DelimitedFormatter) emit(w io.Writer, headers []string, rows [][]string) error {
152+
153+
separator := f.Separator
154+
if separator == 0 {
155+
separator = ','
156+
}
157+
158+
if f.Raw {
159+
for _, row := range rows {
160+
if _, err := fmt.Fprintln(w, strings.Join(row, string(separator))); err != nil {
161+
return err
162+
}
108163
}
164+
return nil
109165
}
110166

111167
writer := csv.NewWriter(w)
112-
if f.Separator != 0 {
113-
writer.Comma = f.Separator
114-
}
168+
writer.Comma = separator
115169

116-
if !f.SuppressHeadings {
170+
if !f.SuppressHeadings && len(headers) > 0 {
117171
if err := writer.Write(headers); err != nil {
118172
return err
119173
}
120174
}
121175

122-
for i := range rv.Len() {
123-
row, err := csvRowFromElement(rv.Index(i), headers)
124-
if err != nil {
125-
return err
126-
}
176+
for _, row := range rows {
127177
if err := writer.Write(row); err != nil {
128178
return err
129179
}
@@ -149,19 +199,20 @@ func csvHeadersFromValue(value any) ([]string, bool) {
149199

150200
// csvHeadersFromElements infers headers from the first element's shape. Slice-of-structs uses field
151201
// declaration order; slice-of-maps uses the union of keys across every element, sorted alphabetically.
152-
func csvHeadersFromElements(rv reflect.Value) ([]string, error) {
202+
func csvHeadersFromElements(rv reflect.Value) []string {
153203

154204
first := indirect(rv.Index(0))
155205
switch first.Kind() {
156206

157207
case reflect.Struct:
158-
return csvHeadersFromStruct(first.Type()), nil
208+
return csvHeadersFromStruct(first.Type())
159209

160210
case reflect.Map:
161-
return csvHeadersFromMaps(rv), nil
211+
return csvHeadersFromMaps(rv)
162212

163213
default:
164-
return nil, fmt.Errorf("result.CSVFormatter: element kind %s is not struct or map", first.Kind())
214+
// Scalars have no field to name, so there is no header row.
215+
return nil
165216
}
166217
}
167218

@@ -229,19 +280,20 @@ func csvHeadersFromMaps(rv reflect.Value) []string {
229280

230281
// csvRowFromElement renders a single element (struct or map) as a row in headers order. Missing
231282
// fields/keys render as "" — the empty string. Anything that is not struct or map errors loudly.
232-
func csvRowFromElement(rv reflect.Value, headers []string) ([]string, error) {
283+
func csvRowFromElement(rv reflect.Value, headers []string) []string {
233284

234285
rv = indirect(rv)
235286
switch rv.Kind() {
236287

237288
case reflect.Struct:
238-
return csvRowFromStruct(rv, headers), nil
289+
return csvRowFromStruct(rv, headers)
239290

240291
case reflect.Map:
241-
return csvRowFromMap(rv, headers), nil
292+
return csvRowFromMap(rv, headers)
242293

243294
default:
244-
return nil, fmt.Errorf("result.CSVFormatter: element kind %s is not struct or map", rv.Kind())
295+
// A scalar element is a single-field row.
296+
return []string{csvCellValue(rv)}
245297
}
246298
}
247299

pkg/result/csv_preset_test.go renamed to pkg/result/delimited_preset_test.go

File renamed without changes.

pkg/result/csv_test.go renamed to pkg/result/delimited_test.go

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestCSVFormatterQuotesNewlines(t *testing.T) {
175175
func TestCSVFormatterEmptySliceProducesNoOutput(t *testing.T) {
176176

177177
var buf bytes.Buffer
178-
if err := (CSVFormatter{}).Format([]map[string]any{}, &buf); err != nil {
178+
if err := (DelimitedFormatter{}).Format([]map[string]any{}, &buf); err != nil {
179179
t.Fatalf("Format: %v", err)
180180
}
181181
if buf.Len() != 0 {
@@ -186,35 +186,47 @@ func TestCSVFormatterEmptySliceProducesNoOutput(t *testing.T) {
186186
func TestCSVFormatterNilProducesNoOutput(t *testing.T) {
187187

188188
var buf bytes.Buffer
189-
if err := (CSVFormatter{}).Format(nil, &buf); err != nil {
189+
if err := (DelimitedFormatter{}).Format(nil, &buf); err != nil {
190190
t.Fatalf("Format: %v", err)
191191
}
192192
if buf.Len() != 0 {
193193
t.Errorf("nil should render zero bytes; got %q", buf.String())
194194
}
195195
}
196196

197-
func TestCSVFormatterRejectsScalar(t *testing.T) {
197+
// TestDelimitedFormatterAcceptsScalar covers the shape `--jq '.count'` produces.
198+
//
199+
// This asserted a refusal until the formatter took on the raw and pipeline presets. A value that is not a
200+
// sequence is one row; refusing it would make the filter stage incomplete, since a query that narrows to a
201+
// single number is the most ordinary thing a caller writes.
202+
func TestDelimitedFormatterAcceptsScalar(t *testing.T) {
198203

199204
var buf bytes.Buffer
200-
err := (CSVFormatter{}).Format(42, &buf)
201-
if err == nil {
202-
t.Fatal("expected error for scalar input; got nil")
205+
if err := (DelimitedFormatter{}).Format(42, &buf); err != nil {
206+
t.Fatalf("Format(42): %v", err)
203207
}
204-
if !strings.Contains(err.Error(), "expected slice or array") {
205-
t.Errorf("error text = %q, want substring 'expected slice or array'", err.Error())
208+
if strings.TrimSpace(buf.String()) != "42" {
209+
t.Errorf("rendered %q, want %q", buf.String(), "42")
206210
}
207211
}
208212

209-
func TestCSVFormatterRejectsSliceOfScalars(t *testing.T) {
213+
// TestDelimitedFormatterAcceptsSliceOfScalars covers a one-column result: no field names, so no header.
214+
//
215+
// This asserted a refusal until the raw preset arrived. `--jq '.entries[] | .target'` yields exactly this
216+
// shape, and it is the case `value` exists to print.
217+
func TestDelimitedFormatterAcceptsSliceOfScalars(t *testing.T) {
210218

211219
var buf bytes.Buffer
212-
err := (CSVFormatter{}).Format([]int{1, 2, 3}, &buf)
213-
if err == nil {
214-
t.Fatal("expected error for slice of scalars; got nil")
220+
if err := (DelimitedFormatter{}).Format([]int{1, 2, 3}, &buf); err != nil {
221+
t.Fatalf("Format([]int): %v", err)
215222
}
216-
if !strings.Contains(err.Error(), "not struct or map") {
217-
t.Errorf("error text = %q, want substring 'not struct or map'", err.Error())
223+
224+
lines := strings.Split(strings.TrimSpace(buf.String()), "\n")
225+
if len(lines) != 3 {
226+
t.Fatalf("lines = %d, want 3 (no header for scalars): %q", len(lines), buf.String())
227+
}
228+
if lines[0] != "1" {
229+
t.Errorf("first line = %q, want %q", lines[0], "1")
218230
}
219231
}
220232

@@ -243,7 +255,7 @@ func TestCSVFormatterRoundTripsThroughCsvReader(t *testing.T) {
243255
}
244256

245257
var buf bytes.Buffer
246-
if err := (CSVFormatter{}).Format(in, &buf); err != nil {
258+
if err := (DelimitedFormatter{}).Format(in, &buf); err != nil {
247259
t.Fatalf("Format: %v", err)
248260
}
249261

@@ -306,7 +318,7 @@ func mustFormatCSV(t *testing.T, value any) string {
306318

307319
t.Helper()
308320
var buf bytes.Buffer
309-
if err := (CSVFormatter{}).Format(value, &buf); err != nil {
321+
if err := (DelimitedFormatter{}).Format(value, &buf); err != nil {
310322
t.Fatalf("Format: %v", err)
311323
}
312324
return buf.String()

‎pkg/result/selection.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
// this makes the conflict impossible by construction. `kubectl` ships the same form -- `-o go-template=`,
1717
// `-o jsonpath=`, `-o custom-columns=`.
1818
//
19-
// The names are "csv", "json", "none", "template=BODY", "tsv", "value", and "yaml". Reshaping a value is the
19+
// The names are "csv", "json", "none", "table", "template=BODY", "tsv", "value", and "yaml". Reshaping a value is the
2020
// filter stage's job -- see [FilterByExprs] -- so only "template" takes an argument.
2121
//
2222
// Parameters:
@@ -55,6 +55,9 @@ func FormatterByName(spec string) (Formatter, error) {
5555
case "none":
5656
return NoneFormatter{}, nil
5757

58+
case "table":
59+
return NewTableFormatter(), nil
60+
5861
case "value":
5962
return NewValueFormatter(), nil
6063

@@ -66,8 +69,8 @@ func FormatterByName(spec string) (Formatter, error) {
6669

6770
default:
6871
return nil, fmt.Errorf(
69-
"result.FormatterByName: unknown formatter %q; expected one of csv, json, none, template=BODY, "+
70-
"tsv, value, yaml", name)
72+
"result.FormatterByName: unknown formatter %q; expected one of csv, json, none, table, "+
73+
"template=BODY, tsv, value, yaml", name)
7174
}
7275
}
7376

‎pkg/result/selection_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ func TestFormatterByNameKnownFormatters(t *testing.T) {
2020
{"json", JSONFormatter{}},
2121
{"JSON", JSONFormatter{}},
2222
{" yaml ", YAMLFormatter{}},
23-
{"csv", CSVFormatter{}},
24-
{"tsv", CSVFormatter{}},
23+
{"csv", DelimitedFormatter{}},
24+
{"tsv", DelimitedFormatter{}},
2525
{"none", NoneFormatter{}},
2626
} {
2727
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)