Remove tablewriter dependency #1351
Conversation
…e rendering function
formatter/table.go
Outdated
| if w := utf8.RuneCountInString(col); w > colWidths[i] { | ||
| colWidths[i] = w | ||
| } |
There was a problem hiding this comment.
So it's about this, no?
| if w := utf8.RuneCountInString(col); w > colWidths[i] { | |
| colWidths[i] = w | |
| } | |
| colWidths[i] = max(colWidths[i], utf8.RuneCountInString(col)) |
There was a problem hiding this comment.
But I'm pretty sure slices.MaxFunc can help also, if you want to try
There was a problem hiding this comment.
Thanks for review.
The 2 for loops can be replaced with the following using MaxFunc.
for i := range colwidths {
colwidths[i] = utf8.runecountinstring(slices.maxfunc(rows, func(row1, row2 []string) int {
return utf8.runecountinstring(row1[i]) - utf8.runecountinstring(row2[i])
})[i])
}
What do you think?
There was a problem hiding this comment.
Let's wait for you to try text/tabwriter as @denisvmedia suggested.
|
Thank you for your PR. Can we have unit tests for that? Also, did you consider using |
I relied on existing tests as the code was refactoring without adding new functionality. I can add unit tests for the |
Nice shout. I'll look into it. Thanks. |
formatter/table.go
Outdated
There was a problem hiding this comment.
I propose to rename this file to table_utils.go (or something like that)
Why? Because this name allows to still easily distinguish formatters implementation files (e.g., checkstyle.go, default.go, stylish.go ... ) from auxiliary files in the formatter packet directory. Keeping table.go might make think that there is a table formatter.
There was a problem hiding this comment.
While I agree with using table_utils.go, I propose an alternative: placing the formatTable function at the end of either the friendly.go or stylish.go file. There's no need to create too many files — it's not the Go way.
formatter/friendly.go
Outdated
| var buf strings.Builder | ||
| tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) | ||
| for _, row := range rows { | ||
| fmt.Fprintf(tw, "\t") | ||
| for _, col := range row { | ||
| fmt.Fprintf(tw, "%s\t", col) | ||
| } | ||
| tw.Write([]byte("\n")) | ||
| } |
There was a problem hiding this comment.
It's a matter of style, but we can use the same bytes.Buffer and don't use fmt.Fprintf:
| var buf strings.Builder | |
| tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) | |
| for _, row := range rows { | |
| fmt.Fprintf(tw, "\t") | |
| for _, col := range row { | |
| fmt.Fprintf(tw, "%s\t", col) | |
| } | |
| tw.Write([]byte("\n")) | |
| } | |
| buf := new(bytes.Buffer) | |
| tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) | |
| for _, row := range rows { | |
| tw.Write([]byte{'\t'}) | |
| for _, col := range row { | |
| tw.Write(append([]byte(col), '\t')) | |
| } | |
| tw.Write([]byte{'\n'}) | |
| } |
There was a problem hiding this comment.
@alexandear idiomatically bytes.Buffer needs no initialization, so it should be var buf bytes.Buffer.
There was a problem hiding this comment.
You're right. I left buf := new(bytes.Buffer) because it was in the original version 🙂
2c75826 to
b491505
Compare
| var buf bytes.Buffer | ||
| tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) | ||
| for _, row := range rows { | ||
| tw.Write([]byte{'\t'}) |
There was a problem hiding this comment.
There was a problem hiding this comment.
To build this indentation:
There was a problem hiding this comment.
As long as we are fine that result are left aligned like this
https://go.dev/play/p/7UT-dXUTa5K
Note: I used debug mode and dash padding to debug how data are displayed. We won't use this. It's for helping understanding.
So now, I'm fine if everyone agree it's fine that words are left aligned
I have no idea how code was formatted before with the tabwritter lib we had.
From what I saw, there is nothing in std tawritter lib to align the first column to right, and second to left.
|
Thanks everybody @volfgox @denisvmedia @alexandear @ccoVeille |
This pull request refactors the table formatting logic by replacing the dependency on
tablewriterwith a customformatTablefunction. It also removes unused imports and updates theFriendlyandStylishformatters to use the new table formatting implementation. Below are the most important changes:Table Formatting Refactor:
formatTablefunction informatter/table.goto handle table formatting without external dependencies. This function aligns columns and ensures consistent formatting.FriendlyandStylishformatters to use the newformatTablefunction instead of the removedtablewriterlogic. [1] [2] [3]Dependency Cleanup:
tablewriterlibrary from thego.modfile. [1]Code Simplification:
tablemethod from theFriendlyformatter as it is no longer needed due to the introduction offormatTable.Friendlyby removing unnecessary pointer references where they are not required. [1] [2]Closes #1345