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

Fix #28: correct width for wide columns #37

Merged
merged 1 commit into from Oct 8, 2018
Merged

Fix #28: correct width for wide columns #37

merged 1 commit into from Oct 8, 2018

Conversation

Sideboard
Copy link
Contributor

This should fix the bug that wide columns receive all the remaining width from max_width described in issue #28. I am not sure why the columns are first flagged and than adjusted if their width exceeds the "fair share" of `max_width", so I left that code in. Maybe it's not needed and this could be solved by removing the lines but I suppose note.

I also added a test to check if a table with a large max_width gives the same string length as table with a small (but sufficient) max_width.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 78.662% when pulling 9787174 on Sideboard:master into d3e55f5 on pri22296:master.

Repository owner deleted a comment from coveralls Oct 7, 2018
Repository owner deleted a comment from coveralls Oct 7, 2018
Repository owner deleted a comment from coveralls Oct 7, 2018
Repository owner deleted a comment from coveralls Oct 7, 2018
@pri22296
Copy link
Owner

pri22296 commented Oct 7, 2018

Thanks for the Pull Request. Can you just squash the 2 commits into a single one? Other than that this looks good to me.

Add test_table_auto_width() that checks for a table to behave the same
way if one column is wider than its equal share of max_width compared
to when all columns stay below that, assuming max_width is large enough
to accomodate the complete table.

Adjust auto_calculate_width() to keep the actual width if it's smaller
than the new one with adjusted share. This fixes issue #28 and passes
the new test.

I'm not sure why the flagging by equally shared width is used, so I
left it in. This may be improvable with more insight into what the
width adjustment is meant to solve.
@Sideboard
Copy link
Contributor Author

Squashed.

@pri22296 pri22296 merged commit b9cd55c into pri22296:master Oct 8, 2018
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