From 362811b7d19c82e943907101bf22a04844b80b2d Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Jun 2024 08:01:20 +0000 Subject: [PATCH 01/13] impl'd speedtest json output --- cli/speedtest.go | 83 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 73 insertions(+), 10 deletions(-) diff --git a/cli/speedtest.go b/cli/speedtest.go index 9f8090ef99731..93004f18d779a 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -6,7 +6,6 @@ import ( "os" "time" - "github.com/jedib0t/go-pretty/v6/table" "golang.org/x/xerrors" tsspeedtest "tailscale.com/net/speedtest" "tailscale.com/wgengine/capture" @@ -19,12 +18,68 @@ import ( "github.com/coder/serpent" ) +// clui will either format this as json, +// or our speedtestTableFormatter will format this as a table +type speedtestResult struct { + Overall speedtestResultInterval `json:"overall"` + Intervals []speedtestResultInterval `json:"intervals"` +} + +type speedtestResultInterval struct { + StartTimeSeconds float64 `json:"start_time_seconds"` + EndTimeSeconds float64 `json:"end_time_seconds"` + ThroughputMbits float64 `json:"throughput_mbits"` +} + +type speedtestTableItem struct { + Interval string `table:"Interval,default_sort"` + Throughput string `table:"Throughput"` +} + +type speedtestTableFormatter struct { + tableFormatter cliui.OutputFormat +} + +var _ cliui.OutputFormat = &speedtestTableFormatter{} + +func (*speedtestTableFormatter) ID() string { + return "table" +} + +func (f *speedtestTableFormatter) AttachOptions(opts *serpent.OptionSet) { + f.tableFormatter = cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}) + f.tableFormatter.AttachOptions(opts) +} + +func (f *speedtestTableFormatter) Format(ctx context.Context, data any) (string, error) { + res, ok := data.(speedtestResult) + if !ok { + panic("attempted speedtest table format with an invalid result") + } + tableRows := make([]speedtestTableItem, len(res.Intervals)+1) + for i, r := range res.Intervals { + tableRows[i] = speedtestTableItem{ + Interval: fmt.Sprintf("%.2f-%.2f sec", r.StartTimeSeconds, r.EndTimeSeconds), + Throughput: fmt.Sprintf("%.4f Mbits/sec", r.ThroughputMbits), + } + } + tableRows[len(res.Intervals)] = speedtestTableItem{ + Interval: "Total", + Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits), + } + return f.tableFormatter.Format(ctx, tableRows) +} + func (r *RootCmd) speedtest() *serpent.Command { var ( direct bool duration time.Duration direction string pcapFile string + formatter = cliui.NewOutputFormatter( + &speedtestTableFormatter{}, + cliui.JSONFormat(), + ) ) client := new(codersdk.Client) cmd := &serpent.Command{ @@ -129,19 +184,26 @@ func (r *RootCmd) speedtest() *serpent.Command { if err != nil { return err } - tableWriter := cliui.Table() - tableWriter.AppendHeader(table.Row{"Interval", "Throughput"}) + var outputResult speedtestResult startTime := results[0].IntervalStart - for _, r := range results { + outputResult.Intervals = make([]speedtestResultInterval, len(results)-1) + for i, r := range results { + tmp := speedtestResultInterval{ + StartTimeSeconds: r.IntervalStart.Sub(startTime).Seconds(), + EndTimeSeconds: r.IntervalEnd.Sub(startTime).Seconds(), + ThroughputMbits: r.MBitsPerSecond(), + } if r.Total { - tableWriter.AppendSeparator() + outputResult.Overall = tmp + } else { + outputResult.Intervals[i] = tmp } - tableWriter.AppendRow(table.Row{ - fmt.Sprintf("%.2f-%.2f sec", r.IntervalStart.Sub(startTime).Seconds(), r.IntervalEnd.Sub(startTime).Seconds()), - fmt.Sprintf("%.4f Mbits/sec", r.MBitsPerSecond()), - }) } - _, err = fmt.Fprintln(inv.Stdout, tableWriter.Render()) + out, err := formatter.Format(inv.Context(), outputResult) + if err != nil { + return err + } + _, err = fmt.Fprintln(inv.Stdout, out) return err }, } @@ -173,5 +235,6 @@ func (r *RootCmd) speedtest() *serpent.Command { Value: serpent.StringOf(&pcapFile), }, } + formatter.AttachOptions(&cmd.Options) return cmd } From 858e15cff13902d48ebdecb35760360c79793ac2 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Jun 2024 08:21:09 +0000 Subject: [PATCH 02/13] added separator back to table output --- cli/cliui/output.go | 66 ++++++++++++++++++++++++++++++++++++- cli/cliui/table.go | 67 +++++++------------------------------- cli/cliui/table_test.go | 36 ++++++++++---------- cli/schedule.go | 2 +- cli/speedtest.go | 5 +-- cli/userstatus.go | 2 +- enterprise/cli/features.go | 2 +- 7 files changed, 100 insertions(+), 80 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index 9f06d0ba5d2cb..fea27aa1cf513 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -6,9 +6,12 @@ import ( "fmt" "reflect" "strings" + "time" "golang.org/x/xerrors" + "github.com/jedib0t/go-pretty/v6/table" + "github.com/coder/serpent" ) @@ -143,7 +146,68 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) { // Format implements OutputFormat. func (f *tableFormat) Format(_ context.Context, data any) (string, error) { - return DisplayTable(data, f.sort, f.columns) + v := reflect.Indirect(reflect.ValueOf(data)) + + headers := make(table.Row, len(f.allColumns)) + for i, header := range f.allColumns { + headers[i] = header + } + // Setup the table formatter. + tw := Table() + tw.AppendHeader(headers) + tw.SetColumnConfigs(filterTableColumns(headers, f.columns)) + if f.sort != "" { + tw.SortBy([]table.SortBy{{ + Name: f.sort, + }}) + } + + // Write each struct to the table. + for i := 0; i < v.Len(); i++ { + cur := v.Index(i) + _, ok := cur.Interface().(TableSeparator) + if ok { + tw.AppendSeparator() + continue + } + // Format the row as a slice. + rowMap, err := valueToTableMap(reflect.ValueOf(cur.Interface())) + if err != nil { + return "", xerrors.Errorf("get table row map %v: %w", i, err) + } + + rowSlice := make([]any, len(headers)) + for i, h := range f.allColumns { + v, ok := rowMap[h] + if !ok { + v = nil + } + + // Special type formatting. + switch val := v.(type) { + case time.Time: + v = val.Format(time.RFC3339) + case *time.Time: + if val != nil { + v = val.Format(time.RFC3339) + } + case *int64: + if val != nil { + v = *val + } + case fmt.Stringer: + if val != nil { + v = val.String() + } + } + + rowSlice[i] = v + } + + tw.AppendRow(table.Row(rowSlice)) + } + + return tw.Render(), nil } type jsonFormat struct{} diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 9962678be902a..3045deb8fe78e 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -1,10 +1,10 @@ package cliui import ( + "context" "fmt" "reflect" "strings" - "time" "github.com/fatih/structtag" "github.com/jedib0t/go-pretty/v6/table" @@ -22,6 +22,8 @@ func Table() table.Writer { return tableWriter } +type TableSeparator struct{} + // filterTableColumns returns configurations to hide columns // that are not provided in the array. If the array is empty, // no filtering will occur! @@ -47,7 +49,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig return columnConfigs } -// DisplayTable renders a table as a string. The input argument must be a slice +// DisplayAsTable renders a table as a string. The input argument must be a slice // of structs. At least one field in the struct must have a `table:""` tag // containing the name of the column in the outputted table. // @@ -62,7 +64,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig // // If sort is empty, the input order will be used. If filterColumns is empty or // nil, all available columns are included. -func DisplayTable(out any, sort string, filterColumns []string) (string, error) { +func DisplayAsTable(out any, sort string, filterColumns []string) (string, error) { v := reflect.Indirect(reflect.ValueOf(out)) if v.Kind() != reflect.Slice { @@ -80,11 +82,6 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) if sort == "" { sort = defaultSort } - headers := make(table.Row, len(headersRaw)) - for i, header := range headersRaw { - headers[i] = header - } - // Verify that the given sort column and filter columns are valid. if sort != "" || len(filterColumns) != 0 { headersMap := make(map[string]string, len(headersRaw)) @@ -131,56 +128,14 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) } } - // Setup the table formatter. - tw := Table() - tw.AppendHeader(headers) - tw.SetColumnConfigs(filterTableColumns(headers, filterColumns)) - if sort != "" { - tw.SortBy([]table.SortBy{{ - Name: sort, - }}) - } - - // Write each struct to the table. - for i := 0; i < v.Len(); i++ { - // Format the row as a slice. - rowMap, err := valueToTableMap(v.Index(i)) - if err != nil { - return "", xerrors.Errorf("get table row map %v: %w", i, err) - } - - rowSlice := make([]any, len(headers)) - for i, h := range headersRaw { - v, ok := rowMap[h] - if !ok { - v = nil - } - - // Special type formatting. - switch val := v.(type) { - case time.Time: - v = val.Format(time.RFC3339) - case *time.Time: - if val != nil { - v = val.Format(time.RFC3339) - } - case *int64: - if val != nil { - v = *val - } - case fmt.Stringer: - if val != nil { - v = val.String() - } - } - - rowSlice[i] = v - } - - tw.AppendRow(table.Row(rowSlice)) + tf := &tableFormat{ + allColumns: headersRaw, + defaultColumns: headersRaw, + columns: filterColumns, + sort: sort, } - return tw.Render(), nil + return tf.Format(context.Background(), out) } // parseTableStructTag returns the name of the field according to the `table` diff --git a/cli/cliui/table_test.go b/cli/cliui/table_test.go index bb0b6c658fe45..55a7d41b7e3bb 100644 --- a/cli/cliui/table_test.go +++ b/cli/cliui/table_test.go @@ -145,7 +145,7 @@ foo 10 [a b c] foo1 11 foo2 12 foo3 ` // Test with non-pointer values. - out, err := cliui.DisplayTable(in, "", nil) + out, err := cliui.DisplayAsTable(in, "", nil) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -156,7 +156,7 @@ foo 10 [a b c] foo1 11 foo2 12 foo3 v := v inPtr[i] = &v } - out, err = cliui.DisplayTable(inPtr, "", nil) + out, err = cliui.DisplayAsTable(inPtr, "", nil) require.NoError(t, err) compareTables(t, expected, out) }) @@ -171,7 +171,7 @@ bar 20 [a] bar1 21 bar3 baz 30 [] baz1 31 baz3 33 {baz4 34 } 2022-08-02T15:49:10Z ` - out, err := cliui.DisplayTable(in, "age", nil) + out, err := cliui.DisplayAsTable(in, "age", nil) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -187,7 +187,7 @@ baz baz1 baz3 2022-08-02T15:49:10Z foo foo1 foo3 2022-08-02T15:49:10Z ` - out, err := cliui.DisplayTable(in, "", []string{"name", "sub_1_name", "sub_3 inner name", "time"}) + out, err := cliui.DisplayAsTable(in, "", []string{"name", "sub_1_name", "sub_3 inner name", "time"}) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -212,7 +212,7 @@ Alice 25 }, }, } - out, err := cliui.DisplayTable(inlineIn, "", []string{"name", "age"}) + out, err := cliui.DisplayAsTable(inlineIn, "", []string{"name", "age"}) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -227,21 +227,21 @@ Alice 25 t.Parallel() var in string - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) t.Run("BadSortColumn", func(t *testing.T) { t.Parallel() - _, err := cliui.DisplayTable(in, "bad_column_does_not_exist", nil) + _, err := cliui.DisplayAsTable(in, "bad_column_does_not_exist", nil) require.Error(t, err) }) t.Run("BadFilterColumns", func(t *testing.T) { t.Parallel() - _, err := cliui.DisplayTable(in, "", []string{"name", "bad_column_does_not_exist"}) + _, err := cliui.DisplayAsTable(in, "", []string{"name", "bad_column_does_not_exist"}) require.Error(t, err) }) @@ -252,7 +252,7 @@ Alice 25 t.Parallel() var in []any - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) @@ -260,7 +260,7 @@ Alice 25 t.Parallel() in := []any{tableTest1{}} - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) }) @@ -272,7 +272,7 @@ Alice 25 t.Parallel() var in []string - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) @@ -280,7 +280,7 @@ Alice 25 t.Parallel() in := []string{"foo", "bar", "baz"} - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) }) @@ -296,7 +296,7 @@ Alice 25 t.Parallel() var in []noTableTagsTest - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) @@ -304,7 +304,7 @@ Alice 25 t.Parallel() in := []noTableTagsTest{{Field: "hi"}} - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) }) @@ -320,7 +320,7 @@ Alice 25 t.Parallel() var in []noNameTest - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) @@ -328,7 +328,7 @@ Alice 25 t.Parallel() in := []noNameTest{{Field: "test"}} - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) }) @@ -344,7 +344,7 @@ Alice 25 t.Parallel() var in []invalidSyntaxTest - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) @@ -352,7 +352,7 @@ Alice 25 t.Parallel() in := []invalidSyntaxTest{{Field: "test"}} - _, err := cliui.DisplayTable(in, "", nil) + _, err := cliui.DisplayAsTable(in, "", nil) require.Error(t, err) }) }) diff --git a/cli/schedule.go b/cli/schedule.go index 80fdc873fb205..5be2d7d656d36 100644 --- a/cli/schedule.go +++ b/cli/schedule.go @@ -286,7 +286,7 @@ func (r *RootCmd) scheduleOverride() *serpent.Command { func displaySchedule(ws codersdk.Workspace, out io.Writer) error { rows := []workspaceListRow{workspaceListRowFromWorkspace(time.Now(), ws)} - rendered, err := cliui.DisplayTable(rows, "workspace", []string{ + rendered, err := cliui.DisplayAsTable(rows, "workspace", []string{ "workspace", "starts at", "starts next", "stops after", "stops next", }) if err != nil { diff --git a/cli/speedtest.go b/cli/speedtest.go index 93004f18d779a..88647e4e0aab6 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -56,14 +56,15 @@ func (f *speedtestTableFormatter) Format(ctx context.Context, data any) (string, if !ok { panic("attempted speedtest table format with an invalid result") } - tableRows := make([]speedtestTableItem, len(res.Intervals)+1) + tableRows := make([]any, len(res.Intervals)+2) for i, r := range res.Intervals { tableRows[i] = speedtestTableItem{ Interval: fmt.Sprintf("%.2f-%.2f sec", r.StartTimeSeconds, r.EndTimeSeconds), Throughput: fmt.Sprintf("%.4f Mbits/sec", r.ThroughputMbits), } } - tableRows[len(res.Intervals)] = speedtestTableItem{ + tableRows[len(res.Intervals)] = cliui.TableSeparator{} + tableRows[len(res.Intervals)+1] = speedtestTableItem{ Interval: "Total", Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits), } diff --git a/cli/userstatus.go b/cli/userstatus.go index fae2805de710d..35ec5b0bb8051 100644 --- a/cli/userstatus.go +++ b/cli/userstatus.go @@ -63,7 +63,7 @@ func (r *RootCmd) createUserStatusCommand(sdkStatus codersdk.UserStatus) *serpen // Display the user. This uses cliui.DisplayTable directly instead // of cliui.NewOutputFormatter because we prompt immediately // afterwards. - table, err := cliui.DisplayTable([]codersdk.User{user}, "", columns) + table, err := cliui.DisplayAsTable([]codersdk.User{user}, "", columns) if err != nil { return xerrors.Errorf("render user table: %w", err) } diff --git a/enterprise/cli/features.go b/enterprise/cli/features.go index e7758121d4b8d..52c154ad26e00 100644 --- a/enterprise/cli/features.go +++ b/enterprise/cli/features.go @@ -126,5 +126,5 @@ func displayFeatures(filterColumns []string, features map[codersdk.FeatureName]c }) } - return cliui.DisplayTable(rows, "name", filterColumns) + return cliui.DisplayAsTable(rows, "name", filterColumns) } From 9a195f7451f82b08d4874609d5c1528d89307cd4 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Jun 2024 09:15:20 +0000 Subject: [PATCH 03/13] . --- cli/cliui/output.go | 7 ++++--- cli/cliui/table.go | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index fea27aa1cf513..ca00fc7c3612d 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -164,14 +164,15 @@ func (f *tableFormat) Format(_ context.Context, data any) (string, error) { // Write each struct to the table. for i := 0; i < v.Len(); i++ { - cur := v.Index(i) - _, ok := cur.Interface().(TableSeparator) + cur := v.Index(i).Interface() + _, ok := cur.(TableSeparator) if ok { tw.AppendSeparator() continue } // Format the row as a slice. - rowMap, err := valueToTableMap(reflect.ValueOf(cur.Interface())) + // ValueToTableMap does what `reflect.Indirect` does + rowMap, err := valueToTableMap(reflect.ValueOf(cur)) if err != nil { return "", xerrors.Errorf("get table row map %v: %w", i, err) } diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 3045deb8fe78e..89d67d07c8885 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -128,6 +128,7 @@ func DisplayAsTable(out any, sort string, filterColumns []string) (string, error } } + // TODO(ethan): remove type import? tf := &tableFormat{ allColumns: headersRaw, defaultColumns: headersRaw, From ff91d688a4f04dfeadcc036abcf4f915ed5f7a88 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Jun 2024 14:38:35 +0000 Subject: [PATCH 04/13] WIP --- cli/cliui/output.go | 67 +---------------------------------------- cli/cliui/table.go | 73 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 66 insertions(+), 74 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index ca00fc7c3612d..3a677a6ba4a6b 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -6,12 +6,9 @@ import ( "fmt" "reflect" "strings" - "time" "golang.org/x/xerrors" - "github.com/jedib0t/go-pretty/v6/table" - "github.com/coder/serpent" ) @@ -146,69 +143,7 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) { // Format implements OutputFormat. func (f *tableFormat) Format(_ context.Context, data any) (string, error) { - v := reflect.Indirect(reflect.ValueOf(data)) - - headers := make(table.Row, len(f.allColumns)) - for i, header := range f.allColumns { - headers[i] = header - } - // Setup the table formatter. - tw := Table() - tw.AppendHeader(headers) - tw.SetColumnConfigs(filterTableColumns(headers, f.columns)) - if f.sort != "" { - tw.SortBy([]table.SortBy{{ - Name: f.sort, - }}) - } - - // Write each struct to the table. - for i := 0; i < v.Len(); i++ { - cur := v.Index(i).Interface() - _, ok := cur.(TableSeparator) - if ok { - tw.AppendSeparator() - continue - } - // Format the row as a slice. - // ValueToTableMap does what `reflect.Indirect` does - rowMap, err := valueToTableMap(reflect.ValueOf(cur)) - if err != nil { - return "", xerrors.Errorf("get table row map %v: %w", i, err) - } - - rowSlice := make([]any, len(headers)) - for i, h := range f.allColumns { - v, ok := rowMap[h] - if !ok { - v = nil - } - - // Special type formatting. - switch val := v.(type) { - case time.Time: - v = val.Format(time.RFC3339) - case *time.Time: - if val != nil { - v = val.Format(time.RFC3339) - } - case *int64: - if val != nil { - v = *val - } - case fmt.Stringer: - if val != nil { - v = val.String() - } - } - - rowSlice[i] = v - } - - tw.AppendRow(table.Row(rowSlice)) - } - - return tw.Render(), nil + return displayTable(data, f.sort, f.columns, nil) } type jsonFormat struct{} diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 89d67d07c8885..106ca2334b8e8 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -1,10 +1,10 @@ package cliui import ( - "context" "fmt" "reflect" "strings" + "time" "github.com/fatih/structtag" "github.com/jedib0t/go-pretty/v6/table" @@ -127,16 +127,73 @@ func DisplayAsTable(out any, sort string, filterColumns []string) (string, error return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`)) } } + return displayTable(out, sort, headersRaw, filterColumns) +} + +func displayTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { + v := reflect.Indirect(reflect.ValueOf(out)) + + headers := make(table.Row, len(headersRaw)) + for i, header := range headersRaw { + headers[i] = header + } + // Setup the table formatter. + tw := Table() + tw.AppendHeader(headers) + tw.SetColumnConfigs(filterTableColumns(headers, filterColumns)) + if sort != "" { + tw.SortBy([]table.SortBy{{ + Name: sort, + }}) + } + + // Write each struct to the table. + for i := 0; i < v.Len(); i++ { + cur := v.Index(i).Interface() + _, ok := cur.(TableSeparator) + if ok { + tw.AppendSeparator() + continue + } + // Format the row as a slice. + // ValueToTableMap does what `reflect.Indirect` does + rowMap, err := valueToTableMap(reflect.ValueOf(cur)) + if err != nil { + return "", xerrors.Errorf("get table row map %v: %w", i, err) + } + + rowSlice := make([]any, len(headers)) + for i, h := range headersRaw { + v, ok := rowMap[h] + if !ok { + v = nil + } + + // Special type formatting. + switch val := v.(type) { + case time.Time: + v = val.Format(time.RFC3339) + case *time.Time: + if val != nil { + v = val.Format(time.RFC3339) + } + case *int64: + if val != nil { + v = *val + } + case fmt.Stringer: + if val != nil { + v = val.String() + } + } + + rowSlice[i] = v + } - // TODO(ethan): remove type import? - tf := &tableFormat{ - allColumns: headersRaw, - defaultColumns: headersRaw, - columns: filterColumns, - sort: sort, + tw.AppendRow(table.Row(rowSlice)) } - return tf.Format(context.Background(), out) + return tw.Render(), nil } // parseTableStructTag returns the name of the field according to the `table` From db700cb0b31ab3daa0d681c25c55d461ae1432aa Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Jun 2024 14:39:38 +0000 Subject: [PATCH 05/13] WIP --- cli/cliui/table.go | 4 ++-- cli/cliui/table_test.go | 36 ++++++++++++++++++------------------ cli/schedule.go | 2 +- cli/userstatus.go | 2 +- enterprise/cli/features.go | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 106ca2334b8e8..d6d2c2b0acb82 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -49,7 +49,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig return columnConfigs } -// DisplayAsTable renders a table as a string. The input argument must be a slice +// DisplayTable renders a table as a string. The input argument must be a slice // of structs. At least one field in the struct must have a `table:""` tag // containing the name of the column in the outputted table. // @@ -64,7 +64,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig // // If sort is empty, the input order will be used. If filterColumns is empty or // nil, all available columns are included. -func DisplayAsTable(out any, sort string, filterColumns []string) (string, error) { +func DisplayTable(out any, sort string, filterColumns []string) (string, error) { v := reflect.Indirect(reflect.ValueOf(out)) if v.Kind() != reflect.Slice { diff --git a/cli/cliui/table_test.go b/cli/cliui/table_test.go index 55a7d41b7e3bb..bb0b6c658fe45 100644 --- a/cli/cliui/table_test.go +++ b/cli/cliui/table_test.go @@ -145,7 +145,7 @@ foo 10 [a b c] foo1 11 foo2 12 foo3 ` // Test with non-pointer values. - out, err := cliui.DisplayAsTable(in, "", nil) + out, err := cliui.DisplayTable(in, "", nil) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -156,7 +156,7 @@ foo 10 [a b c] foo1 11 foo2 12 foo3 v := v inPtr[i] = &v } - out, err = cliui.DisplayAsTable(inPtr, "", nil) + out, err = cliui.DisplayTable(inPtr, "", nil) require.NoError(t, err) compareTables(t, expected, out) }) @@ -171,7 +171,7 @@ bar 20 [a] bar1 21 bar3 baz 30 [] baz1 31 baz3 33 {baz4 34 } 2022-08-02T15:49:10Z ` - out, err := cliui.DisplayAsTable(in, "age", nil) + out, err := cliui.DisplayTable(in, "age", nil) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -187,7 +187,7 @@ baz baz1 baz3 2022-08-02T15:49:10Z foo foo1 foo3 2022-08-02T15:49:10Z ` - out, err := cliui.DisplayAsTable(in, "", []string{"name", "sub_1_name", "sub_3 inner name", "time"}) + out, err := cliui.DisplayTable(in, "", []string{"name", "sub_1_name", "sub_3 inner name", "time"}) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -212,7 +212,7 @@ Alice 25 }, }, } - out, err := cliui.DisplayAsTable(inlineIn, "", []string{"name", "age"}) + out, err := cliui.DisplayTable(inlineIn, "", []string{"name", "age"}) log.Println("rendered table:\n" + out) require.NoError(t, err) compareTables(t, expected, out) @@ -227,21 +227,21 @@ Alice 25 t.Parallel() var in string - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) t.Run("BadSortColumn", func(t *testing.T) { t.Parallel() - _, err := cliui.DisplayAsTable(in, "bad_column_does_not_exist", nil) + _, err := cliui.DisplayTable(in, "bad_column_does_not_exist", nil) require.Error(t, err) }) t.Run("BadFilterColumns", func(t *testing.T) { t.Parallel() - _, err := cliui.DisplayAsTable(in, "", []string{"name", "bad_column_does_not_exist"}) + _, err := cliui.DisplayTable(in, "", []string{"name", "bad_column_does_not_exist"}) require.Error(t, err) }) @@ -252,7 +252,7 @@ Alice 25 t.Parallel() var in []any - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) @@ -260,7 +260,7 @@ Alice 25 t.Parallel() in := []any{tableTest1{}} - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) }) @@ -272,7 +272,7 @@ Alice 25 t.Parallel() var in []string - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) @@ -280,7 +280,7 @@ Alice 25 t.Parallel() in := []string{"foo", "bar", "baz"} - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) }) @@ -296,7 +296,7 @@ Alice 25 t.Parallel() var in []noTableTagsTest - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) @@ -304,7 +304,7 @@ Alice 25 t.Parallel() in := []noTableTagsTest{{Field: "hi"}} - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) }) @@ -320,7 +320,7 @@ Alice 25 t.Parallel() var in []noNameTest - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) @@ -328,7 +328,7 @@ Alice 25 t.Parallel() in := []noNameTest{{Field: "test"}} - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) }) @@ -344,7 +344,7 @@ Alice 25 t.Parallel() var in []invalidSyntaxTest - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) @@ -352,7 +352,7 @@ Alice 25 t.Parallel() in := []invalidSyntaxTest{{Field: "test"}} - _, err := cliui.DisplayAsTable(in, "", nil) + _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) }) diff --git a/cli/schedule.go b/cli/schedule.go index 5be2d7d656d36..80fdc873fb205 100644 --- a/cli/schedule.go +++ b/cli/schedule.go @@ -286,7 +286,7 @@ func (r *RootCmd) scheduleOverride() *serpent.Command { func displaySchedule(ws codersdk.Workspace, out io.Writer) error { rows := []workspaceListRow{workspaceListRowFromWorkspace(time.Now(), ws)} - rendered, err := cliui.DisplayAsTable(rows, "workspace", []string{ + rendered, err := cliui.DisplayTable(rows, "workspace", []string{ "workspace", "starts at", "starts next", "stops after", "stops next", }) if err != nil { diff --git a/cli/userstatus.go b/cli/userstatus.go index 35ec5b0bb8051..fae2805de710d 100644 --- a/cli/userstatus.go +++ b/cli/userstatus.go @@ -63,7 +63,7 @@ func (r *RootCmd) createUserStatusCommand(sdkStatus codersdk.UserStatus) *serpen // Display the user. This uses cliui.DisplayTable directly instead // of cliui.NewOutputFormatter because we prompt immediately // afterwards. - table, err := cliui.DisplayAsTable([]codersdk.User{user}, "", columns) + table, err := cliui.DisplayTable([]codersdk.User{user}, "", columns) if err != nil { return xerrors.Errorf("render user table: %w", err) } diff --git a/enterprise/cli/features.go b/enterprise/cli/features.go index 52c154ad26e00..e7758121d4b8d 100644 --- a/enterprise/cli/features.go +++ b/enterprise/cli/features.go @@ -126,5 +126,5 @@ func displayFeatures(filterColumns []string, features map[codersdk.FeatureName]c }) } - return cliui.DisplayAsTable(rows, "name", filterColumns) + return cliui.DisplayTable(rows, "name", filterColumns) } From 5de0f3bd6ad7c1a9bdd28e212ecaadf507b92ba4 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Jun 2024 14:42:31 +0000 Subject: [PATCH 06/13] WIP --- cli/cliui/output.go | 2 +- cli/cliui/table.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index 3a677a6ba4a6b..21ac7fd72ba25 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -143,7 +143,7 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) { // Format implements OutputFormat. func (f *tableFormat) Format(_ context.Context, data any) (string, error) { - return displayTable(data, f.sort, f.columns, nil) + return doDisplayTable(data, f.sort, f.columns, nil) } type jsonFormat struct{} diff --git a/cli/cliui/table.go b/cli/cliui/table.go index d6d2c2b0acb82..8e75005c442ed 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -127,10 +127,10 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`)) } } - return displayTable(out, sort, headersRaw, filterColumns) + return doDisplayTable(out, sort, headersRaw, filterColumns) } -func displayTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { +func doDisplayTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { v := reflect.Indirect(reflect.ValueOf(out)) headers := make(table.Row, len(headersRaw)) From 2a1eebe96530afb4e9088d06208c7fe5f3d59a80 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 05:21:04 +0000 Subject: [PATCH 07/13] WIP --- cli/cliui/table.go | 21 +++++++++++++++++--- cli/cliui/table_test.go | 44 +++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 8e75005c442ed..585d532ec05ab 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -22,6 +22,9 @@ func Table() table.Writer { return tableWriter } +// This type can be supplied as part of a slice to DisplayTable or to a `TableFormat` `Format` call to render a separator +// Trailing or leading separators are ignored by go-pretty +// e.g. `[]any{someRow, TableSeparator, someRow}` type TableSeparator struct{} // filterTableColumns returns configurations to hide columns @@ -49,8 +52,11 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig return columnConfigs } -// DisplayTable renders a table as a string. The input argument must be a slice -// of structs. At least one field in the struct must have a `table:""` tag +// DisplayTable renders a table as a string. The input argument can be: +// - a struct slice +// - an interface slice, where the first element is a struct, and all other elements are of the same type, or a TableSeperator +// +// At least one field in the struct must have a `table:""` tag // containing the name of the column in the outputted table. // // If `sort` is not specified, the field with the `table:"$NAME,default_sort"` @@ -70,9 +76,18 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) if v.Kind() != reflect.Slice { return "", xerrors.Errorf("DisplayTable called with a non-slice type") } + var fst reflect.Type + if v.Type().Elem().Kind() == reflect.Interface { + if v.Len() == 0 { + return "", xerrors.Errorf("DisplayTable called with empty interface slice") + } + fst = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type() + } else { + fst = v.Type().Elem() + } // Get the list of table column headers. - headersRaw, defaultSort, err := typeToTableHeaders(v.Type().Elem(), true) + headersRaw, defaultSort, err := typeToTableHeaders(fst, true) if err != nil { return "", xerrors.Errorf("get table headers recursively for type %q: %w", v.Type().Elem().String(), err) } diff --git a/cli/cliui/table_test.go b/cli/cliui/table_test.go index bb0b6c658fe45..5772c5cf5869e 100644 --- a/cli/cliui/table_test.go +++ b/cli/cliui/table_test.go @@ -218,6 +218,42 @@ Alice 25 compareTables(t, expected, out) }) + // This test ensures we can display dynamically typed slices + t.Run("Interfaces", func(t *testing.T) { + t.Parallel() + + in := []any{tableTest1{}} + out, err := cliui.DisplayTable(in, "", nil) + t.Log("rendered table:\n" + out) + require.NoError(t, err) + other := []tableTest1{{}} + expected, err := cliui.DisplayTable(other, "", nil) + require.NoError(t, err) + compareTables(t, expected, out) + }) + + t.Run("WithSeparator", func(t *testing.T) { + t.Parallel() + expected := ` +NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR +bar 20 [a] bar1 21 bar3 23 {bar4 24 } 2022-08-02T15:49:10Z +------------------------------------------------------------------------------------------------------------------------------------------------------------- +baz 30 [] baz1 31 baz3 33 {baz4 34 } 2022-08-02T15:49:10Z +------------------------------------------------------------------------------------------------------------------------------------------------------------- +foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z + ` + + var inlineIn []any + for _, v := range in { + inlineIn = append(inlineIn, v) + inlineIn = append(inlineIn, cliui.TableSeparator{}) + } + out, err := cliui.DisplayTable(inlineIn, "", nil) + t.Log("rendered table:\n" + out) + require.NoError(t, err) + compareTables(t, expected, out) + }) + // This test ensures that safeties against invalid use of `table` tags // causes errors (even without data). t.Run("Errors", func(t *testing.T) { @@ -255,14 +291,6 @@ Alice 25 _, err := cliui.DisplayTable(in, "", nil) require.Error(t, err) }) - - t.Run("WithData", func(t *testing.T) { - t.Parallel() - - in := []any{tableTest1{}} - _, err := cliui.DisplayTable(in, "", nil) - require.Error(t, err) - }) }) t.Run("NotStruct", func(t *testing.T) { From a93efdb1371b7ed95a166b2d757a4ebdce00b24e Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 05:26:38 +0000 Subject: [PATCH 08/13] WIP --- cli/cliui/output.go | 2 +- cli/cliui/table.go | 12 ++++++------ cli/speedtest.go | 1 + 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index 21ac7fd72ba25..6fcc85604392c 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -143,7 +143,7 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) { // Format implements OutputFormat. func (f *tableFormat) Format(_ context.Context, data any) (string, error) { - return doDisplayTable(data, f.sort, f.columns, nil) + return renderTable(data, f.sort, f.columns, nil) } type jsonFormat struct{} diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 585d532ec05ab..852bdd1736f78 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -76,18 +76,18 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) if v.Kind() != reflect.Slice { return "", xerrors.Errorf("DisplayTable called with a non-slice type") } - var fst reflect.Type + var tableType reflect.Type if v.Type().Elem().Kind() == reflect.Interface { if v.Len() == 0 { return "", xerrors.Errorf("DisplayTable called with empty interface slice") } - fst = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type() + tableType = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type() } else { - fst = v.Type().Elem() + tableType = v.Type().Elem() } // Get the list of table column headers. - headersRaw, defaultSort, err := typeToTableHeaders(fst, true) + headersRaw, defaultSort, err := typeToTableHeaders(tableType, true) if err != nil { return "", xerrors.Errorf("get table headers recursively for type %q: %w", v.Type().Elem().String(), err) } @@ -142,10 +142,10 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`)) } } - return doDisplayTable(out, sort, headersRaw, filterColumns) + return renderTable(out, sort, headersRaw, filterColumns) } -func doDisplayTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { +func renderTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { v := reflect.Indirect(reflect.ValueOf(out)) headers := make(table.Row, len(headersRaw)) diff --git a/cli/speedtest.go b/cli/speedtest.go index 88647e4e0aab6..3b9f44295f81d 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -36,6 +36,7 @@ type speedtestTableItem struct { Throughput string `table:"Throughput"` } +// Attaches a CLI arg with table output type speedtestTableFormatter struct { tableFormatter cliui.OutputFormat } From ed03cec24c1a5c00711d809ef3b2aafc444af298 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 05:49:55 +0000 Subject: [PATCH 09/13] WIP --- cli/speedtest.go | 59 ++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) diff --git a/cli/speedtest.go b/cli/speedtest.go index 3b9f44295f81d..f376e48bb8a1b 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -18,8 +18,6 @@ import ( "github.com/coder/serpent" ) -// clui will either format this as json, -// or our speedtestTableFormatter will format this as a table type speedtestResult struct { Overall speedtestResultInterval `json:"overall"` Intervals []speedtestResultInterval `json:"intervals"` @@ -36,42 +34,6 @@ type speedtestTableItem struct { Throughput string `table:"Throughput"` } -// Attaches a CLI arg with table output -type speedtestTableFormatter struct { - tableFormatter cliui.OutputFormat -} - -var _ cliui.OutputFormat = &speedtestTableFormatter{} - -func (*speedtestTableFormatter) ID() string { - return "table" -} - -func (f *speedtestTableFormatter) AttachOptions(opts *serpent.OptionSet) { - f.tableFormatter = cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}) - f.tableFormatter.AttachOptions(opts) -} - -func (f *speedtestTableFormatter) Format(ctx context.Context, data any) (string, error) { - res, ok := data.(speedtestResult) - if !ok { - panic("attempted speedtest table format with an invalid result") - } - tableRows := make([]any, len(res.Intervals)+2) - for i, r := range res.Intervals { - tableRows[i] = speedtestTableItem{ - Interval: fmt.Sprintf("%.2f-%.2f sec", r.StartTimeSeconds, r.EndTimeSeconds), - Throughput: fmt.Sprintf("%.4f Mbits/sec", r.ThroughputMbits), - } - } - tableRows[len(res.Intervals)] = cliui.TableSeparator{} - tableRows[len(res.Intervals)+1] = speedtestTableItem{ - Interval: "Total", - Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits), - } - return f.tableFormatter.Format(ctx, tableRows) -} - func (r *RootCmd) speedtest() *serpent.Command { var ( direct bool @@ -79,7 +41,26 @@ func (r *RootCmd) speedtest() *serpent.Command { direction string pcapFile string formatter = cliui.NewOutputFormatter( - &speedtestTableFormatter{}, + cliui.ChangeFormatterData(cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}), func(data any) (any, error) { + res, ok := data.(speedtestResult) + if !ok { + // This should never happen + return "", xerrors.Errorf("expected speedtestResult, got %T", data) + } + tableRows := make([]any, len(res.Intervals)+2) + for i, r := range res.Intervals { + tableRows[i] = speedtestTableItem{ + Interval: fmt.Sprintf("%.2f-%.2f sec", r.StartTimeSeconds, r.EndTimeSeconds), + Throughput: fmt.Sprintf("%.4f Mbits/sec", r.ThroughputMbits), + } + } + tableRows[len(res.Intervals)] = cliui.TableSeparator{} + tableRows[len(res.Intervals)+1] = speedtestTableItem{ + Interval: "Total", + Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits), + } + return tableRows, nil + }), cliui.JSONFormat(), ) ) From f3cc1eace2e3d861b486b0668d54c2810feff8a5 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 06:14:30 +0000 Subject: [PATCH 10/13] WIP --- cli/speedtest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/speedtest.go b/cli/speedtest.go index f376e48bb8a1b..95ff8bfb937d4 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -162,7 +162,7 @@ func (r *RootCmd) speedtest() *serpent.Command { default: return xerrors.Errorf("invalid direction: %q", direction) } - cliui.Infof(inv.Stdout, "Starting a %ds %s test...", int(duration.Seconds()), tsDir) + cliui.Infof(inv.Stderr, "Starting a %ds %s test...", int(duration.Seconds()), tsDir) results, err := conn.Speedtest(ctx, tsDir, duration) if err != nil { return err From fa5e1c05d2db65a117d4141190eba27fdc10ff4a Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 06:17:52 +0000 Subject: [PATCH 11/13] WIP --- cli/cliui/table.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 852bdd1736f78..84727aec47b8b 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -22,8 +22,10 @@ func Table() table.Writer { return tableWriter } -// This type can be supplied as part of a slice to DisplayTable or to a `TableFormat` `Format` call to render a separator -// Trailing or leading separators are ignored by go-pretty +// This type can be supplied as part of a slice to DisplayTable +// or to a `TableFormat` `Format` call to render a separator. +// Leading separators are not supported and trailing separators +// are ignored by the table formatter. // e.g. `[]any{someRow, TableSeparator, someRow}` type TableSeparator struct{} @@ -53,8 +55,9 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig } // DisplayTable renders a table as a string. The input argument can be: -// - a struct slice -// - an interface slice, where the first element is a struct, and all other elements are of the same type, or a TableSeperator +// - a struct slice. +// - an interface slice, where the first element is a struct, +// and all other elements are of the same type, or a TableSeperator. // // At least one field in the struct must have a `table:""` tag // containing the name of the column in the outputted table. From 451180c472b43c4878c653caa27410d21710195c Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 08:07:38 +0000 Subject: [PATCH 12/13] WIP --- cli/cliui/output.go | 7 +++- cli/cliui/table.go | 47 +++++++++++++--------- cli/speedtest.go | 25 ++++++------ cli/speedtest_test.go | 45 +++++++++++++++++++++ cli/testdata/coder_speedtest_--help.golden | 7 ++++ 5 files changed, 98 insertions(+), 33 deletions(-) diff --git a/cli/cliui/output.go b/cli/cliui/output.go index 6fcc85604392c..d15d18b63fe18 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -7,6 +7,7 @@ import ( "reflect" "strings" + "github.com/jedib0t/go-pretty/v6/table" "golang.org/x/xerrors" "github.com/coder/serpent" @@ -143,7 +144,11 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) { // Format implements OutputFormat. func (f *tableFormat) Format(_ context.Context, data any) (string, error) { - return renderTable(data, f.sort, f.columns, nil) + headers := make(table.Row, len(f.allColumns)) + for i, header := range f.allColumns { + headers[i] = header + } + return renderTable(data, f.sort, headers, f.columns) } type jsonFormat struct{} diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 84727aec47b8b..f1fb8075133c8 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -57,7 +57,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig // DisplayTable renders a table as a string. The input argument can be: // - a struct slice. // - an interface slice, where the first element is a struct, -// and all other elements are of the same type, or a TableSeperator. +// and all other elements are of the same type, or a TableSeparator. // // At least one field in the struct must have a `table:""` tag // containing the name of the column in the outputted table. @@ -77,12 +77,12 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) v := reflect.Indirect(reflect.ValueOf(out)) if v.Kind() != reflect.Slice { - return "", xerrors.Errorf("DisplayTable called with a non-slice type") + return "", xerrors.New("DisplayTable called with a non-slice type") } var tableType reflect.Type if v.Type().Elem().Kind() == reflect.Interface { if v.Len() == 0 { - return "", xerrors.Errorf("DisplayTable called with empty interface slice") + return "", xerrors.New("DisplayTable called with empty interface slice") } tableType = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type() } else { @@ -100,6 +100,10 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) if sort == "" { sort = defaultSort } + headers := make(table.Row, len(headersRaw)) + for i, header := range headersRaw { + headers[i] = strings.ReplaceAll(header, "_", " ") + } // Verify that the given sort column and filter columns are valid. if sort != "" || len(filterColumns) != 0 { headersMap := make(map[string]string, len(headersRaw)) @@ -145,16 +149,12 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`)) } } - return renderTable(out, sort, headersRaw, filterColumns) + return renderTable(out, sort, headers, filterColumns) } -func renderTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) { +func renderTable(out any, sort string, headers table.Row, filterColumns []string) (string, error) { v := reflect.Indirect(reflect.ValueOf(out)) - headers := make(table.Row, len(headersRaw)) - for i, header := range headersRaw { - headers[i] = header - } // Setup the table formatter. tw := Table() tw.AppendHeader(headers) @@ -181,8 +181,8 @@ func renderTable(out any, sort string, headersRaw []string, filterColumns []stri } rowSlice := make([]any, len(headers)) - for i, h := range headersRaw { - v, ok := rowMap[h] + for i, h := range headers { + v, ok := rowMap[h.(string)] if !ok { v = nil } @@ -219,25 +219,28 @@ func renderTable(out any, sort string, headersRaw []string, filterColumns []stri // returned. If the table tag is malformed, an error is returned. // // The returned name is transformed from "snake_case" to "normal text". -func parseTableStructTag(field reflect.StructField) (name string, defaultSort, recursive bool, skipParentName bool, err error) { +func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName bool, err error) { tags, err := structtag.Parse(string(field.Tag)) if err != nil { - return "", false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err) + return "", false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err) } tag, err := tags.Get("table") if err != nil || tag.Name == "-" { // tags.Get only returns an error if the tag is not found. - return "", false, false, false, nil + return "", false, false, false, false, nil } defaultSortOpt := false + noSortOpt = false recursiveOpt := false skipParentNameOpt := false for _, opt := range tag.Options { switch opt { case "default_sort": defaultSortOpt = true + case "nosort": + noSortOpt = true case "recursive": recursiveOpt = true case "recursive_inline": @@ -247,11 +250,11 @@ func parseTableStructTag(field reflect.StructField) (name string, defaultSort, r recursiveOpt = true skipParentNameOpt = true default: - return "", false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt) + return "", false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt) } } - return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, recursiveOpt, skipParentNameOpt, nil + return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, nil } func isStructOrStructPointer(t reflect.Type) bool { @@ -275,12 +278,16 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string, headers := []string{} defaultSortName := "" + noSortOpt := false for i := 0; i < t.NumField(); i++ { field := t.Field(i) - name, defaultSort, recursive, skip, err := parseTableStructTag(field) + name, defaultSort, noSort, recursive, skip, err := parseTableStructTag(field) if err != nil { return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err) } + if requireDefault && noSort { + noSortOpt = true + } if name == "" && (recursive && skip) { return nil, "", xerrors.Errorf("a name is required for the field %q. "+ @@ -323,8 +330,8 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string, headers = append(headers, name) } - if defaultSortName == "" && requireDefault { - return nil, "", xerrors.Errorf("no field marked as default_sort in type %q", t.String()) + if defaultSortName == "" && requireDefault && !noSortOpt { + return nil, "", xerrors.Errorf("no field marked as default_sort or nosort in type %q", t.String()) } return headers, defaultSortName, nil @@ -351,7 +358,7 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) { for i := 0; i < val.NumField(); i++ { field := val.Type().Field(i) fieldVal := val.Field(i) - name, _, recursive, skip, err := parseTableStructTag(field) + name, _, _, recursive, skip, err := parseTableStructTag(field) if err != nil { return nil, xerrors.Errorf("parse struct tags for field %q in type %T: %w", field.Name, val, err) } diff --git a/cli/speedtest.go b/cli/speedtest.go index 95ff8bfb937d4..3e0e74668b3b7 100644 --- a/cli/speedtest.go +++ b/cli/speedtest.go @@ -18,19 +18,19 @@ import ( "github.com/coder/serpent" ) -type speedtestResult struct { - Overall speedtestResultInterval `json:"overall"` - Intervals []speedtestResultInterval `json:"intervals"` +type SpeedtestResult struct { + Overall SpeedtestResultInterval `json:"overall"` + Intervals []SpeedtestResultInterval `json:"intervals"` } -type speedtestResultInterval struct { +type SpeedtestResultInterval struct { StartTimeSeconds float64 `json:"start_time_seconds"` EndTimeSeconds float64 `json:"end_time_seconds"` ThroughputMbits float64 `json:"throughput_mbits"` } type speedtestTableItem struct { - Interval string `table:"Interval,default_sort"` + Interval string `table:"Interval,nosort"` Throughput string `table:"Throughput"` } @@ -42,7 +42,7 @@ func (r *RootCmd) speedtest() *serpent.Command { pcapFile string formatter = cliui.NewOutputFormatter( cliui.ChangeFormatterData(cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}), func(data any) (any, error) { - res, ok := data.(speedtestResult) + res, ok := data.(SpeedtestResult) if !ok { // This should never happen return "", xerrors.Errorf("expected speedtestResult, got %T", data) @@ -56,7 +56,7 @@ func (r *RootCmd) speedtest() *serpent.Command { } tableRows[len(res.Intervals)] = cliui.TableSeparator{} tableRows[len(res.Intervals)+1] = speedtestTableItem{ - Interval: "Total", + Interval: fmt.Sprintf("%.2f-%.2f sec", res.Overall.StartTimeSeconds, res.Overall.EndTimeSeconds), Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits), } return tableRows, nil @@ -167,19 +167,20 @@ func (r *RootCmd) speedtest() *serpent.Command { if err != nil { return err } - var outputResult speedtestResult + var outputResult SpeedtestResult startTime := results[0].IntervalStart - outputResult.Intervals = make([]speedtestResultInterval, len(results)-1) + outputResult.Intervals = make([]SpeedtestResultInterval, len(results)-1) for i, r := range results { - tmp := speedtestResultInterval{ + interval := SpeedtestResultInterval{ StartTimeSeconds: r.IntervalStart.Sub(startTime).Seconds(), EndTimeSeconds: r.IntervalEnd.Sub(startTime).Seconds(), ThroughputMbits: r.MBitsPerSecond(), } if r.Total { - outputResult.Overall = tmp + interval.StartTimeSeconds = 0 + outputResult.Overall = interval } else { - outputResult.Intervals[i] = tmp + outputResult.Intervals[i] = interval } } out, err := formatter.Format(inv.Context(), outputResult) diff --git a/cli/speedtest_test.go b/cli/speedtest_test.go index 9878ff04ab527..281fdcc1488d0 100644 --- a/cli/speedtest_test.go +++ b/cli/speedtest_test.go @@ -1,7 +1,9 @@ package cli_test import ( + "bytes" "context" + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -10,6 +12,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/agent/agenttest" + "github.com/coder/coder/v2/cli" "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" @@ -56,3 +59,45 @@ func TestSpeedtest(t *testing.T) { }) <-cmdDone } + +func TestSpeedtestJson(t *testing.T) { + t.Parallel() + t.Skip("Potentially flaky test - see https://github.com/coder/coder/issues/6321") + if testing.Short() { + t.Skip("This test takes a minimum of 5ms per a hardcoded value in Tailscale!") + } + client, workspace, agentToken := setupWorkspaceForAgent(t) + _ = agenttest.New(t, client.URL, agentToken) + coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + require.Eventually(t, func() bool { + ws, err := client.Workspace(ctx, workspace.ID) + if !assert.NoError(t, err) { + return false + } + a := ws.LatestBuild.Resources[0].Agents[0] + return a.Status == codersdk.WorkspaceAgentConnected && + a.LifecycleState == codersdk.WorkspaceAgentLifecycleReady + }, testutil.WaitLong, testutil.IntervalFast, "agent is not ready") + + inv, root := clitest.New(t, "speedtest", "--output=json", workspace.Name) + clitest.SetupConfig(t, client, root) + out := bytes.NewBuffer(nil) + inv.Stdout = out + ctx, cancel = context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + inv.Logger = slogtest.Make(t, nil).Named("speedtest").Leveled(slog.LevelDebug) + cmdDone := tGo(t, func() { + err := inv.WithContext(ctx).Run() + assert.NoError(t, err) + }) + <-cmdDone + + var result cli.SpeedtestResult + require.NoError(t, json.Unmarshal(out.Bytes(), &result)) + require.Len(t, result.Intervals, 5) +} diff --git a/cli/testdata/coder_speedtest_--help.golden b/cli/testdata/coder_speedtest_--help.golden index 60eb4026b1028..538c955fae252 100644 --- a/cli/testdata/coder_speedtest_--help.golden +++ b/cli/testdata/coder_speedtest_--help.golden @@ -6,6 +6,10 @@ USAGE: Run upload and download tests from your machine to a workspace OPTIONS: + -c, --column string-array (default: Interval,Throughput) + Columns to display in table output. Available columns: Interval, + Throughput. + -d, --direct bool Specifies whether to wait for a direct connection before testing speed. @@ -14,6 +18,9 @@ OPTIONS: Specifies whether to run in reverse mode where the client receives and the server sends. + -o, --output string (default: table) + Output format. Available formats: table, json. + --pcap-file string Specifies a file to write a network capture to. From 1c4378ee56a227b27a30388893bcc3bd0f6681a1 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Jun 2024 08:22:58 +0000 Subject: [PATCH 13/13] WIP --- docs/cli/speedtest.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/cli/speedtest.md b/docs/cli/speedtest.md index e2d3a435fb0ea..ab9d9a4f7e49c 100644 --- a/docs/cli/speedtest.md +++ b/docs/cli/speedtest.md @@ -45,3 +45,21 @@ Specifies the duration to monitor traffic. | Type | string | Specifies a file to write a network capture to. + +### -c, --column + +| | | +| ------- | -------------------------------- | +| Type | string-array | +| Default | Interval,Throughput | + +Columns to display in table output. Available columns: Interval, Throughput. + +### -o, --output + +| | | +| ------- | ------------------- | +| Type | string | +| Default | table | + +Output format. Available formats: table, json.