Allow --file and --range with --pages

Accept --file and --range as named parameters in additional to
allowing positional arguments. This is in preparation for adding
additional flags.

10 files changed, 119 insertions, 112 deletions

diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc
index 3bdb4f39..d82d6fc9 100644
--- a/libqpdf/QPDFJob.cc
+++ b/libqpdf/QPDFJob.cc
@@ -2342,6 +2342,9 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector<std::unique_ptr<QPDF>>& page_hea
         if (page_spec.filename == ".") {
             page_spec.filename = m->infilename.get();
         }
+        if (page_spec.range.empty()) {
+            page_spec.range = "1-z";
+        }
     }
 
     if (!m->keep_files_open_set) {
diff --git a/libqpdf/QPDFJob_argv.cc b/libqpdf/QPDFJob_argv.cc
index 737059f0..ea35342b 100644
--- a/libqpdf/QPDFJob_argv.cc
+++ b/libqpdf/QPDFJob_argv.cc
@@ -34,9 +34,10 @@ namespace
         std::shared_ptr<QPDFJob::UOConfig> c_uo;
         std::shared_ptr<QPDFJob::EncConfig> c_enc;
         std::vector<std::string> accumulated_args;
-        std::shared_ptr<char> pages_password{nullptr};
         std::string user_password;
         std::string owner_password;
+        bool called_pages_file{false};
+        bool called_pages_range{false};
         bool used_enc_password_args{false};
         bool gave_input{false};
         bool gave_output{false};
@@ -237,81 +238,43 @@ ArgParser::argPages()
 }
 
 void
