" + strings.Repeat("report line ", 5000) + "
\n \n", + } + + out, _, err := execCommandSplit("member", "notify", + "--subject", "Daily report", "--html", "report
", "--dry-run") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + + var got [][]string + for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") { + field, value, _ := strings.Cut(line, " ") + got = append(got, []string{field, strings.TrimSpace(value)}) + } + want := [][]string{ + {"FIELD", "VALUE"}, + {"HTML", "report line report line report line report line report line ..."}, + {"RECIPIENTS[0].PERSON_ID", "5068740052131"}, + {"RECIPIENTS[0].STATUS", "accepted"}, + {"RECIPIENTS[1].PERSON_ID", "5068740052132"}, + {"RECIPIENTS[1].REASON", "no_email"}, + {"RECIPIENTS[1].STATUS", "skipped"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("dry-run table rows = %q\nwant %q\n---\n%s", got, want, out) + } +} + // TestPrintGenericResultCompleteEnvelopeUnmarked guards the marker's negative // case: a page that fits carries no truncated/emitted_rows keys — the marker // means "this page was reduced", not "this command supports reduction". diff --git a/internal/cli/generic_table.go b/internal/cli/generic_table.go index 6e33023..187b8ba 100644 --- a/internal/cli/generic_table.go +++ b/internal/cli/generic_table.go @@ -1,9 +1,12 @@ package cli import ( + "bytes" + "encoding/base64" "encoding/json" "fmt" "reflect" + "sort" "strconv" "strings" "time" @@ -45,7 +48,8 @@ type genKV struct { // - a paginated list envelope ({Items:[...], Total, ...}) or a top-level row // array prints as an aligned table (columns from displayColumns, else a // reflective heuristic); -// - a single object prints as a vertical key/value table; +// - a single object prints as a vertical key/value table of every value it +// carries, nested ones included; // - anything we can't model falls back to indented JSON, so output is never // empty. func renderGenericTable(ctx *RunContext, data any) error { @@ -59,7 +63,9 @@ func renderGenericTable(ctx *RunContext, data any) error { switch v.Kind() { case reflect.Slice: - return renderRowTable(ctx, v, v.Len()) + if isRowSlice(v.Type()) { + return renderRowTable(ctx, v, v.Len()) + } case reflect.Struct: if rows, total, ok := listEnvelope(v); ok { return renderRowTable(ctx, rows, total) @@ -68,9 +74,8 @@ func renderGenericTable(ctx *RunContext, data any) error { return err } return renderMcpPerUserOAuthNotice(ctx, v) - default: - return jsonFallback(ctx, data) } + return jsonFallback(ctx, data) } // listEnvelope reports whether struct v is a paginated list envelope: exactly @@ -207,23 +212,12 @@ func heuristicColumns(rowType reflect.Type) []output.Column { return cols } -// renderVertical prints a single object as a two-column FIELD/VALUE table, -// showing scalar fields with a non-empty value. Nested objects/arrays are -// omitted (json/toon carries the full shape for machines). +// renderVertical prints a single object as a two-column FIELD/VALUE table with +// one row per non-empty scalar value. A value nested in an object, map or array +// is named by its path — OWNER.EMAIL, LABELS.env, RECIPIENTS[0].STATUS — so +// nested fields are listed alongside the top-level ones instead of dropped. func renderVertical(ctx *RunContext, v reflect.Value) error { - t := v.Type() - rows := make([]genKV, 0, t.NumField()) - for i := 0; i < t.NumField(); i++ { - f := t.Field(i) - if f.PkgPath != "" || !isScalarType(f.Type) { - continue - } - s := scalarString(v.Field(i)) - if s == "" || s == "-" { - continue - } - rows = append(rows, genKV{Field: headerFromField(f), Value: s}) - } + rows := appendLeafRows(nil, "", v) if len(rows) == 0 { return jsonFallback(ctx, v.Interface()) } @@ -234,6 +228,50 @@ func renderVertical(ctx *RunContext, v reflect.Value) error { return ctx.Printer.Print(rows, cols) } +// appendLeafRows appends a row for every non-empty scalar (or timestamp) +// reachable from v, naming it by its path under prefix: struct fields by +// headerFromField, map entries by key (sorted), array elements by index. +func appendLeafRows(rows []genKV, prefix string, v reflect.Value) []genKV { + for v.Kind() == reflect.Pointer || v.Kind() == reflect.Interface { + if v.IsNil() { + return rows + } + v = v.Elem() + } + if isScalarType(v.Type()) { + if s := scalarString(v); s != "" && s != "-" { + rows = append(rows, genKV{Field: prefix, Value: s}) + } + return rows + } + switch v.Kind() { + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + if f := v.Type().Field(i); f.PkgPath == "" { + rows = appendLeafRows(rows, joinFieldPath(prefix, headerFromField(f)), v.Field(i)) + } + } + case reflect.Map: + keys := v.MapKeys() + sort.Slice(keys, func(i, j int) bool { return fmt.Sprint(keys[i]) < fmt.Sprint(keys[j]) }) + for _, k := range keys { + rows = appendLeafRows(rows, joinFieldPath(prefix, fmt.Sprint(k)), v.MapIndex(k)) + } + case reflect.Slice, reflect.Array: + for i := 0; i < v.Len(); i++ { + rows = appendLeafRows(rows, fmt.Sprintf("%s[%d]", prefix, i), v.Index(i)) + } + } + return rows +} + +func joinFieldPath(prefix, name string) string { + if prefix == "" { + return name + } + return prefix + "." + name +} + func renderMcpPerUserOAuthNotice(ctx *RunContext, v reflect.Value) error { if !isMcpPerUserOAuth(v) { return nil @@ -299,7 +337,9 @@ func fieldValue(item any, goField string) any { } // scalarString formats a scalar (or timestamp) reflect value. Non-scalars yield -// "" — the generic table never renders nested objects/arrays. +// "" — a table cell holds one scalar; appendLeafRows reaches nested ones. Byte +// slices print as --json shows them: a json.RawMessage as its compact JSON +// text, any other []byte as base64. func scalarString(fv reflect.Value) string { for fv.Kind() == reflect.Pointer { if fv.IsNil() { @@ -323,20 +363,39 @@ func scalarString(fv reflect.Value) string { return strconv.FormatUint(fv.Uint(), 10) case reflect.Float32, reflect.Float64: return strconv.FormatFloat(fv.Float(), 'f', -1, 64) + case reflect.Slice: + if !isByteSlice(fv.Type()) { + return "" + } + if fv.Type() != rawMessageType { + return base64.StdEncoding.EncodeToString(fv.Bytes()) + } + var compact bytes.Buffer + if err := json.Compact(&compact, fv.Bytes()); err != nil { + return string(fv.Bytes()) + } + return compact.String() default: return "" } } +var rawMessageType = reflect.TypeOf(json.RawMessage(nil)) + +func isByteSlice(t reflect.Type) bool { + return t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8 +} + // isScalarType reports whether t is renderable as a single table cell: a string, -// number, bool, or a timestamp (instant) type. +// number, bool, timestamp (instant) or byte slice ([]byte, json.RawMessage) — +// the byte slices are one encoded value, not an array of numbers. func isScalarType(t reflect.Type) bool { for t.Kind() == reflect.Pointer { t = t.Elem() } // Timestamp/TimestampMilli satisfy instantLike with value receivers, so the // deref'd (non-pointer) type implements it directly. - if t.Implements(instantLikeType) { + if t.Implements(instantLikeType) || isByteSlice(t) { return true } switch t.Kind() { diff --git a/internal/cli/generic_table_test.go b/internal/cli/generic_table_test.go index 238177e..e693cfc 100644 --- a/internal/cli/generic_table_test.go +++ b/internal/cli/generic_table_test.go @@ -2,6 +2,8 @@ package cli import ( "bytes" + "encoding/json" + "fmt" "reflect" "strings" "testing" @@ -144,6 +146,97 @@ func TestRenderGenericTable_DetailVertical(t *testing.T) { } } +type detailOwner struct { + Email string `json:"email"` +} + +type detailRecipient struct { + PersonID int64 `json:"person_id"` + Reason string `json:"reason"` + Status string `json:"status"` +} + +// nestedDetail is a single-object response carrying the non-scalar shapes SDK +// responses use: a nested object, an array of objects, an array of scalars, a +// map, a free-form (any) value, and an unset nested pointer — plus the byte +// slices that are single encoded values rather than arrays (raw JSON, bytes). +type nestedDetail struct { + Name string `json:"name"` + Owner detailOwner `json:"owner"` + Recipients []detailRecipient `json:"recipients"` + Tags []string `json:"tags"` + Labels map[string]string `json:"labels"` + Payload any `json:"payload"` + Backup *detailOwner `json:"backup"` + Result json.RawMessage `json:"result"` + Digest []byte `json:"digest"` +} + +// TestRenderGenericTable_DetailShowsNestedFields pins that a single object's +// table output carries every non-empty value, not only its top-level scalars: +// nested fields print under their path, so nothing json/toon would show is +// missing from the table. +func TestRenderGenericTable_DetailShowsNestedFields(t *testing.T) { + var buf bytes.Buffer + resp := &nestedDetail{ + Name: "db-rollback", + Owner: detailOwner{Email: "sre@example.com"}, + Recipients: []detailRecipient{ + {PersonID: 1, Status: "accepted"}, + {PersonID: 2, Status: "skipped", Reason: "no_email"}, + }, + Tags: []string{"prod", "db"}, + Labels: map[string]string{"service": "api", "env": "prod"}, + Payload: map[string]any{"window": []any{"22:00", "23:00"}}, + Result: json.RawMessage(`{ "rows": [1, 2] }`), + Digest: []byte("hi"), + } + if err := renderGenericTable(tableCtx(&buf), resp); err != nil { + t.Fatalf("render: %v", err) + } + + rows := [][2]string{ + {"FIELD", "VALUE"}, + {"NAME", "db-rollback"}, + {"OWNER.EMAIL", "sre@example.com"}, + {"RECIPIENTS[0].PERSON_ID", "1"}, + {"RECIPIENTS[0].STATUS", "accepted"}, + {"RECIPIENTS[1].PERSON_ID", "2"}, + {"RECIPIENTS[1].REASON", "no_email"}, + {"RECIPIENTS[1].STATUS", "skipped"}, + {"TAGS[0]", "prod"}, + {"TAGS[1]", "db"}, + {"LABELS.env", "prod"}, + {"LABELS.service", "api"}, + {"PAYLOAD.window[0]", "22:00"}, + {"PAYLOAD.window[1]", "23:00"}, + // Byte slices print as --json encodes them, not one row per byte. + {"RESULT", `{"rows":[1,2]}`}, + {"DIGEST", "aGk="}, + } + var want strings.Builder + for _, r := range rows { + fmt.Fprintf(&want, "%-25s%s\n", r[0], r[1]) + } + if got := buf.String(); got != want.String() { + t.Errorf("detail output mismatch\n got:\n%s\nwant:\n%s", got, want.String()) + } +} + +// TestRenderGenericTable_TopLevelScalarArray pins that a bare array of scalars +// (e.g. a list of names) is printed, not treated as a row table it cannot be. +func TestRenderGenericTable_TopLevelScalarArray(t *testing.T) { + var buf bytes.Buffer + if err := renderGenericTable(tableCtx(&buf), flashduty.SLSLogstoresResponse{"app-log", "audit-log"}); err != nil { + t.Fatalf("render: %v", err) + } + for _, want := range []string{"app-log", "audit-log"} { + if !strings.Contains(buf.String(), want) { + t.Errorf("output missing %q\n---\n%s", want, buf.String()) + } + } +} + func TestRenderGenericTable_McpServerItemRendersDetail(t *testing.T) { var buf bytes.Buffer item := &flashduty.McpServerItem{ diff --git a/internal/output/table.go b/internal/output/table.go index 267369f..10b7cb2 100644 --- a/internal/output/table.go +++ b/internal/output/table.go @@ -31,7 +31,10 @@ func (p *TablePrinter) Print(data any, columns []Column) error { for r, item := range items { vals := make([]string, len(columns)) for i, col := range columns { - v := col.Field(item) + // A cell is one line: fold line breaks and indentation so a + // multi-line value neither spills onto lines that read as further + // rows nor spends the column width on whitespace. + v := strings.Join(strings.Fields(col.Field(item)), " ") if !p.noTrunc && col.MaxWidth > 0 { v = Truncate(v, col.MaxWidth) } diff --git a/internal/output/table_test.go b/internal/output/table_test.go index 4ebf8f2..5d0edb8 100644 --- a/internal/output/table_test.go +++ b/internal/output/table_test.go @@ -193,6 +193,33 @@ func TestTablePrinter_NoTruncSkipsTruncation(t *testing.T) { } } +// A cell is one line: a value's line breaks and indentation fold to single +// spaces, so a multi-line value (an HTML body, a prompt) neither spills onto +// lines that read as further rows nor spends the column width on indentation. +func TestTablePrinter_MultilineValueStaysOnItsRow(t *testing.T) { + value := "\n
\r\n\tRollback at 22:00
\n \n" + for _, tt := range []struct { + noTrunc bool + want string + }{ + {noTrunc: false, want: "Rol..."}, + {noTrunc: true, want: "
Rollback at 22:00
"}, + } { + var buf bytes.Buffer + p := &TablePrinter{w: &buf, noTrunc: tt.noTrunc} + if err := p.Print([]testRow{{Name: value, Value: "next"}}, []Column{nameCol(23), valueCol(0)}); err != nil { + t.Fatalf("Print returned error: %v", err) + } + lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n") + if len(lines) != 2 { + t.Fatalf("noTrunc=%v: want header + 1 row, got %d lines: %q", tt.noTrunc, len(lines), buf.String()) + } + if got := strings.TrimSuffix(lines[1], " next"); strings.TrimRight(got, " ") != tt.want { + t.Errorf("noTrunc=%v: cell = %q, want %q", tt.noTrunc, got, tt.want) + } + } +} + func TestTablePrinter_EmptyData(t *testing.T) { // 31 var buf bytes.Buffer