Skip to content

Commit

Permalink
refactor: tablewriter that can handle unicode (#995)
Browse files Browse the repository at this point in the history 
The previously used `text/tabwriter` does not even attempt to handle
Unicode characters that may have a default width != 0. This causes
issues in tables where any column has such a Unicode character (like
emojis), as the alignment for that row would be broken.

We already use the new package for generating our docs, so this "only"
moves it from tools to production dependency.

I tried to copy the previous style as much as possible and left the
surrounding code as is. There is some potential for simplification
because the new package can handle a lot of the column/view logic for
us.
  • Loading branch information
@apricote
apricote authored Mar 3, 2025
1 parent cb1bb68 commit adbbd24
Show file tree
Hide file tree
Showing 25 changed files with 157 additions and 115 deletions.
16 changes: 8 additions & 8 deletions internal/cmd/all/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ func TestListAll(t *testing.T) {
expOut := `SERVERS
---
ID NAME STATUS IPV4 IPV6 PRIVATE NET DATACENTER AGE
123 my server running 192.0.2.1 - - hel1-dc2 3d
123 my server running 192.0.2.1 - - hel1-dc2 3d
IMAGES
---
ID TYPE NAME DESCRIPTION ARCHITECTURE IMAGE SIZE DISK SIZE CREATED DEPRECATED
1 backup test - arm - 20 GB Wed Aug 20 12:00:00 UTC 2036 -
1 backup test - arm - 20 GB Wed Aug 20 12:00:00 UTC 2036 -
PLACEMENT GROUPS
---
Expand All @@ -194,12 +194,12 @@ ID NAME SERVERS TYPE AGE
PRIMARY IPS
---
ID TYPE NAME IP ASSIGNEE DNS AUTO DELETE AGE
123 ipv4 test 127.0.0.1 - - no 2h
123 ipv4 test 127.0.0.1 - - no 2h
ISOS
---
ID NAME DESCRIPTION TYPE ARCHITECTURE
123 test - private arm
123 test - private arm
VOLUMES
---
Expand All @@ -209,12 +209,12 @@ ID NAME SIZE SERVER LOCATION AGE
LOAD BALANCER
---
ID NAME HEALTH IPV4 IPV6 TYPE LOCATION NETWORK ZONE AGE
123 foobar healthy 192.0.2.1 :: lb11 fsn1 eu-central 5h
123 foobar healthy 192.0.2.1 :: lb11 fsn1 eu-central 5h
FLOATING IPS
---
ID TYPE NAME DESCRIPTION IP HOME SERVER DNS AGE
123 ipv4 test - 127.0.0.1 fsn1 - - 1d
123 ipv4 test - 127.0.0.1 fsn1 - - 1d
NETWORKS
---
Expand All @@ -223,7 +223,7 @@ ID NAME IP RANGE SERVERS AGE
FIREWALLS
---
ID NAME RULES COUNT APPLIED TO COUNT
ID NAME RULES COUNT APPLIED TO COUNT
123 test 5 Rules 2 Servers | 0 Label Selectors
CERTIFICATES
Expand All @@ -234,7 +234,7 @@ ID NAME TYPE DOMAIN NAMES NOT VALID AFTER AGE
SSH KEYS
---
ID NAME FINGERPRINT AGE
123 test - 2h
123 test - 2h
`

Expand Down
16 changes: 12 additions & 4 deletions internal/cmd/base/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,12 @@ func TestList(t *testing.T) {
const resourceSchema = `[{"id": 123, "name": "test"}, {"id": 456, "name": "test2"}, {"id": 789, "name": "test3"}]`
testutil.TestCommand(t, fakeListCmd, map[string]testutil.TestCase{
"no flags": {
Args: []string{"list"},
ExpOut: "ID NAME\n123 test\n456 test2\n789 test3\n",
Args: []string{"list"},
ExpOut: `ID NAME
123 test
456 test2
789 test3
`,
},
"json": {
Args: []string{"list", "-o=json"},
Expand All @@ -108,8 +112,12 @@ func TestList(t *testing.T) {
ExpOutType: testutil.DataTypeYAML,
},
"quiet": {
Args: []string{"list", "--quiet"},
ExpOut: "ID NAME\n123 test\n456 test2\n789 test3\n",
Args: []string{"list", "--quiet"},
ExpOut: `ID NAME
123 test
456 test2
789 test3
`,
},
"json quiet": {
Args: []string{"list", "-o=json", "--quiet"},
Expand Down
54 changes: 27 additions & 27 deletions internal/cmd/config/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,52 +58,52 @@ func TestList(t *testing.T) {
{
name: "default",
args: []string{},
expOut: `KEY VALUE
context test_context
debug yes
deeply.nested.option bar
expOut: `KEY VALUE
context test_context
debug yes
deeply.nested.option bar
endpoint https://test-endpoint.com
poll-interval 1.234s
quiet yes
token [redacted]
poll-interval 1.234s
quiet yes
token [redacted]
`,
},
{
name: "only key",
args: []string{"-o=columns=key"},
expOut: `KEY
context
debug
expOut: `KEY
context
debug
deeply.nested.option
endpoint
poll-interval
quiet
token
endpoint
poll-interval
quiet
token
`,
},
{
name: "no header",
args: []string{"-o=noheader"},
expOut: `context test_context
debug yes
deeply.nested.option bar
expOut: `context test_context
debug yes
deeply.nested.option bar
endpoint https://test-endpoint.com
poll-interval 1.234s
quiet yes
token [redacted]
poll-interval 1.234s
quiet yes
token [redacted]
`,
},
{
name: "allow sensitive",
args: []string{"--allow-sensitive"},
expOut: `KEY VALUE
context test_context
debug yes
deeply.nested.option bar
expOut: `KEY VALUE
context test_context
debug yes
deeply.nested.option bar
endpoint https://test-endpoint.com
poll-interval 1.234s
quiet yes
token super secret token
poll-interval 1.234s
quiet yes
token super secret token
`,
},
{
Expand Down
12 changes: 6 additions & 6 deletions internal/cmd/context/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ token = "yet another super secret token"
name: "default",
args: []string{},
config: testConfig,
expOut: `ACTIVE NAME
* my-context
expOut: `ACTIVE NAME
* my-context
my-other-context
my-third-context
`,
Expand All @@ -59,7 +59,7 @@ token = "yet another super secret token"
name: "no header",
args: []string{"-o=noheader"},
config: testConfig,
expOut: `* my-context
expOut: `* my-context
my-other-context
my-third-context
`,
Expand All @@ -68,7 +68,7 @@ token = "yet another super secret token"
name: "no header only name",
args: []string{"-o=noheader", "-o=columns=name"},
config: testConfig,
expOut: `my-context
expOut: `my-context
my-other-context
my-third-context
`,
Expand All @@ -83,8 +83,8 @@ my-third-context
postRun: func() {
_ = os.Unsetenv("HCLOUD_CONTEXT")
},
expOut: `ACTIVE NAME
my-context
expOut: `ACTIVE NAME
my-context
* my-other-context
my-third-context
`,
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/datacenter/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestList(t *testing.T) {
out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID NAME DESCRIPTION LOCATION
4 fsn1-dc14 Falkenstein 1 virtual DC 14 fsn1
4 fsn1-dc14 Falkenstein 1 virtual DC 14 fsn1
`

require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/firewall/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestList(t *testing.T) {

out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID NAME RULES COUNT APPLIED TO COUNT
expOut := `ID NAME RULES COUNT APPLIED TO COUNT
123 test 5 Rules 2 Servers | 0 Label Selectors
`

Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/image/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestList(t *testing.T) {
out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID TYPE NAME DESCRIPTION ARCHITECTURE IMAGE SIZE DISK SIZE CREATED DEPRECATED
123 system test - x86 20.00 GB 15 GB Wed Aug 20 12:00:00 UTC 2036 -
123 system test - x86 20.00 GB 15 GB Wed Aug 20 12:00:00 UTC 2036 -
`

require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/iso/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestList(t *testing.T) {
out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID NAME DESCRIPTION TYPE ARCHITECTURE
123 test Test ISO public x86
123 test Test ISO public x86
`

require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/loadbalancertype/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestList(t *testing.T) {
out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID NAME DESCRIPTION MAX SERVICES MAX CONNECTIONS MAX TARGETS
123 test - 12 100 5
123 test - 12 100 5
`

require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/location/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestList(t *testing.T) {

out, errOut, err := fx.Run(cmd, []string{})

expOut := `ID NAME DESCRIPTION NETWORK ZONE COUNTRY CITY
expOut := `ID NAME DESCRIPTION NETWORK ZONE COUNTRY CITY
1 fsn1 - eu-central DE Falkenstein
`

Expand Down
44 changes: 31 additions & 13 deletions internal/cmd/output/output.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import (
"reflect"
"sort"
"strings"
"text/tabwriter"
"unicode"

"github.com/fatih/structs"
"github.com/jedib0t/go-pretty/v6/table"
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/cmpl"
Expand Down Expand Up @@ -149,8 +149,27 @@ func parseOutputFlags(in []string) Opts {

// NewTable creates a new Table.
func NewTable(out io.Writer) *Table {
style := table.Style{
Name: "minimal",
Box: table.StyleBoxLight,
Color: table.ColorOptionsDefault,
Format: table.FormatOptionsDefault,
HTML: table.DefaultHTMLOptions,
Options: table.OptionsNoBordersAndSeparators,
Size: table.SizeOptionsDefault,
Title: table.TitleOptionsDefault,
}
style.Box.MiddleVertical = " "
style.Box.PaddingLeft = ""
style.Box.PaddingRight = ""
style.Options.SeparateColumns = true

w := table.NewWriter()
w.SetStyle(style)

return &Table{
w: tabwriter.NewWriter(out, 0, 0, 3, ' ', 0),
out: out,
w: w,
columns: map[string]bool{},
fieldMapping: map[string]FieldFn{},
fieldAlias: map[string]string{},
Expand All @@ -160,14 +179,10 @@ func NewTable(out io.Writer) *Table {

type FieldFn func(obj interface{}) string

type writerFlusher interface {
io.Writer
Flush() error
}

// Table is a generic way to format object as a table.
type Table struct {
w writerFlusher
out io.Writer
w table.Writer
columns map[string]bool
fieldMapping map[string]FieldFn
fieldAlias map[string]string
Expand Down Expand Up @@ -248,18 +263,21 @@ func (o *Table) ValidateColumns(cols []string) error {

// WriteHeader writes the table header.
func (o *Table) WriteHeader(columns []string) {
var header []string
header := table.Row{}
for _, col := range columns {
if alias, ok := o.fieldAlias[col]; ok {
col = alias
}

header = append(header, strings.ReplaceAll(strings.ToUpper(col), "_", " "))
}
_, _ = fmt.Fprintln(o.w, strings.Join(header, "\t"))
o.w.AppendHeader(header)
}

func (o *Table) Flush() error {
return o.w.Flush()
_, _ = o.out.Write([]byte(o.w.Render()))
_, _ = o.out.Write([]byte("\n"))
return nil
}

// Write writes a table line.
Expand All @@ -270,7 +288,7 @@ func (o *Table) Write(columns []string, obj interface{}) {
dataL[strings.ToLower(key)] = value
}

var out []string
var out table.Row
for _, col := range columns {
colName := strings.ToLower(col)
if alias, ok := o.fieldAlias[colName]; ok {
Expand Down Expand Up @@ -299,7 +317,7 @@ func (o *Table) Write(columns []string, obj interface{}) {
out = append(out, fmt.Sprintf("%v", value))
}
}
_, _ = fmt.Fprintln(o.w, strings.Join(out, "\t"))
o.w.AppendRow(out)
}

func fieldName(name string) string {
Expand Down
Loading

0 comments on commit adbbd24

Please sign in to comment.