From 980df238438bb616e9ab04eb76b96fc9117a2e04 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 14 Sep 2025 22:17:25 +0100 Subject: [PATCH] Refactor `QPDFJob`: consolidate `QPDFPageData` into `Selection`, simplify page range handling, and cleanup obsolete logic. --- libqpdf/QPDFJob.cc | 63 ++++++++++++++++++--------------------------------------------- libqpdf/qpdf/QPDFJob_private.hh | 12 ++++++++++++ qpdf/qpdf.testcov | 3 --- 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index e9f8bde..fb7491f 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -58,17 +58,6 @@ namespace std::shared_ptr config; }; - struct QPDFPageData - { - QPDFPageData(std::string const& filename, QPDF* qpdf, std::string const& range); - QPDFPageData(QPDFPageData const& other, int page); - - std::string filename; - QPDF* qpdf; - std::vector orig_pages; - std::vector selected_pages; - }; - class ProgressReporter final: public QPDFWriter::ProgressReporter { public: @@ -267,26 +256,6 @@ struct QPDFJob::PageNo int no{1}; }; -QPDFPageData::QPDFPageData(std::string const& filename, QPDF* qpdf, std::string const& range) : - filename(filename), - qpdf(qpdf), - orig_pages(qpdf->getAllPages()) -{ - try { - selected_pages = QUtil::parse_numrange(range.c_str(), QIntC::to_int(orig_pages.size())); - } catch (std::runtime_error& e) { - throw std::runtime_error("parsing numeric range for " + filename + ": " + e.what()); - } -} - -QPDFPageData::QPDFPageData(QPDFPageData const& other, int page) : - filename(other.filename), - qpdf(other.qpdf), - orig_pages(other.orig_pages) -{ - selected_pages.push_back(page); -} - QPDFJob::QPDFJob() : m(std::make_shared()) { @@ -2411,7 +2380,6 @@ QPDFJob::handlePageSpecs(QPDF& pdf) std::map page_spec_qpdfs; std::map page_spec_cfis; page_spec_qpdfs[m->infilename] = &pdf; - std::vector parsed_specs; std::map> copied_pages; for (auto& selection: m->selections) { if (!page_spec_qpdfs.contains(selection.filename)) { @@ -2425,7 +2393,6 @@ QPDFJob::handlePageSpecs(QPDF& pdf) auto password = selection.password; if (!m->encryption_file.empty() && password.empty() && selection.filename == m->encryption_file) { - QTC::TC("qpdf", "QPDFJob pages encryption password"); password = m->encryption_file_password; } doIfVerbose([&](Pipeline& v, std::string const& prefix) { @@ -2434,13 +2401,11 @@ QPDFJob::handlePageSpecs(QPDF& pdf) std::shared_ptr is; ClosedFileInputSource* cis = nullptr; if (!m->keep_files_open) { - QTC::TC("qpdf", "QPDFJob keep files open n"); - cis = new ClosedFileInputSource(selection.filename.c_str()); + cis = new ClosedFileInputSource(selection.filename.data()); is = std::shared_ptr(cis); cis->stayOpen(true); } else { - QTC::TC("qpdf", "QPDFJob keep files open y"); - FileInputSource* fis = new FileInputSource(selection.filename.c_str()); + FileInputSource* fis = new FileInputSource(selection.filename.data()); is = std::shared_ptr(fis); } std::unique_ptr qpdf_sp; @@ -2455,8 +2420,15 @@ QPDFJob::handlePageSpecs(QPDF& pdf) // Read original pages from the PDF, and parse the page range associated with this // occurrence of the file. - parsed_specs.emplace_back( - selection.filename, page_spec_qpdfs[selection.filename], selection.range); + selection.qpdf = page_spec_qpdfs[selection.filename]; + selection.orig_pages = selection.qpdf->getAllPages(); + try { + selection.selected_pages = QUtil::parse_numrange( + selection.range.data(), static_cast(selection.orig_pages.size())); + } catch (std::runtime_error& e) { + throw std::runtime_error( + "parsing numeric range for " + selection.filename + ": " + e.what()); + } } std::map remove_unreferenced; @@ -2491,16 +2463,17 @@ QPDFJob::handlePageSpecs(QPDF& pdf) } auto n_collate = m->collate.size(); - auto n_specs = parsed_specs.size(); + auto n_specs = m->selections.size(); if (!(n_collate == 0 || n_collate == 1 || n_collate == n_specs)) { usage( "--pages: if --collate has more than one value, it must have one value per page " "specification"); } + + std::vector new_specs; if (n_collate > 0 && n_specs > 1) { // Collate the pages by selecting one page from each spec in order. When a spec runs out of // pages, stop selecting from it. - std::vector new_parsed_specs; // Make sure we have a collate value for each spec. We have already checked that a non-empty // collate has either one value or one value per spec. for (auto i = n_collate; i < n_specs; ++i) { @@ -2511,20 +2484,20 @@ QPDFJob::handlePageSpecs(QPDF& pdf) while (got_pages) { got_pages = false; for (size_t i = 0; i < n_specs; ++i) { - QPDFPageData& page_data = parsed_specs.at(i); + auto& page_data = m->selections.at(i); for (size_t j = 0; j < m->collate.at(i); ++j) { if (cur_page.at(i) + j < page_data.selected_pages.size()) { got_pages = true; - new_parsed_specs.emplace_back( + new_specs.emplace_back( page_data, page_data.selected_pages.at(cur_page.at(i) + j)); } } cur_page.at(i) += m->collate.at(i); } } - parsed_specs = new_parsed_specs; } + std::vector& page_specs = new_specs.empty() ? m->selections : new_specs; // Add all the pages from all the files in the order specified. Keep track of any pages from the // original file that we are selecting. std::set selected_from_orig; @@ -2533,7 +2506,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf) int out_pageno = 0; auto& this_afdh = pdf.acroform(); std::set referenced_fields; - for (auto& page_data: parsed_specs) { + for (auto& page_data: page_specs) { ClosedFileInputSource* cis = nullptr; if (page_spec_cfis.contains(page_data.filename)) { cis = page_spec_cfis[page_data.filename]; diff --git a/libqpdf/qpdf/QPDFJob_private.hh b/libqpdf/qpdf/QPDFJob_private.hh index a2fd4b7..b23a18a 100644 --- a/libqpdf/qpdf/QPDFJob_private.hh +++ b/libqpdf/qpdf/QPDFJob_private.hh @@ -17,9 +17,21 @@ struct QPDFJob::Selection { } + Selection(QPDFJob::Selection const& other, int page) : + filename(other.filename), + // range and password are no longer required when this constructor is called. + qpdf(other.qpdf), + orig_pages(other.orig_pages), + selected_pages({page}) + { + } + std::string filename; std::string password; std::string range; + QPDF* qpdf; + std::vector orig_pages; + std::vector selected_pages; }; struct QPDFJob::RotationSpec diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 1c38d16..24dea97 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -188,7 +188,6 @@ QPDF insert foreign page 0 QPDFWriter copy use_aes 1 QPDFParser indirect without context 0 QPDFObjectHandle trailing data in parse 0 -QPDFJob pages encryption password 0 QPDFTokenizer EOF reading token 0 QPDFTokenizer EOF reading appendable token 0 QPDFWriter extra header text no newline 0 @@ -286,8 +285,6 @@ QPDFAcroFormDocumentHelper non-dictionary field 0 QPDFAcroFormDocumentHelper loop 0 QPDFAcroFormDocumentHelper field found 1 QPDFAcroFormDocumentHelper annotation found 1 -QPDFJob keep files open n 0 -QPDFJob keep files open y 0 QPDFJob automatically set keep files open 1 QPDFOutlineDocumentHelper string named dest 0 QPDFObjectHandle merge top type mismatch 0 -- libgit2 0.21.4