feat: used go-pretty to improve table formatting on screen width #414
Conversation
| @@ -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()) | |||
There was a problem hiding this comment.
Callout: Removing the tab space does not change the output visually.
| @@ -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()) | |||
There was a problem hiding this comment.
Callout: Same as above
Aerex
force-pushed
the
feat/better-formating-table-go-pretty
branch
from
0fa5825 to
b1ff345
Compare
|
Ready for review @steveclay |
steveclay
changed the title
| columnConfig[i].WidthMax = t.maxSizes[i] | ||
| remainingSpace -= t.maxSizes[i] | ||
| } else { | ||
| remainingSpace -= maxColWidth |
There was a problem hiding this comment.
Wondering if there can be a scenario that remainingSpace becomes 0 or less.
There was a problem hiding this comment.
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.
| tbl.Render() | ||
| } | ||
|
|
||
| func (t *PrintableTable) createColumnConfigs() []table.ColumnConfig { |
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
Added more unit tests for various scenarios.
| terminalWidth = 80 | ||
| } | ||
|
|
||
| testTerminalWidth, envSet := os.LookupEnv("TEST_TERMINAL_WIDTH") |
There was a problem hiding this comment.
Callout: Added test environment variable to test variable terminal width in unit tests
| terminalWidth = terminalWidth() | ||
| // total amount padding space that a row will take up | ||
| totalPaddingSpace = (colCount - 1) * minSpace | ||
| remainingSpace = max(0, terminalWidth-totalPaddingSpace) |
There was a problem hiding this comment.
Callout: It is unlikely for the remainingSpace to be negative but if that does happen we will set it to 0.
Context
The purpose of this PR is to improve the table formatting on the columns to wrap when the text when it reaches an estimated max column width. The max col width is calculated by excluding the padding and applying the remaining space evenly among the columns. Any columns that are expected to be wider than the others (eg. ID and Description) will given more space than the others
The PR uses the go-pretty library to manage the column text wrapping, and max column width and coloring.