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
Horizontally merged rows always centered #207
Comments
This is not a bug but it is definitely something I want to fix by giving the user an option on the alignment to use. Will get this fixed this week. 🤞 |
That'd be awesome! I was going to attempt to do it if there wasn't a large technical hurdle, but my guess is you can do it in a fraction of the time if you have an idea of what's needed. |
One other semi-related question, when merging rows horizontally column width seems to auto-size to the width of the contents of the merged cell. Is that related to the changes you're aware of related to merged cell alignment? |
You mean cell A with width 5 and cell B with width 7 when merged have a cell with width 12? If this is the case, the behavior is right. If not, can you please explain? |
Hey @jlambert121 can you check the commit above and lemme know if this works for you? At the moment, I cannot provide a way to customize alignment modes per instance and it will be global to a row. |
Yep, would agree with that. What I was seeing was column A was 5 wide (3 rows), column B was 7 wide (3 rows), footer merged between column A and B where the text was 8 would make both column A and B 8 wide by adding the footer rather than looking at the total width of the footer (12) and saying it would fit. |
This is awesome - exactly what I was expecting, thank you! |
Here's an example of what I was looking at. If you comment out the footer it draws the cells at the correct width, but adding the footer looks like it calculates the correct width of the cell based on all columns (including the footer) and not taking into account that that cell is actually wider. As I typed this out, this may be a difficult calculation to make on the fly.
Without auto-merge enabled it's exactly what I'd expect, with it enabled I was not expecting the results, but it also is somewhat of a thorny problem. |
Interesting. The issue here is that the Footer's 1st column length is being used for the 1st and 2nd columns (incorrectly). Let me debug this a bit. |
What's your thoughts about merging the alignment work and I can open a new issue for the column width issue? I might be able to get some time this week to look at it as well. |
@jlambert121 sorry for the delay - I slept on this issue. I've merged the fix proposed for the first issue and cut a tag for use: https://github.com/jedib0t/go-pretty/releases/tag/v6.3.2 --- I'll try to address the footer issue - it is not easy and I remember I decided to skip supporting this back then - let me revisit and see if there is a way to address this without breaking anything or introducing any weird behavior in corner cases. |
@jlambert121 Had a brain fart when I agreed the footer column length was an issue. Here is the render with the auto-merge:
Here is the render without the auto-merge:
As you can see, it is just using the longest column length (that being the column from the footer row) as the first column length. Currently merging does not adjust the column lengths (even if all the rows have their columns merged) and supporting such a case with the current code base will be a pain to maintain. I'll close this for now and try to address this in a future rewrite/refactor if possible. |
I have a pretty complex data set to output and ran into this design choice with merged values. |
@andrewpmartinez I've delivered a bug-fix for the issue relating to merging and wrapped lines: https://github.com/jedib0t/go-pretty/releases/tag/v6.3.4 --- can you please try this? |
Updated to v6.3.4 and I see the following:
I can provide sample code and input if you would like. Let me know as it will take some effort. |
@andrewpmartinez For #2, the default align used is AlignCenter (as shown in comments): // RowConfig contains configurations that determine and modify the way the
// contents of a row get rendered.
type RowConfig struct {
// AutoMerge merges cells with similar values and prevents separators from
// being drawn. Caveats:
// * Align is overridden to text.AlignCenter on the merged cell (unless set
// by AutoMergeAlign value below)
// * Does not work in CSV/HTML/Markdown render modes
// * Does not work well with vertical auto-merge (ColumnConfig.AutoMerge)
AutoMerge bool
// Alignment to use on a merge (defaults to text.AlignCenter)
AutoMergeAlign text.Align
} Can you try overriding it with your preferred Alignment options? |
That did it, thank you very much! Unrelated but looks like the last horizontal line that has auto merge has a single vertical bar tick at the end. In the output below you can see it after
|
At this point this isn't a bug report or feature request, but a question. I was attempting to create a summary footer row I wanted merged and aligned to the right, but it was always centered. Digging into it some more, it looks like this is the desired effect - https://github.com/jedib0t/go-pretty/blob/main/table/render.go#L86. Could you add some context as to why you override the alignment to always be centered (instead of maybe a default if unset)?
The text was updated successfully, but these errors were encountered: