search

traviscooper / node-wkhtml

Convert html to PDF or PNG format using the webkit rendering engine, and qt.
104 stars 17 forks source link

Changed to use "spawn" instead of "exec" #7

Closed bencpeters closed 12 years ago

bencpeters commented 12 years ago

Also, unified functionality somewhat along the lines that you proposed, so that everything uses the single function "convert" instead of having "convert" and "convertAs". Now, if no filename is given to have the command save locally, the function includes the data in a buffer to the callback function for the user to do with as they please.

I haven't tested it extensively, but it seemed to work on my example server. If you want to have the unified interface different than the one I propose here, by all means go ahead.

mhemesath commented 12 years ago

Nice work, I'll review this more thoroughly tonight. After I merge, I'm probably going to change a few things and add tests before releasing to NPM.

This was a great start, but in addition to using spawns, we need to expose the spawned process, or at least its stream, to the caller so the generated PDF can be piped directly to the response if needed. I'll probably implement something similar to what @mlegenhausen proposed in https://github.com/mhemesath/node-wkhtml/issues/5 in regards to this.

bencpeters commented 12 years ago

Cool, that looks good. I like the ability to have access to the spawned process, or the buffer, whichever is more convenient. Let me know if you want any help implementing this further!

bencpeters commented 12 years ago

Doh, just ran it again today and realized that when I added in the "globalOptions" in my original commit I messed it up, my latest commit should fix that...

mhemesath commented 12 years ago

Thanks for waking this project back up :p I went ahead and merged this through. Before I release I'm going to refine a few things. Hopefully I'll have this out this week. Thanks!