Table cell annotations are misplaced (or crash) when the cell is vertically padded - #122
Open
jimbofreedman wants to merge 1 commit into
Open
Conversation
The multi-line path in `TableCell.get_annotations` built `line_break_pos` from the post-padded `self.line_width` (which contains zero-length entries for the top and/or bottom padding added by `height.setter`) and then indexed `annotation_lines` by `no + self.vertical_padding`. Because `no` was already a padded line index, adding `vertical_padding` on top double-counted the top padding: * `valign=bottom` cells shorter than their row could write past the end of `annotation_lines`, raising `IndexError` on the second or later content line's annotations. * `valign=middle` cells silently shifted annotations onto the bottom-padding lines below the actual content, so `text[start:end]` returned whitespace or text from the next column. Fix: scan only the content-width slice of `line_width` (using a new `_content_height` recorded in `normalize_blocks`) so `no` is a content-line index 0..rows-1, and the destination `annotation_lines[no + top_pad]` then correctly references the output line holding the content. Regression tests in tests/test_table_annotation_vertical_padding.py cover the `IndexError` crash and the silent middle-/bottom-align offset shift.
jimbofreedman
marked this pull request as ready for review
July 28, 2026 04:55
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.
Fixes #121.
The multi-line branch of
TableCell.get_annotationsdouble-counts a cell's top padding, so annotations inside a cell that is shorter than its row either land on the wrong line or index past the end ofannotation_lines.Cause
height.setterpads short cells with empty lines, andwidth.setterthen recordsline_widthfrom the padded blocks — soline_widthcarries one zero-length entry per padding line.get_annotationsaccumulatesline_break_posover that padded list, which makes the loop variablenoa padded line index, and then addsself.vertical_paddingon top of it:Annotation
startoffsets reference the pre-padding cell content, sonohas to be a content-line index for the+ vertical_paddingshift to be correct.valigntopvertical_padding == 0middletext[start:end]returns whitespace or text from the next columnbottomIndexError: list index out of rangeas soon as annotations reach the second content lineMinimal reproducer on
master(2.7.3):See #121 for the
valign=middlecase and full output.Fix
Record the pre-padding row count as
_content_heightinnormalize_blocks, and accumulateline_break_posover only the content slice ofline_width.nois then a content-line index0..rows-1, andannotation_lines[no + top_pad]addresses the output line that actually holds the content. The single-line branch andvalign=topare untouched.Tests
tests/test_table_annotation_vertical_padding.pyadds four regression tests (bottom-align crash, middle-align offset shift, annotation on the last content line, padded two-column list). All four fail onmaster— two withIndexError, two with wrong slices — and pass with the fix.Locally clean:
uv run ruff check,uv run ruff format --check,uv run ty check,uv build, anduv run pytest(76 passed) on each of Python 3.10–3.14.Relation to #92
As noted in #121, this is not the bug reported in #92 — that snippet's two columns are equal height, so no padding is applied and its output is unchanged by this PR. #92 remains open.