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

feat: add suppressTrailingSpaces to remove all trailing spaces in final column #287

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

shreddedbacon
Copy link
Contributor

@shreddedbacon shreddedbacon commented Jan 3, 2024

Proposed Changes

  • Add new SuppressTrailingSpaces that removes all the trailing spaces in the final column of a row. This is useful when using the OptionsNoBordersAndSeparators options to reduce the number of spaces at the end of the column.

@shreddedbacon
Copy link
Contributor Author

I hope this is OK for me to submit. It isn't super important that it gets merged, I can deal with the trailing spaces. But it would be nice to be able to disable them as required for cleaner output.

@coveralls
Copy link

coveralls commented Jan 3, 2024

Pull Request Test Coverage Report for Build 7404733626

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6674369350: 0.0%
Covered Lines: 3368
Relevant Lines: 3368

💛 - Coveralls

@jedib0t
Copy link
Owner

jedib0t commented Jan 3, 2024

Can you show what happens when OptionsNoBordersAndSeparators is false and SuppressLastColumnSpaces is true?

table/writer.go Outdated Show resolved Hide resolved
@shreddedbacon shreddedbacon changed the title feat: add suppressLastColumnSpaces to remove all trailing spaces in final column feat: add suppressTrailingSpaces to remove all trailing spaces in final column Jan 3, 2024
@shreddedbacon
Copy link
Contributor Author

Can you show what happens when OptionsNoBordersAndSeparators is false and SuppressLastColumnSpaces is true?

I added coverage in the tests to show how this looks when borders are turned on (they look bad as expected)

@jedib0t
Copy link
Owner

jedib0t commented Jan 4, 2024

Can you show what happens when OptionsNoBordersAndSeparators is false and SuppressLastColumnSpaces is true?

I added coverage in the tests to show how this looks when borders are turned on (they look bad as expected)

Yeah this code cannot be merged in this state. How about just adding a block of code after https://github.com/jedib0t/go-pretty/blob/main/table/table.go#L681? If the suppressTrailingSpaces is set to true, split the outStr into lines, and then trim each line before returning that?

@shreddedbacon shreddedbacon force-pushed the suppress-last-column-spaces branch 2 times, most recently from 2e85ce9 to 2b3820c Compare January 4, 2024 01:53
@shreddedbacon
Copy link
Contributor Author

shreddedbacon commented Jan 4, 2024

Can you show what happens when OptionsNoBordersAndSeparators is false and SuppressLastColumnSpaces is true?

I added coverage in the tests to show how this looks when borders are turned on (they look bad as expected)

Yeah this code cannot be merged in this state. How about just adding a block of code after main/table/table.go#L681? If the suppressTrailingSpaces is set to true, split the outStr into lines, and then trim each line before returning that?

Ahh, that works even better, thanks for the tip. And it retains the table shape when borders are present which is better than what I originally proposed.

The sonarcloud code analysis complains about duplicated code in the render_test file though, most of it seems unrelated to my PR specifically here, happy to address if required though?
I reduced the test cases down to eliminate what I think it was complaining about

Copy link

sonarcloud bot commented Jan 4, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jedib0t jedib0t merged commit 8dba2e0 into jedib0t:main Jan 4, 2024
3 checks passed
@shreddedbacon shreddedbacon deleted the suppress-last-column-spaces branch January 4, 2024 21:25
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.

None yet

3 participants