markdown-pdf

markdown-pdf copied to clipboard

A breaking change: I changed the -o/--out flags to expect a directory. Specifying a file path down to the name is nonsensical when dealing with multiple files. The files are processed with as much parallelism as I could manage with async.

The CLI getting bigger was making me uncomfortable, so I added some initial unit tests for it. They only cover the really important parts for now, but it's some peace of mind.

Closes #47.

Is there a reason this hasn't been merged?

Incredible work. I'm sorry I never got round to reviewing this. I'm really stoked that you've added tests and in principle I'm happy for the CLI to be able to process and output multiple files.

The small issue I can see is here is that the existing functionality of specifying multiple files and having them combined into one PDF has gone. The original thought behind this was that if you were creating a thesis or a book or something you might want to have the chapters in separate markdown files and combine them together with the tool. I'd like to retain this functionality if possible, and I think we can with a little inspection of the arguments.

I'd propose:

multiple inputs + single file path output (ends in .pdf) = combine into one multiple inputs + directory path output = process individually

...and for single file input:

single file input + single file path output (ends in .pdf) = process individually (specify name) single file input + directory path output = process individually (automatic name)

What do you think @anko?

@alanshaw

The original thought behind this was that if you were creating a thesis or a book or something you might want to have the chapters in separate markdown files and combine them together with the tool. I'd like to retain this functionality if possible, and I think we can with a little inspection of the arguments.

Some thoughts:

I'm at least initially against this. It is trivial to cat files together as necessary (or type/copy on Windows) before passing them to markdown-pdf. Adding more implicit rules for what certain types of argument combinations mean makes it harder to document, understand, and maintain.

After all this time to think, I'm doubting my own PR here too. It's easy to use other programs to run multiple markdown-pdf processes in parallel, like with & or GNU Parallel. This PR's functionality-changes only remove the overhead of having multiple Node.js processes, which is very little. However, this also enables a future optimisation of rendering multiple documents using the same PhantomJS process, which would be a huge performance improvement.

Maybe we should cherry-pick the cleanups and tests from this and leave the rest to merge later, together with a hypothetical only-one-PhantomJS-process optimisation patch. Then at least we don't break API for (almost) nothing.