search

walle / gimli

Utility for converting markup files to pdf files
https://github.com/walle/gimli
MIT License
538 stars 44 forks source link

Add support to merge files to one pdf #3

Closed walle closed 13 years ago

walle commented 13 years ago

Add a -m (merge) flag to support merging all files to one. This should perhaps allow us to specify a list of files to convert/merge instead of just one or all in the folder.

nicknovitski commented 13 years ago

I made a topic branch that merges all matched files, but it's based on the Converter class being changed (converted, you could say) to taking an array of MarkupFiles instead of one each: njay@5522f2b095937c16d25c

It's set up so the old tests still pass, but I was wondering what your thoughts would be on changing the API in that way. It seems like a better deal to me, especially with the merge functionality in mind: converting the files all at once seems better than tracking, merging and cleaning up the pdfs after-the-fact (even though having page-breaks between each source file would be pretty nice (oh boy, a new option to code)).

walle commented 13 years ago

I think making the converter take an array of MarkupFiles is the way to go. It should have been designed that way from the beginning.

I think we should add another option to explicitly set the name of the output file.

The page break functionality does not seem to hard to code, I have only skimmed through the cast but here is a description on how to do it. http://asciicasts.com/episodes/220-pdfkit It would be nice to add a class that does the page break so you can use it in your markup too.

I will take a closer look on your branch when I get home tonight.

Thanks for your work on the project!

walle commented 13 years ago

I have made some commits in https://github.com/walle/gimli/tree/merge. The branch is based on yours.

nicknovitski commented 13 years ago

Everything looks good, except in 6050e92f0fb3e7cdf8cc9814edf19fde11580a5f, I'm pretty sure that the check to make sure files is an array is unnecessary.

walle commented 13 years ago

Good point. I'll fix that now.

walle commented 13 years ago

This is merged to development. Closing this issue and opening new ones for the output filename and the page break class. Thanks for your contribution!

walle commented 13 years ago

This is now included in 0.1.7. Thanks again!