Repository navigation
BUG: Insert outline items in front of before and cover more of PdfWriter - #4415
Open
MartinThoma wants to merge 5 commits into
Open
MartinThoma wants to merge 5 commits into
MartinThoma wants to merge 5 commits into
Conversation
Member
Author
|
I'll check if there is any comment from #4342 left to integrate |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4415 +/- ##
==========================================
+ Coverage 98.87% 99.05% +0.17%
==========================================
Files 59 59
Lines 12299 12308 +9
Branches 2342 2345 +3
==========================================
+ Hits 12161 12192 +31
+ Misses 68 55 -13
+ Partials 70 61 -9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…riter PdfWriter.add_outline_item(before=item) appended the new item at the end unless `item` was the last one of its level, and inserting in front of the first item lost the new item: it was counted in /Count, but not linked into the outline, as /First of the parent was not updated (ISO 32000-2:2020, 12.3.3, Table 150). Closes #4340. TreeObject.insert_child() looked for `before` starting at /Last and following /Next, so it only found the last child. It now starts at /First when `before` is given and updates /First when inserting in front of the first child. Appending without `before` still starts at /Last, so it stays O(1). A `before` that is no child of the parent still appends at the end. Add tests for PdfWriter branches without coverage so far: * add_outline_item() with `before` being None, the first, a middle, the last or the only item, without a page (no /A action), and with a PageObject, * the page_mode getter, add_annotation() with an invalid page type and clean_page() converting a named destination inside an /A action, * merge() with excluded_fields=None and with form fields that are not on any merged page, * reset_translation() for a reader the writer never copied from and for an IndirectObject, checked by appending the reader again and comparing the copied /Resources. Where the expected behavior comes from ISO 32000-2:2020, the docstring names the section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MartinThoma
force-pushed
the
tst/writer-branch-coverage
branch
from
November 12, 2026 21:44
a19b6ef to
152ae7c
Compare
before and cover more of PdfWriter
MartinThoma
marked this pull request as ready for review
November 13, 2026 14:06
This branch has not been deployed
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.
PdfWriter.add_outline_item(before=item)only worked when item was the last item of its level.TreeObject.insert_child()searched for before starting at /Last and following /Next, so it only ever found the last child. It now starts at /First when before is given, and updates /First when inserting in front of the first child. Appending without before still starts at /Last, so it stays O(1). A before that is not a child of the parent is still appended at the end, as before. #3965 also touches that part.The PR also adds tests for PdfWriter branches without coverage so far:
Closes #4340.