lillallol / outline-pdf

Adds collapsible outline to pdf and works both in nodejs and the browser.
MIT License
21 stars 0 forks source link

Allows Outline to have page numbers not in sequence #5

Open bbevers2 opened 2 months ago

bbevers2 commented 2 months ago

Hi! šŸ‘‹

Firstly, thanks for your work on this project! šŸ™‚

Today I used patch-package to patch @lillallol/outline-pdf@4.0.0 for the project I'm working on.

I am not sure what the original Error is trying to catch. It is valid to allow outlines to reference page numbers "out of order". This is especially apparent if you want "subpages" of an outline to reference pages that are appended.

The suggested logic is not perfect (as you may want to check the parent of the subpage), but allows for subpages to reference pages later in the document then the next top level.

A generated PDF is still valid without this check too.

Consider the following outline when testing

             1||Top level
             1|-|Section 1.0
             3|--|Page of attachment
             4|--|Another attachment
             2|-|Section 2.0

Here is the diff that solved my problem:

diff --git a/node_modules/@lillallol/outline-pdf/dist/index.esm.js b/node_modules/@lillallol/outline-pdf/dist/index.esm.js
index 86b928d..69e8a43 100644
--- a/node_modules/@lillallol/outline-pdf/dist/index.esm.js
+++ b/node_modules/@lillallol/outline-pdf/dist/index.esm.js
@@ -615,7 +615,7 @@ function printedToOutline(inputOutline, totalNumberOfPages) {
             if (!(nodeToReturn.depth <= lastNode.depth + 1)) {
                 throw Error(errorMessages.errorMessages.wrongDepthDisplacement(lastNode.line, nodeToReturn.line));
             }
-            if (nodeToReturn.pageNumber < lastNode.pageNumber) {
+            if (nodeToReturn.pageNumber < lastNode.pageNumber && nodeToReturn.depth >= lastNode.depth) {
                 throw Error(errorMessages.errorMessages.invalidDisplacementOfPage(lastNode.line, nodeToReturn.line));
             }
             if (lastNode.collapse && lastNode.depth >= nodeToReturn.depth) {

This issue body was partially generated by patch-package.

EDIT: I edited the example based on the this comment

lillallol commented 2 months ago

I think the outline you provide does not work because the nesting depth jumps from 1 to 3. Try this one:

1||Top level
1|-|Section 1.0
3|--|Page of attachment
4|--|Another attachment
2|-|Section 2.0

and delete the following if clause from my package:

if (nodeToReturn.pageNumber < lastNode.pageNumber) {
    throw Error(errorMessages.invalidDisplacementOfPage(lastNode.line, nodeToReturn.line));
}

Let me know if that works as you want.

bbevers2 commented 2 months ago

Removing that check entirely works for me. My suggestion was just to have the least amount of impact in case there is a specific reason why the page numbers need to be sequential.

lillallol commented 2 months ago

in case there is a specific reason why the page numbers need to be sequential

To be honest I do not know why I added that if clause. There is a use case for the page numbers to not be sequential as you mentioned. Maybe I should take a look at the pdf specification. For the current time being I have other stuff to deal with, so expect me to do that in the next weeks.