Skip to content
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

Add document ID to trailer for PDF/X #16

Merged
merged 6 commits into from
May 30, 2015
Merged

Conversation

bousquet
Copy link
Contributor

Adds a default trailer that includes a document ID consisting of two random hex values. This is used for PDF-X compatibility but does not negatively affect other PDF formats. A :trailer option can also be passed in during initialization which can override the default.

Also added basic specs or the PDF::Core::DocumentState class which had none.

@bousquet bousquet mentioned this pull request May 26, 2015
@bousquet bousquet changed the title Add document ID to trailer for PDF-X spec compat Add document ID to trailer for PDF/X May 26, 2015
@packetmonkey
Copy link
Contributor

I'm not comfortable by default including random information in a document. That means if we run the same PDF generation code twice we would get different PDFs out the other side.

I think I am ok requiring someone to set the trailer on document initialization to set an ID if they plan on kicking out a PDF/X document. We could also after the merge discuss possibly creating a Prawn::PDFX class that does the PDF/X initialization for you and can do other things like try to ensure CMYK only images and what-not.

@bousquet
Copy link
Contributor Author

Ok, I think that's a good point. I've removed the default_trailer and left it as the empty hash it was before, but left in the option to pass the trailer in via the options.

@packetmonkey
Copy link
Contributor

Some minor tweaks and I think this will be ready to merge.

  1. I think we can remove the added require "secure random" as we aren't adding a random ID anymore.
  2. Your describe "default trailer" do test isn't describing the default trailer, it's describing a trailer that was set. I think we if we rename the block describe explicit trailer it would be fine
  3. We should add a test to in fact ensure the trailer without any extra parameters is initialized correctly.

Once this gets merged we can add a changelog file to the repo so we can start to track changes in pdf-core as opposed to prawn.

Thanks!

@bousquet
Copy link
Contributor Author

I have adjusted the tests and removed the securerandom call. Good catch.

packetmonkey added a commit that referenced this pull request May 30, 2015
Add document ID to trailer for PDF/X
@packetmonkey packetmonkey merged commit 6b1b646 into prawnpdf:master May 30, 2015
@packetmonkey
Copy link
Contributor

I'm good with this update. thanks for the pull request! Once @practicingruby gives me access to the contributors group you will get commit access to the repositories because you had a pull request merged. here are the details.

Thanks again! Hopefully we can wrap up and give you easy access to all the PDF/X features your heart desires ❤️

@packetmonkey
Copy link
Contributor

You have been added to the contributors group, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants