Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: used go-pretty to improve table formatting on screen width #414

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

feat: used go-pretty to improve table formatting on screen width #414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1ff345
feat: used go-pretty to improved table formatting with screen width
Aerex Nov 11, 2024
a77672d
chore: added more safety check on conditions when checking wide columns
Aerex Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 146 additions & 37 deletions bluemix/terminal/table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@ package terminal
import (
"encoding/csv"
"fmt"
"io"
"os"
"strconv"
"strings"

"golang.org/x/term"

. "github.com/IBM-Cloud/ibm-cloud-cli-sdk/i18n"

"io"

"github.com/jedib0t/go-pretty/v6/table"
"github.com/jedib0t/go-pretty/v6/text"
"github.com/mattn/go-runewidth"
)

Expand All @@ -24,11 +32,10 @@ type Table interface {
}

type PrintableTable struct {
writer io.Writer
headers []string
headerPrinted bool
maxSizes []int
rows [][]string //each row is single line
writer io.Writer
headers []string
maxSizes []int
rows [][]string //each row is single line
}

func NewTable(w io.Writer, headers []string) Table {
Expand Down Expand Up @@ -69,58 +76,160 @@ func (t *PrintableTable) Add(row ...string) {
}
}

func isWideColumn(col string) bool {
// list of common columns that are usually wide
largeColumnTypes := []string{T("ID"), T("Description")}

for _, largeColn := range largeColumnTypes {
if strings.Contains(largeColn, col) {
return true
}
}

return false

}

func terminalWidth() int {
var err error
terminalWidth, _, err := term.GetSize(int(os.Stdin.Fd()))

if err != nil {
// Assume normal 80 char width line
terminalWidth = 80
}

testTerminalWidth, envSet := os.LookupEnv("TEST_TERMINAL_WIDTH")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callout: Added test environment variable to test variable terminal width in unit tests

if envSet {
envWidth, err := strconv.Atoi(testTerminalWidth)
if err == nil {
terminalWidth = envWidth
}
}
return terminalWidth
}

func (t *PrintableTable) Print() {
for _, row := range append(t.rows, t.headers) {
t.calculateMaxSize(row)
}

if t.headerPrinted == false {
t.printHeader()
t.headerPrinted = true
tbl := table.NewWriter()
tbl.SetOutputMirror(t.writer)
tbl.SuppressTrailingSpaces()
// remove padding from the left to keep the table aligned to the left
tbl.Style().Box.PaddingLeft = ""
tbl.Style().Box.PaddingRight = strings.Repeat(" ", minSpace)
// remove all border and column and row separators
tbl.Style().Options.DrawBorder = false
tbl.Style().Options.SeparateColumns = false
tbl.Style().Options.SeparateFooter = false
tbl.Style().Options.SeparateHeader = false
tbl.Style().Options.SeparateRows = false
tbl.Style().Format.Header = text.FormatDefault

headerRow, rows := t.createPrettyRowsAndHeaders()
columnConfig := t.createColumnConfigs()

tbl.SetColumnConfigs(columnConfig)
tbl.AppendHeader(headerRow)
tbl.AppendRows(rows)
tbl.Render()
}

func (t *PrintableTable) createColumnConfigs() []table.ColumnConfig {
Copy link
Collaborator

@steveclay steveclay Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add some more testing to explicitly try out some odd column headings to ensure there's no breaking conditions. I don't have any in mind, but would like to see if we can test some boundary conditions here.

For example, these paths are not unit tested:
image

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more unit tests for various scenarios.

// there must be at row in order to configure column
if len(t.rows) == 0 {
return []table.ColumnConfig{}
}

for _, line := range t.rows {
t.printRow(line)
colCount := len(t.rows[0])
var (
widestColIndicies []int
terminalWidth = terminalWidth()
// total amount padding space that a row will take up
totalPaddingSpace = (colCount - 1) * minSpace
remainingSpace = max(0, terminalWidth-totalPaddingSpace)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callout: It is unlikely for the remainingSpace to be negative but if that does happen we will set it to 0.

// the estimated max column width by dividing the remaining space evenly across the columns
maxColWidth = remainingSpace / colCount
)
columnConfig := make([]table.ColumnConfig, colCount)

for i := range columnConfig {
columnConfig[i] = table.ColumnConfig{
AlignHeader: text.AlignLeft,
Align: text.AlignLeft,
WidthMax: maxColWidth,
Number: i + 1,
}

// assuming the table has headers: store columns with wide content where the max width may need to be adjusted
// using the remaining space
if t.maxSizes[i] > maxColWidth && (i < len(t.headers) && isWideColumn(t.headers[i])) {
widestColIndicies = append(widestColIndicies, i)
} else if t.maxSizes[i] < maxColWidth {
// use the max column width instead of the estimated max column width
// if it is shorter
columnConfig[i].WidthMax = t.maxSizes[i]
remainingSpace -= t.maxSizes[i]
} else {
remainingSpace -= maxColWidth
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if there can be a scenario that remainingSpace becomes 0 or less.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I don't think there is a valid scenario since that would mean the user would need to have a lot of columns. For instance, for terminal width of 80, the user would need to create more than 27 columns (27 * 3 > 80) for there to be negative remainingSpace.

}
}

t.rows = [][]string{}
}
// if only one wide column use the remaining space as the max column width
if len(widestColIndicies) == 1 {
idx := widestColIndicies[0]
columnConfig[idx].WidthMax = remainingSpace
}

func (t *PrintableTable) calculateMaxSize(row []string) {
for index, value := range row {
cellLength := runewidth.StringWidth(Decolorize(value))
if t.maxSizes[index] < cellLength {
t.maxSizes[index] = cellLength
// if more than one wide column, spread the remaining space between the columns
if len(widestColIndicies) > 1 {
remainingSpace /= len(widestColIndicies)
for _, columnCfgIdx := range widestColIndicies {
columnConfig[columnCfgIdx].WidthMax = remainingSpace
}

origRemainingSpace := remainingSpace
moreRemainingSpace := origRemainingSpace % len(widestColIndicies)
if moreRemainingSpace != 0 {
columnConfig[0].WidthMax += moreRemainingSpace
}
}

return columnConfig
}

func (t *PrintableTable) printHeader() {
output := ""
for col, value := range t.headers {
output = output + t.cellValue(col, HeaderColor(value))
func (t *PrintableTable) createPrettyRowsAndHeaders() (headerRow table.Row, rows []table.Row) {
for _, header := range t.headers {
headerRow = append(headerRow, header)
}
fmt.Fprintln(t.writer, output)
}

func (t *PrintableTable) printRow(row []string) {
output := ""
for columnIndex, value := range row {
if columnIndex == 0 {
value = TableContentHeaderColor(value)
for i := range t.rows {
var row, emptyRow table.Row
for j, cell := range t.rows[i] {
if j == 0 {
cell = TableContentHeaderColor(cell)
}
row = append(row, cell)
emptyRow = append(emptyRow, "")
}

output = output + t.cellValue(columnIndex, value)
if i == 0 && len(t.headers) == 0 {
rows = append(rows, emptyRow)
}
rows = append(rows, row)
}
fmt.Fprintln(t.writer, output)

return
}

func (t *PrintableTable) cellValue(col int, value string) string {
padding := ""
if col < len(t.maxSizes)-1 {
padding = strings.Repeat(" ", t.maxSizes[col]-runewidth.StringWidth(Decolorize(value))+minSpace)
func (t *PrintableTable) calculateMaxSize(row []string) {
for index, value := range row {
cellLength := runewidth.StringWidth(Decolorize(value))
if t.maxSizes[index] < cellLength {
t.maxSizes[index] = cellLength
}
}
return fmt.Sprintf("%s%s", value, padding)
}

// Prints out a nicely/human formatted Json string instead of a table structure
Expand Down
47 changes: 45 additions & 2 deletions bluemix/terminal/table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package terminal_test

import (
"bytes"
"os"
"strings"
"testing"

Expand Down Expand Up @@ -38,7 +39,7 @@ func TestEmptyHeaderTable(t *testing.T) {
testTable.Add("row1", "row2")
testTable.Print()
assert.Contains(t, buf.String(), "row1")
assert.Equal(t, " \nrow1 row2\n", buf.String())
assert.Equal(t, "\nrow1 row2\n", buf.String())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callout: Removing the tab space does not change the output visually.

image

}

func TestEmptyHeaderTableJson(t *testing.T) {
Expand Down Expand Up @@ -79,7 +80,49 @@ func TestNotEnoughRowEntires(t *testing.T) {
testTable.Add("", "row2")
testTable.Print()
assert.Contains(t, buf.String(), "row1")
assert.Equal(t, "col1 col2\nrow1 \n row2\n", buf.String())
assert.Equal(t, "col1 col2\nrow1\n row2\n", buf.String())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callout: Same as above

image

}

func TestMoreColThanTerminalWidth(t *testing.T) {
os.Setenv("TEST_TERMINAL_WIDTH", "1")
buf := bytes.Buffer{}
testTable := NewTable(&buf, []string{"col1"})
testTable.Add("row1", "row2")
testTable.Print()
assert.Contains(t, buf.String(), "row1")
assert.Equal(t, "col1\nrow1 row2\n", buf.String())
os.Unsetenv("TEST_TERMINAL_WIDTH")
}

func TestWideHeaderNames(t *testing.T) {
buf := bytes.Buffer{}
testTable := NewTable(&buf, []string{"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt u", "NAME"})
testTable.Add("col1", "col2")
testTable.Print()
assert.Contains(t, buf.String(), "Lorem ipsum dolor sit amet, consectetu")
assert.Equal(t, "Lorem ipsum dolor sit amet, consectetu NAME\nr adipiscing elit, sed do eiusmod temp\nor incididunt u\ncol1 col2\n", buf.String())
}

func TestWidestColumn(t *testing.T) {
buf := bytes.Buffer{}
id := "ABCDEFG-9b8babbd-f2ed-4371-b817-a839e4130332"
testTable := NewTable(&buf, []string{"ID", "Name"})
testTable.Add(id, "row2")
testTable.Print()
assert.Contains(t, buf.String(), id)
assert.Equal(t, buf.String(), "ID Name\nABCDEFG-9b8babbd-f2ed-4371-b817-a839e4130332 row2\n")
}

func TestMultiWideColumns(t *testing.T) {
buf := bytes.Buffer{}
id := "ABCDEFG-9b8babbd-f2ed-4371-b817-a839e4130332"
desc := "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut"
testTable := NewTable(&buf, []string{"ID", "Description", "Name"})
testTable.Add(id, desc, "col3")
testTable.Print()
assert.Contains(t, buf.String(), "ABCDEFG-9b8babbd-f2ed-4371-b817-a839")
assert.Contains(t, buf.String(), "e4130332")
assert.Equal(t, buf.String(), "ID Description Name\nABCDEFG-9b8babbd-f2ed-4371-b817-a839 Lorem ipsum dolor sit amet, consect col3\ne4130332 etur adipiscing elit, sed do eiusmo\n d tempor incididunt ut\n")
}

func TestNotEnoughRowEntiresJson(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ require (
github.com/fatih/color v1.7.1-0.20180516100307-2d684516a886
github.com/fatih/structs v1.0.1-0.20171020064819-f5faa72e7309
github.com/gofrs/flock v0.8.1
github.com/jedib0t/go-pretty/v6 v6.6.1
github.com/mattn/go-colorable v0.0.0-20160210001857-9fdad7c47650
github.com/mattn/go-runewidth v0.0.0-20151118072159-d96d1bd051f2
github.com/mattn/go-runewidth v0.0.15
github.com/nicksnyder/go-i18n/v2 v2.2.0
github.com/onsi/gomega v1.33.0
github.com/spf13/cobra v1.6.1
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.2.2
github.com/stretchr/testify v1.8.4
golang.org/x/crypto v0.21.0
golang.org/x/text v0.14.0
gopkg.in/cheggaaa/pb.v1 v1.0.15
Expand All @@ -26,6 +27,7 @@ require (
github.com/inconshreveable/mousetrap v1.0.1 // indirect
github.com/mattn/go-isatty v0.0.5-0.20180830101745-3fb116b82035 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rivo/uniseg v0.2.0 // indirect
golang.org/x/net v0.23.0 // indirect
golang.org/x/sys v0.18.0 // indirect
golang.org/x/term v0.18.0 // indirect
Expand Down
16 changes: 10 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@ github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE8dj7HMvPfh66eeA2JYW7eFpSE=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/google/pprof v0.0.0-20211214055906-6f57359322fd h1:1FjCyPC+syAzJ5/2S8fqdZK1R22vvA0J7JZKcuOIQ7Y=
github.com/google/pprof v0.0.0-20211214055906-6f57359322fd/go.mod h1:KgnwoLYCZ8IQu3XUZ8Nc/bM9CCZFOyjUNOSygVozoDg=
github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc=
github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/jedib0t/go-pretty/v6 v6.6.1 h1:iJ65Xjb680rHcikRj6DSIbzCex2huitmc7bDtxYVWyc=
github.com/jedib0t/go-pretty/v6 v6.6.1/go.mod h1:zbn98qrYlh95FIhwwsbIip0LYpwSG8SUOScs+v9/t0E=
github.com/mattn/go-colorable v0.0.0-20160210001857-9fdad7c47650 h1:pwtfAm8Do0gwFJ2J+iUrEVR9qI03BpDSuDQCIqbd6iY=
github.com/mattn/go-colorable v0.0.0-20160210001857-9fdad7c47650/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-isatty v0.0.5-0.20180830101745-3fb116b82035 h1:USWjF42jDCSEeikX/G1g40ZWnsPXN5WkZ4jMHZWyBK4=
github.com/mattn/go-isatty v0.0.5-0.20180830101745-3fb116b82035/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
github.com/mattn/go-runewidth v0.0.0-20151118072159-d96d1bd051f2 h1:K4BQSf+ZGZ8QlDL8RsUD1DES25Lgetj1JJGJz1G7Bno=
github.com/mattn/go-runewidth v0.0.0-20151118072159-d96d1bd051f2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U=
github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
github.com/nicksnyder/go-i18n/v2 v2.2.0 h1:MNXbyPvd141JJqlU6gJKrczThxJy+kdCNivxZpBQFkw=
github.com/nicksnyder/go-i18n/v2 v2.2.0/go.mod h1:4OtLfzqyAxsscyCb//3gfqSvBc81gImX91LrZzczN1o=
github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8=
Expand All @@ -34,13 +36,15 @@ github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE=
github.com/onsi/gomega v1.33.0/go.mod h1:+925n5YtiFsLzzafLUHzVMBpvvRAzrydIBiSIxjX3wY=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA=
github.com/spf13/cobra v1.6.1/go.mod h1:IOw/AERYS7UzyrGinqmz6HLUo219MORXGxhbaJUqzrY=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
Expand Down