Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 aspdf-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
andCe/docgen
and wkhtmltopdf fails with lots of messages like: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 usingspawn-args
I could remove the dependancy frompackages.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.