[Improved] PdfMerger: Allow to select the pages when merging documents #248
Conversation
Codecov Report
@@ Coverage Diff @@
## master #248 +/- ##
==========================================
+ Coverage 64.39% 64.40% +0.01%
==========================================
Files 600 600
Lines 39714 39758 +44
==========================================
+ Hits 25572 25607 +35
- Misses 14142 14151 +9
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| /// <summary> | ||
| /// Merge the set of PDF documents. | ||
| /// </summary> | ||
| public static byte[] Merge(IReadOnlyList<byte[]> files) | ||
| public static byte[] Merge(IReadOnlyList<byte[]> files, IReadOnlyList<IReadOnlyCollection<int>> pagesBundle = null) |
There was a problem hiding this comment.
worth considering, a IReadOnlyList<(byte[] File, IReadOnlyCollection<int> Pages)> signature.
|
This looks really useful. PdfPig looks nice for text extraction but is missing some features such as splitting / merging documents, modify existing documents, etc. Looks like you've put a lot of work into these areas @InusualZ thanks for the effort and hope these get merged soon. |
| } | ||
|
|
||
| /// <summary> | ||
| /// Merge the set of PDF documents. | ||
| /// </summary> | ||
| public static byte[] Merge(IReadOnlyList<byte[]> files) | ||
| public static byte[] Merge(IReadOnlyList<byte[]> files, IReadOnlyList<IReadOnlyCollection<int>> pagesBundle = null) |
There was a problem hiding this comment.
IReadOnlyCollection or ICollection are probably the wrong types here, since they do not guarantee order iirc, I'd recommend IReadOnlyList since order is important here.
PdfMerger: Allow to select the pages when merging documents Supersede UglyToad#245
Plus, Use IReadOnlyList instead of IReadOnlyCollection
InusualZ
force-pushed
the
merger-selection-v2
branch
from
69806b4
to
82806b7
Compare
Supersede #245
This PR offers a much better option and flexible approach to allow a page selection when merging documents. Also, I address the drawback of the other PR which was related to copying pages out of order. As a by product we now allow a page to be copied multiple times (By repeating the page index multiple time in the selection)
Illustration of the problem (Copied)
This is my main document
Now, let's try to merge the selection of pages (1, 3, 2) in that order:
I solve that by copying the page from bottom to top, instead of top to bottom like it was before. By doing so, we got rid of one recursive method (
DocumentMerger::CopyPagesTree()). I think thanks to this last bit, we can introduceParallelism, but that's a topic for another day.I would like to comment, that I don't like the current state of public PdfMerger API
cc @Poltuu I think this would entirely resolve #244, any comments?