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 1 commit
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
174 changes: 137 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,17 @@ package terminal
import (
"encoding/csv"
"fmt"
"io"
"os"
"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 +31,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 +75,152 @@ 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
}
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{}
}

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 = terminalWidth - totalPaddingSpace
// the estimated max column width by dividing the remaining space evenly across the columns
maxColWidth = (terminalWidth - totalPaddingSpace) / colCount
Aerex marked this conversation as resolved.
Show resolved Hide resolved
)
columnConfig := make([]table.ColumnConfig, len(t.maxSizes))

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 && (len(t.headers) > 0 && isWideColumn(t.headers[i])) {
Aerex marked this conversation as resolved.
Show resolved Hide resolved
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.

}
}

for _, line := range t.rows {
t.printRow(line)
// 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
}

t.rows = [][]string{}
}
// 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
}

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
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 i, cell := range t.rows[i] {
Aerex marked this conversation as resolved.
Show resolved Hide resolved
if i == 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
4 changes: 2 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 @@ -38,7 +38,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 +79,7 @@ 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 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