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

Add WithHeaderSeparatorRow #40

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

Bios-Marcel
Copy link
Contributor

@Bios-Marcel Bios-Marcel commented Mar 29, 2024

This function will print a separator line between the header and the data rows. It will use the same formatter as the header. The width of each separator cell will be equal to the width of the header cell in the same column. It supports runes, meaning dependning on the rune width, it might not be perfect.

Oh and, I use gofumpt, it reformatted some comments, hope that's okay.

This function will print a separator line between the header and the
data rows. It will use the same formatter as the header. The width of
each separator cell will be equal to the width of the header cell in the
same column. It supports runes, meaning dependning on the rune width, it
might not be perfect.
Copy link
Owner

@rodaine rodaine left a comment

Choose a reason for hiding this comment

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

Thanks for the patch! A couple of comments when the seperator widths cannot cleanly divide the header names. Also, this only puts the separator directly below the header, instead of continuing the line across the whole row. Is that the intent?

table.go Outdated
Comment on lines 253 to 254
headerRuneCount := t.Width(headerName)
separatorLen := headerRuneCount / separatorRuneWidth
Copy link
Owner

Choose a reason for hiding this comment

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

If headerRuneCount and separatorRuneWidth aren't cleanly divisible then the length here will be truncated, resulting in too short of a separator line.

Copy link
Contributor Author

@Bios-Marcel Bios-Marcel Apr 2, 2024

Choose a reason for hiding this comment

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

It's not a bug, it's a feature 😄

The intent is, that in a case where you have a separator rune that's of cell width 2 and a header of length 5, you get a separator of length 2 (cell width 4).

In this case I wanted the separator to be one char shorter, instead of too long. I thought better too short than too long 🤔 Either way, this is more of an edge case, I doubt many people are gonna use chars other than _-..

table.go Outdated Show resolved Hide resolved
@Bios-Marcel
Copy link
Contributor Author

Bios-Marcel commented Apr 2, 2024

Thanks for the patch! A couple of comments when the seperator widths cannot cleanly divide the header names. Also, this only puts the separator directly below the header, instead of continuing the line across the whole row. Is that the intent?

Yes, that was the intend. I was going for this style.

I wanted to replicate this:

wezterm-gui_5NyrUOgf3u

We could however make this configurable.

Since this feature didn't exist before though, I wanted to refrain from adding more stuff nobody asked for.

Copy link
Owner

@rodaine rodaine left a comment

Choose a reason for hiding this comment

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

Thanks again!

@rodaine rodaine merged commit a8aa9fe into rodaine:master Apr 2, 2024
3 checks passed
@Bios-Marcel
Copy link
Contributor Author

Thanks for merging and the quick response.

Have a nice day ;)

@Bios-Marcel Bios-Marcel deleted the feature/header_separator_row branch April 3, 2024 08:13
@punnie punnie mentioned this pull request Aug 2, 2024
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.

2 participants