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

table: ignore non-printable characters when suppressing empty columns #327

Merged
merged 1 commit into from
Aug 21, 2024
Merged

table: ignore non-printable characters when suppressing empty columns #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion table/render_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package table
import (
"fmt"
"strings"
"unicode"

"github.com/jedib0t/go-pretty/v6/text"
)
Expand Down Expand Up @@ -255,7 +256,15 @@ func (t *Table) initForRenderSuppressColumns() {
shouldSuppressColumn := func(colIdx int) bool {
for _, row := range t.rows {
if colIdx < len(row) && row[colIdx] != "" {
return false
// Columns may contain non-printable characters. For example
// the text.Direction modifiers. These should not be considered
// when deciding to suppress a column.
for _, r := range row[colIdx] {
if unicode.IsPrint(r) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered only ignoring the specific characters used by text.LeftToRight (\u202a) and text.RightToLeft (\u202b). However, this approach would require the check to be updated if any new modifiers were added later, which could easily be overlooked.

I figure it’s unlikely someone would want to print a column containing only non-printable characters. More likely, this will confuse people—just as it did me when my seemingly empty columns weren't being suppressed. 😅

Copy link
Owner

Choose a reason for hiding this comment

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

No I like the final approach much better than checking for a couple of characters that we know of today.

return false
}
}
return true
}
}
return true
Expand Down
16 changes: 16 additions & 0 deletions table/render_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,22 @@ func TestTable_Render_SuppressEmptyColumns(t *testing.T) {
├─────┼────────────┼────────┼─────────────────────────────┤
│ │ │ 10000 │ │
└─────┴────────────┴────────┴─────────────────────────────┘`)

tw.SetStyle(Style{
Format: FormatOptions{
Direction: text.LeftToRight,
},
})
// Columns with non-printable characters should still be suppressed.
compareOutput(t, tw.Render(), strings.Join([]string{
"\u202a \u202a#\u202aFirst Name\u202aSalary ",
"\u202a \u202a1\u202aArya \u202a3000 ",
"\u202a \u202a20\u202aJon \u202a2000\u202aYou know nothing, Jon Snow!",
"\u202a\u202a300\u202aTyrion \u202a5000 ",
"\u202a \u202a11\u202aSansa \u202a6000 ",
"\u202a \u202a\u202a \u202a10000 ",
}, "\n"))

}

func TestTable_Render_TableWithinTable(t *testing.T) {
Expand Down