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

added support for multi-source input #53

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

Conversation

knassar
Copy link

@knassar knassar commented Oct 13, 2015

this adds support for the multi-file concatenation features of the underlying wkhtmltopdf library, including cover-page and TOC generation, with options on a per-source basis.

Also, added a couple of basic tests while I was working on this.

also, added basic tests
@knassar knassar changed the title #33: added support for multi-source input added support for multi-source input Oct 13, 2015
@devongovett
Copy link
Owner

Cool, thanks for adding some tests too. We should see about possibly testing the generation of command line args more thoroughly as well, rather than actually generating a PDF every time. That will be faster.

@knassar
Copy link
Author

knassar commented Oct 25, 2015

That would be faster. Want me to make those changes to the tests?

@devongovett
Copy link
Owner

That would be great, thanks!

@knassar
Copy link
Author

knassar commented Oct 26, 2015

The two tests now just compare the generated CLI args against expected, instead of running the wkhtmltopdf bin. You'll need to do another npm install, as I added a dependency on rewire for tests in order to mock out the private execution code.

@zxlin
Copy link
Collaborator

zxlin commented Apr 14, 2016

@knassar Thanks for the contribution, can you refactor it and I'd be happy to pull it in.

@davesag
Copy link

davesag commented Jun 9, 2016

👍 exactly what I need.

@chamnap
Copy link

chamnap commented Jun 20, 2016

When can it be merged? +1

@zxlin
Copy link
Collaborator

zxlin commented Jun 20, 2016

@chamnap: the repo's changed a lot since this PR was made, we'd need someone to refactor this before I can pull it.

@knassar
Copy link
Author

knassar commented Jun 20, 2016

Would love to get this merged, but I currently don’t have time to refactor the PR to match the repo. Will be happy to tackle it when I have a window, but not sure when that will be.

—Karim

On Jun 20, 2016, at 2:25 PM, Zhi Xiang Lin notifications@github.com wrote:

@chamnap https://github.com/chamnap: the repo's changed a lot since this PR was made, we'd need someone to refactor this before I can pull it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #53 (comment), or mute the thread https://github.com/notifications/unsubscribe/AATQm-HK_r6dMqwjkR1VVSgSQz_jByfSks5qNtsUgaJpZM4GOJ0p.

@DrMabuse23
Copy link

is it possible to use it with a blank string ?

This was referenced May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants