-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
DOC: Notes about form fields and annotations #1945
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dmjohnsson23
thank you for your very good feed backs that will help people with the library
I've proposed you a few points that should complete your PR. can you review them ?
docs/user/add-watermark.md
Outdated
reader = PdfReader(content_pdf) | ||
if page_indices == "ALL": | ||
page_indices = list(range(0, len(reader.pages))) | ||
for index in page_indices: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you review your code based on the exchanges in #1902
this will provide a nicer solution not changing pages from the reader object and also keeping the pdf structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PR to use writer.append() based on your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way you are using .append()
will not capture the outlines, you should more likely pass the indices(as a list) as second parameter and remove the loop:
writer.append(reader,indices)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think I understand now. Wouldn't it be the pages
parameter though? writer.append(reader, pages=page_indices)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
docs/user/forms.md
Outdated
# If you want to fill out *all* pages, it is also safe to do this: | ||
data = {"fieldname": "some filled in text", "othername": "more text for an input on a different page"} | ||
for page in writer.pages: | ||
writer.update_page_form_field_values(page, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you have a look at #1903. more details about auto_regenerate = False
may help many people
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read over the linked thread and honestly I failed to fully understand it. At least, not well enough to be able to write any useful documentation about it. Someone more knowledgeable than I should probably provide those additional details as to why auto_regenerate = False
is recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously update_page_form_field_values() is computing the rendering of the field but also sets for the whole document the field NeedAppearances
forcing the viewer to recompute the rendering of the fields. the side effect is that the document will be marked as modified and the user will be ask to save the file. if you set the auto_regenerate parameter to false, NeedAppearances will be set to false.
the default value of value of auto_regenerate is true to keep compatibility with legacy behavior.
See #1945 Co-authored-by: dmjohnsson23 <[email protected]>
See #1945 Co-authored-by: dmjohnsson23 <[email protected]>
@dmjohnsson23 Thank you for your contributions! If you want, I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html (in case you're not already listed) |
I'm sorry that it took so long. There were some parts I was not happy with and some I was unsure about. In general, it is better to have smaller PRs. That makes it easier to review and merge. |
## What's new ### Bug Fixes (BUG) - Handle IndirectObject as image filter (#2355) by @stefan6419846 ### Documentation (DOC) - Quote specs in generate_file_identifiers (#2363) by @exiledkingcc - Notes about form fields and annotations (#1945) by @dmjohnsson23 - Notes about update_page_form_field_values(auto_regenerate) (#2359) by @dmjohnsson23 - Fix stamping example (#2358) by @dmjohnsson23 - Stamp images directly on a PDF (#2357) by @dmjohnsson23 - Correct the example of adding highlight annotation (#2341) by @Tobeabellwether ### Maintenance (MAINT) - Update upload-artifact and download-artifact actions from v3 to v4 (#2352) by @stefan6419846 ### Testing (TST) - Add xfail test for #2336 (#2365) by @MartinThoma - Increase test coverage for flate handling of image mode 1 (#2339) by @stefan6419846 ### Code Style (STY) - File identifier generation restructuring (#2362) by @exiledkingcc - Add PdfWriter._ID attribute (#2361) by @exiledkingcc - Variable naming convention (#2360) by @MartinThoma [Full Changelog](3.17.3...3.17.4)
These notes and additions are based on my experimentation with the library rather than a deep understanding of the PDF format or the inner workings of the code, so someone more knowledgeable than me might want to double-check what I've written. But I believe these notes will be beneficial to others experimenting with the library.