From df30706e7805a659dcbff3f7fd1a8728b22b402f Mon Sep 17 00:00:00 2001 From: Michael Fridman Date: Sat, 2 Nov 2024 19:26:50 -0400 Subject: [PATCH] Use charmbracelet/lipgloss for tables and remove tablewriter (#136) --- CHANGELOG.md | 8 +++- go.mod | 8 ++-- go.sum | 14 +++---- internal/app/table.go | 45 +++++++++++++++++++++ internal/app/table_summary.go | 53 +++++++++++------------- internal/app/table_tests.go | 76 +++++++++++++++++++++-------------- internal/app/table_writer.go | 32 --------------- internal/utils/utils.go | 9 +---- main.go | 2 +- 9 files changed, 132 insertions(+), 115 deletions(-) create mode 100644 internal/app/table.go delete mode 100644 internal/app/table_writer.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 03da73e..e61bdc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Add a `-follow-output` flag to allow writing go test output directly into a file. This will be useful (especially in CI jobs) for outputting overly verbose testing output into a file instead of - the standard stream. (#133) + the standard stream. (#134) | flag combination | `go test` output destination | | ------------------------ | ---------------------------- | @@ -18,6 +18,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | `-follow-output` | Write to file | | `-follow -follow-output` | Write to file | +- Use [charmbracelet/lipgloss](https://github.com/charmbracelet/lipgloss) for table rendering. + - This will allow for more control over the output and potentially more features in the future. + (#136) + - Minor changes to the output format are expected, but the overall content should remain the same. + If you have any feedback, please let me know. + ## [v0.15.0] - Add `-trimpath` flag, which removes the path prefix from package names in the output, simplifying diff --git a/go.mod b/go.mod index 680068f..240e08e 100644 --- a/go.mod +++ b/go.mod @@ -1,12 +1,13 @@ module github.com/mfridman/tparse -go 1.18 +go 1.21 + +toolchain go1.23.2 require ( github.com/charmbracelet/lipgloss v1.0.0 github.com/mfridman/buildversion v0.3.0 - github.com/muesli/termenv v0.15.2 - github.com/olekukonko/tablewriter v0.0.5 + github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a github.com/stretchr/testify v1.9.0 ) @@ -16,7 +17,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect - github.com/mattn/go-runewidth v0.0.16 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect golang.org/x/sys v0.26.0 // indirect diff --git a/go.sum b/go.sum index 76f6c3b..ccc5ad0 100644 --- a/go.sum +++ b/go.sum @@ -1,27 +1,25 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/aymanbagabas/go-udiff v0.2.0 h1:TK0fH4MteXUDspT88n8CKzvK0X9O2xu9yQjWpi6yML8= +github.com/aymanbagabas/go-udiff v0.2.0/go.mod h1:RE4Ex0qsGkTAJoQdQQCA0uG+nAzJO/pI/QwceO5fgrA= github.com/charmbracelet/lipgloss v1.0.0 h1:O7VkGDvqEdGi93X+DeqsQ7PKHDgtQfF8j8/O2qFMQNg= github.com/charmbracelet/lipgloss v1.0.0/go.mod h1:U5fy9Z+C38obMs+T+tJqst9VGzlOYGj4ri9reL3qUlo= github.com/charmbracelet/x/ansi v0.4.2 h1:0JM6Aj/g/KC154/gOP4vfxun0ff6itogDYk41kof+qk= github.com/charmbracelet/x/ansi v0.4.2/go.mod h1:dk73KoMTT5AX5BsX0KrqhsTqAnhZZoCBjs7dGWp4Ktw= +github.com/charmbracelet/x/exp/golden v0.0.0-20240806155701-69247e0abc2a h1:G99klV19u0QnhiizODirwVksQB91TJKV/UaTnACcG30= +github.com/charmbracelet/x/exp/golden v0.0.0-20240806155701-69247e0abc2a/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= -github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= -github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= github.com/mfridman/buildversion v0.3.0 h1:hehEX3IbBZJBqquXctUEOWJfIM46P0ku9naK9h1BGuY= github.com/mfridman/buildversion v0.3.0/go.mod h1:sfXvYxwfmLvkklTJLv9xJ0Wffw57z9ZFOK4KOGJYafU= -github.com/muesli/termenv v0.15.2 h1:GohcuySI0QmI3wN8Ok9PtKGkgkFIk7y6Vpb5PvrY+Wo= -github.com/muesli/termenv v0.15.2/go.mod h1:Epx+iuz8sNs7mNKhxzH4fWXGNpZwUaJKRS1noLXviQ8= -github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= -github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= +github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a h1:2MaM6YC3mGu54x+RKAA6JiFFHlHDY1UbkxqppT7wYOg= +github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a/go.mod h1:hxSnBBYLK21Vtq/PHd0S2FYCxBXzBua8ov5s1RobyRQ= 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/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= diff --git a/internal/app/table.go b/internal/app/table.go new file mode 100644 index 0000000..9f53362 --- /dev/null +++ b/internal/app/table.go @@ -0,0 +1,45 @@ +package app + +import ( + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" +) + +func newTable( + format OutputFormat, + override func(style lipgloss.Style, row, col int) lipgloss.Style, +) *table.Table { + tbl := table.New() + switch format { + case OutputFormatPlain: + tbl.Border(lipgloss.HiddenBorder()).BorderTop(false).BorderBottom(false) + case OutputFormatMarkdown: + tbl.Border(markdownBorder).BorderBottom(false).BorderTop(false) + case OutputFormatBasic: + tbl.Border(lipgloss.RoundedBorder()) + } + return tbl.StyleFunc(func(row, col int) lipgloss.Style { + // Default style, may be overridden. + style := lipgloss.NewStyle().PaddingLeft(1).PaddingRight(1).Align(lipgloss.Center) + if override != nil { + style = override(style, row, col) + } + return style + }) +} + +var markdownBorder = lipgloss.Border{ + Top: "-", + Bottom: "-", + Left: "|", + Right: "|", + TopLeft: "", // empty for markdown + TopRight: "", // empty for markdown + BottomLeft: "", // empty for markdown + BottomRight: "", // empty for markdown + MiddleLeft: "|", + MiddleRight: "|", + Middle: "|", + MiddleTop: "|", + MiddleBottom: "|", +} diff --git a/internal/app/table_summary.go b/internal/app/table_summary.go index cb5f485..b422ea4 100644 --- a/internal/app/table_summary.go +++ b/internal/app/table_summary.go @@ -7,7 +7,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" - "github.com/olekukonko/tablewriter" + "github.com/charmbracelet/lipgloss/table" "github.com/mfridman/tparse/internal/utils" "github.com/mfridman/tparse/parse" @@ -33,16 +33,16 @@ func (c *consoleWriter) summaryTable( options SummaryTableOptions, against *parse.GoTestSummary, ) { - var tableString strings.Builder - tbl := newTableWriter(&tableString, c.format) - tbl.SetColumnAlignment([]int{ - tablewriter.ALIGN_LEFT, - tablewriter.ALIGN_CENTER, - tablewriter.ALIGN_LEFT, - tablewriter.ALIGN_CENTER, - tablewriter.ALIGN_CENTER, - tablewriter.ALIGN_CENTER, - tablewriter.ALIGN_CENTER, + tbl := newTable(c.format, func(style lipgloss.Style, row, col int) lipgloss.Style { + switch row { + case table.HeaderRow: + default: + if col == 2 { + // Package name + style = style.Align(lipgloss.Left) + } + } + return style }) header := summaryRow{ status: "Status", @@ -53,7 +53,8 @@ func (c *consoleWriter) summaryTable( fail: "Fail", skip: "Skip", } - tbl.SetHeader(header.toRow()) + tbl.Headers(header.toRow()...) + data := table.NewStringData() // Capture as separate slices because notests are optional when passed tests are available. // The only exception is if passed=0 and notests=1, then we display them regardless. This @@ -80,7 +81,7 @@ func (c *consoleWriter) summaryTable( packageName: packageName, cover: "--", pass: "--", fail: "--", skip: "--", } - tbl.Append(row.toRow()) + data.Append(row.toRow()) continue } if pkg.HasFailedBuildOrSetup { @@ -90,7 +91,7 @@ func (c *consoleWriter) summaryTable( packageName: packageName + "\n[" + pkg.Summary.Output + "]", cover: "--", pass: "--", fail: "--", skip: "--", } - tbl.Append(row.toRow()) + data.Append(row.toRow()) continue } if pkg.NoTestFiles { @@ -187,32 +188,24 @@ func (c *consoleWriter) summaryTable( passed = append(passed, row) } - if tbl.NumLines() == 0 && len(passed) == 0 && len(notests) == 0 { + if data.Rows() == 0 && len(passed) == 0 && len(notests) == 0 { return } - - for _, p := range passed { - tbl.Append(p.toRow()) + for _, r := range passed { + data.Append(r.toRow()) } + // Only display the "no tests to run" cases if users want to see them when passed // tests are available. // An exception is made if there are no passed tests and only a single no test files // package. This is almost always because the user forgot to match one or more packages. if showNoTests || (len(passed) == 0 && len(notests) == 1) { - for _, p := range notests { - tbl.Append(p.toRow()) + for _, r := range notests { + data.Append(r.toRow()) } } - // The table gets written to a strings builder so we can further modify the output - // with lipgloss. - tbl.Render() - output := tableString.String() - if c.format == OutputFormatBasic { - output = lipgloss.NewStyle(). - Border(lipgloss.NormalBorder()). - Render(strings.TrimSuffix(tableString.String(), "\n")) - } - fmt.Fprintln(c.w, output) + + fmt.Fprintln(c.w, tbl.Data(data).Render()) } type summaryRow struct { diff --git a/internal/app/table_tests.go b/internal/app/table_tests.go index f2c8303..4eb7a0f 100644 --- a/internal/app/table_tests.go +++ b/internal/app/table_tests.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" "github.com/mfridman/tparse/internal/utils" "github.com/mfridman/tparse/parse" @@ -44,16 +45,25 @@ type packageTests struct { func (c *consoleWriter) testsTable(packages []*parse.Package, option TestTableOptions) { // Print passed tests, sorted by elapsed DESC. Grouped by alphabetically sorted packages. - var tableString strings.Builder - tbl := newTableWriter(&tableString, c.format) - + tbl := newTable(c.format, func(style lipgloss.Style, row, col int) lipgloss.Style { + switch row { + case table.HeaderRow: + default: + if col == 2 || col == 3 { + // Test name and package name + style = style.Align(lipgloss.Left) + } + } + return style + }) header := testRow{ status: "Status", elapsed: "Elapsed", testName: "Test", packageName: "Package", } - tbl.SetHeader(header.toRow()) + tbl.Headers(header.toRow()...) + data := table.NewStringData() names := make([]string, 0, len(packages)) for _, pkg := range packages { @@ -95,40 +105,40 @@ func (c *consoleWriter) testsTable(packages []*parse.Package, option TestTableOp testName: testName, packageName: packageName, } - tbl.Append(row.toRow()) + data.Append(row.toRow()) } if i != (len(packages) - 1) { - // TODO(mf): is it possible to add a custom separator with tablewriter instead of empty space? - tbl.Append(testRow{}.toRow()) + // Add a blank row between packages. + data.Append(testRow{}.toRow()) } } - if tbl.NumLines() > 0 { - // The table gets written to a strings builder so we can further modify the output - // with lipgloss. - tbl.Render() - output := tableString.String() - if c.format == OutputFormatBasic { - output = lipgloss.NewStyle(). - Border(lipgloss.NormalBorder()). - Render(strings.TrimSuffix(output, "\n")) - } - fmt.Fprintln(c.w, output) + if data.Rows() > 0 { + fmt.Fprintln(c.w, tbl.Data(data).Render()) } } func (c *consoleWriter) testsTableMarkdown(packages []*parse.Package, option TestTableOptions) { for _, pkg := range packages { // Print passed tests, sorted by elapsed DESC. Grouped by alphabetically sorted packages. - var tableString strings.Builder - tbl := newTableWriter(&tableString, c.format) - + tbl := newTable(c.format, func(style lipgloss.Style, row, col int) lipgloss.Style { + switch row { + case table.HeaderRow: + default: + if col == 2 { + // Test name + style = style.Align(lipgloss.Left) + } + } + return style + }) header := []string{ "Status", "Elapsed", "Test", } - tbl.SetHeader(header) + tbl.Headers(header...) + data := table.NewStringData() // Discard packages where we cannot generate a sensible test summary. if pkg.NoTestFiles || pkg.NoTests || pkg.HasPanic { @@ -154,30 +164,34 @@ func (c *consoleWriter) testsTableMarkdown(packages []*parse.Package, option Tes case parse.ActionFail: status = c.red(status) } - tbl.Append([]string{ + data.Append([]string{ status, strconv.FormatFloat(t.Elapsed(), 'f', 2, 64), testName, }) } - if tbl.NumLines() > 0 { - tbl.Render() - + if data.Rows() > 0 { fmt.Fprintf(c.w, "## 📦 Package **`%s`**\n", pkg.Summary.Package) fmt.Fprintln(c.w) - fmt.Fprintf(c.w, - "**%d passed** tests (out of %d) | **%d skipped** tests (out of %d)\n", - len(pkgTests.passed), + + msg := fmt.Sprintf("Tests: ✓ %d passed | %d skipped\n", pkgTests.passedCount, - len(pkgTests.skipped), pkgTests.skippedCount, ) + if option.Slow > 0 && option.Slow < pkgTests.passedCount { + msg += fmt.Sprintf("↓ Slowest %d passed tests shown (of %d)\n", + option.Slow, + pkgTests.passedCount, + ) + } + fmt.Fprint(c.w, msg) + fmt.Fprintln(c.w) fmt.Fprintln(c.w, "
") fmt.Fprintln(c.w) fmt.Fprintln(c.w, "Click for test summary") fmt.Fprintln(c.w) - fmt.Fprintln(c.w, tableString.String()) + fmt.Fprintln(c.w, tbl.Data(data).Render()) fmt.Fprintln(c.w, "
") fmt.Fprintln(c.w) } diff --git a/internal/app/table_writer.go b/internal/app/table_writer.go deleted file mode 100644 index 745f0e1..0000000 --- a/internal/app/table_writer.go +++ /dev/null @@ -1,32 +0,0 @@ -package app - -import ( - "io" - - "github.com/olekukonko/tablewriter" -) - -func newTableWriter(w io.Writer, format OutputFormat) *tablewriter.Table { - tbl := tablewriter.NewWriter(w) - tbl.SetAutoWrapText(false) - switch format { - case OutputFormatPlain: - tbl.SetBorder(false) - tbl.SetRowSeparator("") - tbl.SetColumnSeparator("") - tbl.SetHeaderLine(false) - case OutputFormatMarkdown: - tbl.SetBorders(tablewriter.Border{Left: true, Top: false, Right: true, Bottom: false}) - tbl.SetCenterSeparator("|") - case OutputFormatBasic: - // TODO(mf): sigh, we're going to hack around this limitation by wrapping the table - // with a lipgloss border around the un-borded tablewriter output. Wish upstream would - // consider this PR: - // https://github.com/olekukonko/tablewriter/pull/115 - tbl.SetBorder(false) - tbl.SetRowSeparator("─") - tbl.SetColumnSeparator("│") - tbl.SetCenterSeparator("┼") - } - return tbl -} diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 878e1d7..1210962 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -27,7 +27,7 @@ func FindLongestCommonPrefix(paths []string) string { // Find the common prefix between the first and last sorted paths. commonPrefixLength := 0 - minLength := minimum(len(first), len(last)) + minLength := min(len(first), len(last)) for commonPrefixLength < minLength && first[commonPrefixLength] == last[commonPrefixLength] { commonPrefixLength++ } @@ -40,13 +40,6 @@ func FindLongestCommonPrefix(paths []string) string { return "" } -func minimum(a, b int) int { - if a < b { - return a - } - return b -} - // DiscardCloser is an io.Writer that implements io.Closer by doing nothing. // // https://github.com/golang/go/issues/22823 diff --git a/main.go b/main.go index 99298e4..cca00ce 100644 --- a/main.go +++ b/main.go @@ -135,7 +135,7 @@ func main() { return } *followPtr = true - case *followPtr: + case *followPtr, *followVerbosePtr: followOutput = os.Stdout default: // If no follow flags are set, we should not write to followOutput.