Conversation
print_table measured column widths with len() and padded cells with str.ljust(). Both count code points rather than terminal cells, so a table containing wide characters (CJK, emoji) lost its alignment: such characters occupy two cells but were counted as one. This is reachable through `toot lists`, where a list title may use any script. Measure widths with wcswidth() and pad with wcstring.pad() instead. wcswidth() is already used for alignment elsewhere in this module, and pad() is the existing wide-character aware helper. ASCII tables render exactly as before.
Contributor
Author
|
Closing this to clean up my open PRs. It's a small change and not really worth taking up your review time. Sorry for the noise. |
Owner
|
You're an impatient one. :) I merged your contribution: 25b0057 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
print_table(used bytoot lists) sized its columns withlen(cell)and padded them withstr.ljust(width). Both count code points, not terminal cells, so wide characters break the layout: CJK and emoji take two cells but were counted as one, which pushes every following column out of alignment.The fix measures widths with
wcswidth()and pads withwcstring.pad(), the wide-character aware helper already in the codebase.wcswidth()is also whatstatus_linesuses to align the status header, so this just brings the table in line with the rest of the module. ASCII tables are unchanged.Before (a list whose title contains CJK)
After
Tests
Added
tests/test_output.py, which renders a table mixing ASCII and CJK rows and asserts every row ends up the same display width. It fails on the old code and passes with the fix.pytest,flake8andverminall pass locally.