-ArgParser::argPagesPassword(std::string const& parameter)
-{
-    if (this->pages_password) {
-        QTC::TC("qpdf", "QPDFJob duplicated pages password");
-        usage("--password already specified for this file");
-    }
-    if (this->accumulated_args.size() != 1) {
-        QTC::TC("qpdf", "QPDFJob misplaced pages password");
-        usage("in --pages, --password must immediately follow a file name");
-    }
-    this->pages_password = QUtil::make_shared_cstr(parameter);
-}
-
-void
 ArgParser::argPagesPositional(std::string const& arg)
 {
-    if (arg.empty()) {
-        if (this->accumulated_args.empty()) {
-            return;
-        }
-    } else {
-        this->accumulated_args.push_back(arg);
+    if (!called_pages_file) {
+        c_pages->file(arg);
+        called_pages_file = true;
+        return;
     }
-
-    std::string file = this->accumulated_args.at(0);
-    char const* range_p = nullptr;
-
-    size_t n_args = this->accumulated_args.size();
-    if (n_args >= 2) {
-        // will be copied before accumulated_args is cleared
-        range_p = this->accumulated_args.at(1).c_str();
+    if (called_pages_range) {
+        c_pages->file(arg);
+        called_pages_range = false;
+        return;
     }
-
-    // See if the user omitted the range entirely, in which case we assume "1-z".
-    std::string next_file;
-    if (range_p == nullptr) {
-        if (arg.empty()) {
-            // The filename or password was the last argument
-            QTC::TC("qpdf", "QPDFJob pages range omitted at end", this->pages_password ? 0 : 1);
+    // This could be a range or a file. Try parsing.
+    try {
+        QUtil::parse_numrange(arg.c_str(), 0);
+        c_pages->range(arg);
+        called_pages_range = true;
+    } catch (std::runtime_error& e1) {
+        // The range is invalid.  Let's see if it's a file.
+        if (arg == ".") {
+            // "." means the input file.
+            QTC::TC("qpdf", "QPDFJob pages range omitted with .");
+        } else if (QUtil::file_can_be_opened(arg.c_str())) {
+            QTC::TC("qpdf", "QPDFJob pages range omitted in middle");
+            // Yup, it's a file.
         } else {
-            // We need to accumulate some more arguments
-            return;
+            // Give the range error
+            usage(e1.what());
         }
-    } else {
-        try {
-            QUtil::parse_numrange(range_p, 0);
-        } catch (std::runtime_error& e1) {
-            // The range is invalid.  Let's see if it's a file.
-            if (strcmp(range_p, ".") == 0) {
-                // "." means the input file.
-                QTC::TC("qpdf", "QPDFJob pages range omitted with .");
-            } else if (QUtil::file_can_be_opened(range_p)) {
-                QTC::TC("qpdf", "QPDFJob pages range omitted in middle");
-                // Yup, it's a file.
-            } else {
-                // Give the range error
-                usage(e1.what());
-            }
-            next_file = range_p;
-            range_p = nullptr;
-        }
-    }
-    std::string range(range_p ? range_p : "1-z");
-    this->c_pages->pageSpec(file, range, this->pages_password.get());
-    this->accumulated_args.clear();
-    this->pages_password = nullptr;
-    if (!next_file.empty()) {
-        this->accumulated_args.push_back(next_file);
+        c_pages->file(arg);
+        called_pages_range = false;
     }
 }
 
 void
 ArgParser::argEndPages()
 {
-    argPagesPositional("");
     c_pages->endPages();
     c_pages = nullptr;
 }
diff --git a/libqpdf/QPDFJob_config.cc b/libqpdf/QPDFJob_config.cc
index db6c5948..c43cc82f 100644
--- a/libqpdf/QPDFJob_config.cc
+++ b/libqpdf/QPDFJob_config.cc
@@ -968,6 +968,45 @@ QPDFJob::PagesConfig::pageSpec(
     return this;
 }
 
+QPDFJob::PagesConfig*
+QPDFJob::PagesConfig::file(std::string const& arg)
+{
+    this->config->o.m->page_specs.emplace_back(arg, nullptr, "");
+    return this;
+}
+
+QPDFJob::PagesConfig*
+QPDFJob::PagesConfig::range(std::string const& arg)
+{
+    if (config->o.m->page_specs.empty()) {
+        QTC::TC("qpdf", "QPDFJob misplaced page range");
+        usage("in --range must follow a file name");
+    }
+    auto& last = config->o.m->page_specs.back();
+    if (!last.range.empty()) {
+        QTC::TC("qpdf", "QPDFJob duplicated range");
+        usage("--range already specified for this file");
+    }
+    last.range = arg;
+    return this;
+}
+
+QPDFJob::PagesConfig*
+QPDFJob::PagesConfig::password(std::string const& arg)
+{
+    if (config->o.m->page_specs.empty()) {
+        QTC::TC("qpdf", "QPDFJob misplaced pages password");
+        usage("in --pages, --password must follow a file name");
+    }
+    auto& last = config->o.m->page_specs.back();
+    if (last.password) {
+        QTC::TC("qpdf", "QPDFJob duplicated pages password");
+        usage("--password already specified for this file");
+    }
+    last.password = QUtil::make_shared_cstr(arg);
+    return this;
+}
+
 std::shared_ptr<QPDFJob::UOConfig>
 QPDFJob::Config::overlay()
 {
diff --git a/libqpdf/QPDFJob_json.cc b/libqpdf/QPDFJob_json.cc
index 754cb81b..5565ea93 100644
--- a/libqpdf/QPDFJob_json.cc
+++ b/libqpdf/QPDFJob_json.cc
@@ -467,25 +467,17 @@ Handlers::endPagesArray()
 void
 Handlers::beginPages(JSON j)
 {
-    std::string file;
-    std::string range("1-z");
-    std::string password;
     bool file_seen = false;
-    bool password_seen = false;
-    j.forEachDictItem([&](std::string const& key, JSON value) {
+    j.forEachDictItem([&](std::string const& key, JSON const& value) {
         if (key == "file") {
-            file_seen = value.getString(file);
-        } else if (key == "range") {
-            value.getString(range);
-        } else if (key == "password") {
-            password_seen = value.getString(password);
+            std::string v;
+            file_seen = value.getString(v);
         }
     });
     if (!file_seen) {
         QTC::TC("qpdf", "QPDFJob json pages no file");
         usage("file is required in page specification");
     }
-    this->c_pages->pageSpec(file, range, password_seen ? password.c_str() : nullptr);
 }
 
 void
@@ -495,24 +487,9 @@ Handlers::endPages()
 }
 
 void
-Handlers::setupPagesFile()
-{
-    // handled in beginPages
-    ignoreItem();
-}
-
-void
 Handlers::setupPagesPassword()
 {
-    // handled in beginPages
-    ignoreItem();
-}
-
-void
-Handlers::setupPagesRange()
-{
-    // handled in beginPages
-    ignoreItem();
+    addParameter([this](char const* p) { c_pages->password(p); });
 }
 
 void
diff --git a/libqpdf/qpdf/auto_job_decl.hh b/libqpdf/qpdf/auto_job_decl.hh
index 38f7cb32..d70af259 100644
--- a/libqpdf/qpdf/auto_job_decl.hh
+++ b/libqpdf/qpdf/auto_job_decl.hh
@@ -31,7 +31,6 @@ void argReplaceInput();
 void argSetPageLabels();
 void argUnderlay();
 void argPagesPositional(std::string const&);
-void argPagesPassword(std::string const&);
 void argEndPages();
 void argEncPositional(std::string const&);
 void argEncUserPassword(std::string const&);
diff --git a/libqpdf/qpdf/auto_job_help.hh b/libqpdf/qpdf/auto_job_help.hh
index 35c1a0d2..a3572aed 100644
--- a/libqpdf/qpdf/auto_job_help.hh
+++ b/libqpdf/qpdf/auto_job_help.hh
@@ -311,10 +311,24 @@ static void add_help_4(QPDFArgParser& ap)
 ap.addHelpTopic("modification", "change parts of the PDF", R"(Modification options make systematic changes to certain parts of
 the PDF, causing the PDF to render differently from the original.
 )");
-ap.addOptionHelp("--pages", "modification", "begin page selection", R"(--pages file [--password=password] [page-range] [...] --
+ap.addOptionHelp("--pages", "modification", "begin page selection", R"(--pages [--file=]file [options] [...] --
 
 Run qpdf --help=page-selection for details.
 )");
+ap.addOptionHelp("--file", "modification", "source for pages", R"(--file=file
+
+Specify the file for the current page operation. This is used
+with --pages, --overlay, and --underlay and appears between the
+option and the terminating --. Run qpdf --help=page-selection
+for details.
+)");
+ap.addOptionHelp("--range", "modification", "page range", R"(--range=numeric-range
+
+Specify the page range for the current page operation with
+--pages. If omitted, all pages are selected. This is used
+with --pages and appears between --pages and --. Run
+qpdf --help=page-selection for details.
+)");
 ap.addOptionHelp("--collate", "modification", "collate with --pages", R"(--collate[=n[,m,...]]
 
 Collate rather than concatenate pages specified with --pages.
@@ -437,6 +451,9 @@ iv, then the remaining pages with Arabic numerals starting with
 1 and continuing sequentially until the end of the document. For
 additional examples, please consult the manual.
 )");
+}
+static void add_help_5(QPDFArgParser& ap)
+{
 ap.addHelpTopic("encryption", "create encrypted files", R"(Create encrypted files. Usage:
 
 --encrypt \
@@ -520,9 +537,6 @@ ap.addOptionHelp("--user-password", "encryption", "specify user password", R"(--
 
 Set the user password of the encrypted file.
 )");
-}
-static void add_help_5(QPDFArgParser& ap)
-{
 ap.addOptionHelp("--owner-password", "encryption", "specify owner password", R"(--owner-password=owner-password
 
 Set the owner password of the encrypted file.
@@ -620,12 +634,24 @@ should not be used except for compatibility testing.
 )");
 ap.addHelpTopic("page-selection", "select pages from one or more files", R"(Use the --pages option to select pages from multiple files. Usage:
 
+qpdf in.pdf --pages --file=input-file \
+    [--range=page-range] [--password=password] [...] -- out.pdf
+
+OR
+
 qpdf in.pdf --pages input-file [--password=password] [page-range] \
     [...] -- out.pdf
 
 Between --pages and the -- that terminates pages option, repeat
 the following:
 
+--file=filename [--range=page-range] [--password=password] [options]
+
+For compatibility, the file and range can be specified
+positionally. qpdf versions prior to 11.9.0
+require --password=password to immediately follow the filename. In
+the older syntax, repeat the following:
+
 filename [--password=password] [page-range]
 
 Document-level information, such as outlines, tags, etc., is taken
@@ -654,7 +680,7 @@ Examples:
   information from in.pdf is retained. Note the use of "." to refer
   to in.pdf.
 
-  qpdf in.pdf --pages . a.pdf b.pdf:even -- out.pdf
+  qpdf in.pdf --pages . a.pdf b.pdf 1-z:even -- out.pdf
 
 - Take all the pages from a.pdf, all the pages from b.pdf in
   reverse, and only pages 3 and 6 from c.pdf and write the result
@@ -687,6 +713,9 @@ those pages to be repeated after the original pages are exhausted.
 
 Run qpdf --help=page-ranges for help with page ranges.
 )");
+}
+static void add_help_6(QPDFArgParser& ap)
+{
 ap.addOptionHelp("--to", "overlay-underlay", "destination pages for underlay/overlay", R"(--to=page-range
 
 Specify the range of pages in the primary output to apply
@@ -700,9 +729,6 @@ the destination pages. See qpdf --help=page-ranges for help
 with the page range syntax. The page range may be omitted
 if --repeat is used.
 )");
-}
-static void add_help_6(QPDFArgParser& ap)
-{
 ap.addOptionHelp("--repeat", "overlay-underlay", "overlay/underlay pages to repeat", R"(--repeat=page-range
 
 Specify pages from the overlay/underlay that are repeated after
@@ -801,6 +827,9 @@ ap.addHelpTopic("inspection", "inspect PDF files", R"(These options provide tool
 the options in this section are specified, no output file may be
 given.
 )");
+}
+static void add_help_7(QPDFArgParser& ap)
+{
 ap.addOptionHelp("--is-encrypted", "inspection", "silently test whether a file is encrypted", R"(Silently exit with a code indicating the file's encryption status:
 
 0: the file is encrypted
@@ -817,9 +846,6 @@ ap.addOptionHelp("--requires-password", "inspection", "silently test a file's pa
 2: the file is not encrypted
 3: the file is encrypted, and correct password (if any) has been supplied
 )");
-}
-static void add_help_7(QPDFArgParser& ap)
-{
 ap.addOptionHelp("--check", "inspection", "partially check whether PDF is valid", R"(Check the structure of the PDF file as well as a number of other
 aspects of the file, and write information about the file to
 standard output. Note that qpdf does not perform any validation
@@ -894,6 +920,9 @@ Describe the format of the JSON output by writing to standard
 output a JSON object with the same keys and with values
 containing descriptive text.
 )");
+}
+static void add_help_8(QPDFArgParser& ap)
+{
 ap.addOptionHelp("--json-key", "json", "limit which keys are in JSON output", R"(--json-key=key
 
 This option is repeatable. If given, only the specified
@@ -907,9 +936,6 @@ This option is repeatable. If given, only specified objects will
 be shown in the "objects" key of the JSON output. Otherwise, all
 objects will be shown.
 )");
-}
-static void add_help_8(QPDFArgParser& ap)
-{
 ap.addOptionHelp("--json-stream-data", "json", "how to handle streams in json output", R"(--json-stream-data={none|inline|file}
 
 When used with --json, this option controls whether streams in
diff --git a/libqpdf/qpdf/auto_job_init.hh b/libqpdf/qpdf/auto_job_init.hh
index 6ac88762..401406db 100644
--- a/libqpdf/qpdf/auto_job_init.hh
+++ b/libqpdf/qpdf/auto_job_init.hh
@@ -127,7 +127,9 @@ this->ap.addChoices("json", [this](std::string const& x){c_main->json(x);}, fals
 this->ap.addChoices("json-output", [this](std::string const& x){c_main->jsonOutput(x);}, false, json_output_choices);
 this->ap.registerOptionTable("pages", b(&ArgParser::argEndPages));
 this->ap.addPositional(p(&ArgParser::argPagesPositional));
-this->ap.addRequiredParameter("password", p(&ArgParser::argPagesPassword), "password");
+this->ap.addRequiredParameter("file", [this](std::string const& x){c_pages->file(x);}, "file");
+this->ap.addRequiredParameter("range", [this](std::string const& x){c_pages->range(x);}, "page-range");
+this->ap.addRequiredParameter("password", [this](std::string const& x){c_pages->password(x);}, "password");
 this->ap.registerOptionTable("encryption", b(&ArgParser::argEndEncryption));
 this->ap.addPositional(p(&ArgParser::argEncPositional));
 this->ap.addRequiredParameter("user-password", p(&ArgParser::argEncUserPassword), "user_password");
diff --git a/libqpdf/qpdf/auto_job_json_decl.hh b/libqpdf/qpdf/auto_job_json_decl.hh
index f3576494..b2ae4c1e 100644
--- a/libqpdf/qpdf/auto_job_json_decl.hh
+++ b/libqpdf/qpdf/auto_job_json_decl.hh
@@ -41,9 +41,7 @@ void beginPagesArray(JSON);
 void endPagesArray();
 void beginPages(JSON);
 void endPages();
-void setupPagesFile();
 void setupPagesPassword();
-void setupPagesRange();
 void beginSetPageLabelsArray(JSON);
 void endSetPageLabelsArray();
 void setupSetPageLabels();
diff --git a/libqpdf/qpdf/auto_job_json_init.hh b/libqpdf/qpdf/auto_job_json_init.hh
index 22621271..fb4d93b9 100644
--- a/libqpdf/qpdf/auto_job_json_init.hh
+++ b/libqpdf/qpdf/auto_job_json_init.hh
@@ -402,13 +402,13 @@ pushKey("pages");
 beginArray(bindJSON(&Handlers::beginPagesArray), bindBare(&Handlers::endPagesArray)); // .pages[]
 beginDict(bindJSON(&Handlers::beginPages), bindBare(&Handlers::endPages)); // .pages
 pushKey("file");
-setupPagesFile();
+addParameter([this](std::string const& p) { c_pages->file(p); });
 popHandler(); // key: file
 pushKey("password");
 setupPagesPassword();
 popHandler(); // key: password
 pushKey("range");
-setupPagesRange();
+addParameter([this](std::string const& p) { c_pages->range(p); });
 popHandler(); // key: range
 popHandler(); // array: .pages[]
 popHandler(); // key: pages
diff --git a/libqpdf/qpdf/auto_job_schema.hh b/libqpdf/qpdf/auto_job_schema.hh
index f7b8590c..bb4b04ea 100644
--- a/libqpdf/qpdf/auto_job_schema.hh
+++ b/libqpdf/qpdf/auto_job_schema.hh
@@ -140,7 +140,7 @@ static constexpr char const* JOB_SCHEMA_DATA = R"({
   "optimizeImages": "use efficient compression for images",
   "pages": [
     {
-      "file": "source for for pages",
+      "file": "source for pages",
       "password": "password for encrypted file",
       "range": "page range"
     }