-
Notifications
You must be signed in to change notification settings - Fork 56
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
Support for writing and reading manifest data from simple PDFs (without incremental updates or signatures) #249
Conversation
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 78.73% 78.94% +0.20%
==========================================
Files 76 77 +1
Lines 22465 22764 +299
==========================================
+ Hits 17687 17970 +283
- Misses 4778 4794 +16
☔ View full report in Codecov by Sentry. |
adc4694
to
a1e902d
Compare
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.
One tiny documentation nit and a couple of non-blocker questions to consider.
I'd like @mauricefisher64 to review this since he knows PDF better than I do, but otherwise, this looks really solid. Easy code to follow!
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.
👍
hey @scouten-adobe & @mauricefisher64 : I believe I've addressed all of your feedback. Please take a look again when you have a moment! |
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.
Please either explain the Vec of slices or remove the abstraction. Otherwise 👍
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.
👍
Not holding up this PR but does the lopdf handle PDF permissions and passwords? |
@mauricefisher64: Yes, |
Changes in this pull request
This PR introduces the
pdf_utils::Pdf
struct and thepdf_utils::C2paPdf
trait, public (to the crate),lopdf
abstractions with functionality to read and write manifest data from PDFs. This code works againstwasm
andnon-wasm
targets.I intend to add read support next, then round out the work with incremental update support. I opened this PR against
origin/pdf
instead ofmain
as I want to get this in front of folks before getting to far and this work doesn't support reading manifests or incremental PDF updates.Checklist
TO DO
items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.