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.

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

Conversation

iamjoemccormick
Copy link
Contributor

Proposed Changes

This change ensures that non-printable characters (e.g., text direction markers) are ignored when determining whether to suppress empty columns. This is required to use FormatOptions.Direction in conjunction with SuppressEmptyColumns().

This change ensures that non-printable characters (e.g., text direction markers) are ignored when determining whether to suppress empty columns. This is required to use FormatOptions.Direction in conjunction with SuppressEmptyColumns().
Copy link

@coveralls
Copy link

Pull Request Test Coverage Report for Build 10482223208

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 8953482681: 0.0%
Covered Lines: 3561
Relevant Lines: 3561

💛 - Coveralls

@jedib0t jedib0t merged commit 6ea4b17 into jedib0t:main Aug 21, 2024
3 checks passed
@jedib0t
Copy link
Owner

jedib0t commented Aug 21, 2024

Thanks for your contribution. Will cut a release later this week with this change incorporated.

// 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants