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 support for multiple documents #768

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bibstha
Copy link

@bibstha bibstha commented Sep 19, 2018

wkhtmltopdf supports downloading multiple html documents and combining them into a single pdf.

wkhtmltopdf url1 url2 ... urlN path/to/file.pdf

WickedPdf doesn't support it right now.

This PR adds this support. It introduces two new interfaces.

  • WickedPdf#pdf_from_multiple_strings
  • WickedPdf#pdf_from_multiple_html_files

While keeping the existing interfaces as it is.

@bibstha
Copy link
Author

bibstha commented Sep 19, 2018

Looking at the test failures, seems like we need a patched version of QT for this. 🤔

@bibstha bibstha force-pushed the add_support_for_multiple_documents branch from 2a3be0f to 79ff029 Compare September 19, 2018 15:08
@unixmonkey
Copy link
Collaborator

Great! This seems very useful, especially for cover pages etc!
I think you might be able to make CI use a QT patched version by modifying this line

But it'll likely need to be a bit more involved to be something like this:

sudo apt-get install -y libqt5webkit5-dev libqt5xmlpatterns5-dev libqt5svg5-dev &&
  wget https://downloads.wkhtmltopdf.org/0.12/0.12.5/wkhtmltox_0.12.5-1.bionic_amd64.deb &&
  sudo dpkg -i wkhtmltox_0.12.5-1.bionic_amd64.deb &&
  sudo apt-get install -f

BTW, there's a lot of cool stuff in the wkhtmltopdf Travis config that might help.

@bibstha bibstha force-pushed the add_support_for_multiple_documents branch 3 times, most recently from 03b5460 to 4eb9f89 Compare September 22, 2018 20:02
@bibstha bibstha force-pushed the add_support_for_multiple_documents branch from 4eb9f89 to 7de447a Compare September 22, 2018 20:10
@bibstha
Copy link
Author

bibstha commented Sep 22, 2018

@unixmonkey I've updated the PR by adding wkhtmltopdf with patched-qt.
All the tests passed except the with ruby version 1.8.7. This is because I added pdf-inspector as a development dependency - to make sure the pdfs generated could be tested easily.
Do we still want to support 1.8.7 (Or can we move it to the "Allowed failures" section on Travis)?
Otherwise the rest of the things on this PR looks good.

@unixmonkey
Copy link
Collaborator

I think it's high time to drop support for 1.8.7. If someone still on that platform needs this, I think it's safe to assume they are ok pinning to an existing version or writing their own patches. Doing this officially should be a major version bump though.

Since this is a development-only dependency requirement right now, let's move 1.8.7 to allowed failures.

@miguelms95
Copy link

Hi! Is this cool feature going to be merged??

@KristenWilde
Copy link

Would love to see this feature added.

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

Successfully merging this pull request may close these issues.

4 participants