search

mtmacdonald / docgen

A documentation tool
http://mtmacdonald.github.io/docgen
Other
44 stars 15 forks source link

Spaces in path breaks wkhtmltopdf #20

Open davidnewcomb opened 8 years ago

davidnewcomb commented 8 years ago

This branch contains 2 commits. The first commit contains 2 fixes.

source/docgen.js:getPdfArguments() When adding PDF option header-html, footer-html several of the files did not exist and so always wkhtmltopdf always threw an error. I modified them to point to template versions in the same way as pdf-stylesheet. Really, we/you should have a policy on overriding the default templates and add the options accordingly (rather than a load of errors, even if it is in debug).

The main reason for this commit was I noticed a problem when trying to use Docgen with Maven and Jenkins. After a lot of trouble and strange messages the problem boiled down to including spaces in the input and output options of the docgen command: docgen -v -i "/path/with/a/spa Ce/docgen" -o "/path/with/a/spa Ce/docgen-web" -p Node’s Commander returns the arguments correctly but when the are joined together as a string the parameter boundaries are lost. By the time it comes out of spawnArgs the correct arguments have been split into 2 separate arguments /path/with/a/spa and Ce/docgen and wkhtmltopdf fails with lots of messages like:

Creating the PDF copy (may take some time) Loading pages (1/6) Error: Failed loading page http://Ce/docgen-web/docs/temp/pdfFooter.html (sometimes it will work just to ignore this error with --load-error-handling ignore) Error: Failed loading page http:///path/with/a/spa (sometimes it will work just to ignore this error with --load-error-handling ignore) Received createRequest signal on a disposed ResourceObject's NetworkAccessManager. This might be an indication of an iframe taking too long to load. ignore this error with --load-error handling ignore) Exit with code 1 due to network error: ProtocolUnknownError Warning: Received createRequest signal on a disposed ResourceObject's NetworkAccessManager. This might be an indication of an iframe taking too long to load.

The problem could have been solved by trying to wrap every file argument in quotes but it started to get messy so I decided it was simpler to modify the pdfOptions to be an array of arguments instead of an array of strings. As I was no longer using spawn-args I could remove the dependancy from packages.json.

The second commit just contains a bit of styling. Javascript declarations may or may not have a semi-colon at the end, but the style should choose and stick to it. I spent ages hunting for a missing ‘}’ because the javascript just reported that the error was on the last line. Adding the extra semi-colons forced the parser to error closer to the actual problem.

I think this fixes #13 as the new code adds pdfName into the argument's array as one item and so it no longer matters how many spaces are in it.

davidnewcomb commented 6 years ago

Been doing a bit of house keeping and deleted my repo for this project. The pull request should still work though.