Optional page buffering

[ef4]

This closes #225 by adding an optional page buffer. The API works as follows:

1. Pass the option bufferPages: true when creating your document.
2. Call doc.bufferedPageCount() at any time to ask how many pages are buffered.
3. Call doc.switchToPage(n) to make any of the buffered pages the current page again. This lets you add additional content.
4. Call doc.flushPages() to flush the page buffer out to the underlying stream. This is optional, it also happens automatically at end().

[tuance]

This is the perfect solution to what I need to allow me to upgrade to the latest version and keeping the functionality of post page rendering operations. Thanks!

[MathieuLoutre]

This looks really nice, thank you!

[devongovett]

I like this solution, thanks for the PR. Just a few minor changes, and I think we can merge this.

I think instead of creating @_pageBuffer only when in bufferPages mode, we should always create it, and then just call flushPages internally when a new page is added if NOT in bufferPages mode. I think the code will be simpler this way. That way, flushPages will be used both internally and externally. I'll put some line comments to clarify what I mean. Does this make sense to you?

[devongovett]

Always create this array, regardless of @options.bufferPages.

[ef4]

Sure, I like the proposed edits. I'll push an update.

[devongovett]

One more thing that might be confusing for some people is that switchToPage takes indexes in the buffer, not indexes in the document. So, switchToPage(0) doesn't switch to the first page in the document, it switches to the first page in the buffer.

Maybe we should keep a @_pageBufferStart variable that tracks the first index in the @_pageBuffer so that calling switchToPage(0) throws an error if page 0 in the document had already been flushed. What do you think?

[ef4]

If we did change the meaning of switchToPage like that, we would need to change bufferedPageCount to bufferedPageRange so people know what they can iterate over.

But I think the dominant use case will be buffering all the pages until the end, and that case wouldn't benefit from this added complexity. So I prefer the API as it is.

[devongovett]

Yeah I agree about the dominant use case, but it still seems strange to me. Right now, if I call switchToPage(0) when not in bufferPages mode, it does nothing since it switches back to the current page.

I think, if we keep it how it is, the method should be renamed to something like switchToBufferedPage to be more clear. Otherwise, we switch it to global page numbers and rename the other method to bufferedPageRange as you point out.

The complexity of the second solution is not that much, just one additional variable that would get incremented by @_pageBuffer.length when flushPages is called. What do you think?

[ef4]

Yeah, that sounds ok. Would you prefer

bufferedPageRange: -> {start: @_pageBufferStart, count: @_pageBuffer.length}

vs

bufferedPageCount: -> @_pageBuffer.length
bufferedPageStart: -> @_pageBufferStart

[devongovett]

The first one. I think just one method is better.

[ef4]

Oops, I just realized I pushed the wrong version, hang on...

[ef4]

Ok, all suggestions incorporated.

[devongovett]

Great, thanks so much for the PR! I know a lot of people have been itching for this feature for a few months, so glad to incorporate something for them.

[ef4]

Sweet, glad to help. Thanks for fast merges.

[MathieuLoutre]

Awesome that this got merged so fast. Really looking forward to using this now!

On Fri, Sep 12, 2014 at 8:06 PM, Devon Govett notifications@github.com
wrote:

Great, thanks so much for the PR! I know a lot of people have been itching for this feature for a few months, so glad to incorporate something for them.

Reply to this email directly or view it on GitHub:
#302 (comment)

[devongovett]

Just released v0.7.0 with this feature and lots of other fixes. Check it out! You can find docs on this particular feature, on the website. Thanks again, @ef4!

[ef4]

😄