Added removePage() method #65

Closed
wants to merge 5 commits into
from

Projects

None yet

5 participants

@niftylettuce

Allows us to remove the current page, very useful for doing stuff in sequences for flow control.

@niftylettuce

Coffeescript is different... and I wasn't familiar with pdfkit's lib on that first commit, hehe.

@niftylettuce

That should be it, go ahead and pull/merge if it looks OK to you.

@devongovett
Owner

Looks ok, but what is the use case for removing the current page after you just created it? Why not just not create it in the first place?

@NathanaelA

Well, if you add another page in anticipation that you will put data on it; and figure out after the fact that you don't have any markup that got printed; then being able to delete this "blank" page at the end of the report.

@niftylettuce

@devongovett it looks like @Nathanaela said it best, however I'm sure there are other scenarios. It's extremely useful to be able to both add and remove pages. Please merge this in, thank you 👍.

@HughePaul

This would be useful to remove the letter-sized page that is automatically added when a new document is created, so that all pages can be A4.
Or is there a way to set what size that initial page is?

@devongovett
Owner

Yeah, you can set global options when you create the document:

doc = new PDFDocument
    size: 'a4'

This way all pages you create with addPage() will also automatically be set to 'a4' size.

@brianreavis

Hey @devongovett, first, thanks for such an awesome project – it's a lifesaver! But out of curiousity, why is there resistance to this PR?

I think the pattern presented in #120 is very common. Anyone who's iterating through a list of pages to spit out is going to hit this. Without this merged (or without the default behavior changed), we're left shimming this into our applications to work around it:

PDFDocument.prototype.removePage = function() {
    var pages = this._root.data.Pages.data;
    pages.Kids.pop();
    pages.Count--;
    this.page = null;
};
@NathanaelA

I'm using code like this in my new PDF Import routine that I will be pushing shortly to my fluentReports repository -- as I need to be able to automatically delete a blank page at the close of the document. Since it is very possible & likely someone will do:
Page 1: Write Some Data using pdfkit...
Page 2-x: Imported PDF Document(s)
Page y: <--- This page needs to be deleted as they really don't need another page to write any more data too.

Although the routine I wrote will allow them to manually delete any page in the document; but by default my code monkeypatches the pdfkit Document.end and then auto-deletes any blank last page before calling the pdfkit's document.end....

I suspect the reason why it isn't supported is because technically the only page that can actually be deleted (and not saved to the output data stream) is the very last page. All other pages are finalized and written to the data output when you add a new page. However, you can remove any page from the page count and index and it won't show up as nothing links to it.

@devongovett
Owner

Yes, exactly. By default, pages are written directly to the output file stream once another page has been added. So if you deleted a page, it wouldn't actually be removed from the file, just the reference would be removed.

However, with v0.7, we have the bufferPages option, which allows the user to control when pages are written to the file manually, and switch to previous pages. I wouldn't be opposed to revisiting a removePages method integrated with bufferPages. If bufferPages was turned on, you could remove any page currently in the buffer, with doc.removePage(index). If it was not turned on (default), you could only remove the last page (the only one in the buffer). If someone wants to write a PR for that, I'd be happy to merge it. I don't have time at the moment to write it myself, but it should be fairly straightforward.

@brianreavis

Awesome. Would you (also) be open to a pull request for an option autoFirstPage that defaults true? (or similar) Setting it to false would disable the first page. That way there'd be no need to turn on buffering if you just don't want the automatic first page.

@devongovett
Owner

The problem with that is that the rest of PDFKit assumes that there is always a page available, and weird things could happen if you tried to write to a page before calling addPage.

@brianreavis

Isn't that fine? The person would explicitly have to set autoFirstPage:truefalse, which would imply they know what they're doing I think

@devongovett
Owner

Sure. Let's make autoFirstPage: false mean not to add the first page automatically. Any other value or none at all would have the current behavior of automatically adding the first page.

@brianreavis

Oops, that's what I meant to write. Sounds great!

@devongovett
Owner

Any update on this @brianreavis?

@brianreavis

@devongovett Sadly no. I got busy with other things, but I'll try to see if I can fit it in today

@brianreavis brianreavis added a commit to naturalatlas/pdfkit that referenced this pull request Apr 2, 2015
@brianreavis brianreavis Allow automatic first page to be disabled (#65). d28983d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment