QPDFArgParser: handle optional choices including help

Handle optional choices in addition to required choices. Refactor the
way help options are added to completion to make it work with optional
help choices.

generate_auto_job

@@ -119,7 +119,7 @@ class Main:
             self.check_keys('top', o, set(
                 ['table', 'prefix', 'bare', 'positional',
                  'optional_parameter', 'required_parameter',
-                 'required_choices', 'from_table']))
+                 'required_choices', 'optional_choices', 'from_table']))
 
     def to_identifier(self, label, prefix, const):
         identifier = re.sub(r'[^a-zA-Z0-9]', '_', label)
@@ -157,6 +157,9 @@ class Main:
             for i in o.get('required_choices', {}):
                 identifier = self.to_identifier(i, prefix, False)
                 print(f'void {identifier}(char *);', file=f)
+            for i in o.get('optional_choices', {}):
+                identifier = self.to_identifier(i, prefix, False)
+                print(f'void {identifier}(char *);', file=f)
             if table not in ('main', 'help'):
                 identifier = self.to_identifier(table, 'argEnd', False)
                 print(f'void {identifier}();', file=f)
@@ -204,9 +207,14 @@ class Main:
                       f', "{v}");', file=f)
             for k, v in o.get('required_choices', {}).items():
                 identifier = self.to_identifier(k, prefix, False)
-                print(f'this->ap.addRequiredChoices("{k}", '
+                print(f'this->ap.addChoices("{k}", '
                       f'p(&ArgParser::{identifier})'
-                      f', {v}_choices);', file=f)
+                      f', true, {v}_choices);', file=f)
+            for k, v in o.get('optional_choices', {}).items():
+                identifier = self.to_identifier(k, prefix, False)
+                print(f'this->ap.addChoices("{k}", '
+                      f'p(&ArgParser::{identifier})'
+                      f', false, {v}_choices);', file=f)
         for o in data['options']:
             table = o['table']
             if 'from_table' not in o:

include/qpdf/QPDFArgParser.hh

@@ -115,13 +115,14 @@ class QPDFArgParser
     QPDF_DLL
     void addOptionalParameter(std::string const& arg, param_arg_handler_t);
     QPDF_DLL
-    void addRequiredChoices(
+    void addChoices(
-        std::string const& arg, param_arg_handler_t, char const** choices);
+        std::string const& arg, param_arg_handler_t,
-    QPDF_DLL
+        bool required, char const** choices);
 
     // If an option is shared among multiple tables and uses identical
     // handlers, you can just copy it instead of repeating the
     // registration call.
+    QPDF_DLL
     void copyFromOtherTable(std::string const& arg,
                             std::string const& other_table);
 
@@ -179,7 +180,7 @@ class QPDFArgParser
         bare_arg_handler_t bare_arg_handler;
         param_arg_handler_t param_arg_handler;
     };
-    friend struct OptionEntry;
+    typedef std::map<std::string, OptionEntry> option_table_t;
 
     OptionEntry& registerArg(std::string const& arg);
 
@@ -187,18 +188,20 @@ class QPDFArgParser
 
     void argCompletionBash();
     void argCompletionZsh();
+    void argHelp(char*);
 
     void checkCompletion();
     void handleArgFileArguments();
     void handleBashArguments();
     void readArgsFromFile(char const* filename);
     void doFinalChecks();
-    void addOptionsToCompletions();
+    void addOptionsToCompletions(option_table_t&);
-    void addChoicesToCompletions(std::string const&, std::string const&);
+    void addChoicesToCompletions(
+        option_table_t&, std::string const&, std::string const&);
+    void insertCompletions(
+        option_table_t&, std::string const&, std::string const&);
     void handleCompletion();
 
-    typedef std::map<std::string, OptionEntry> option_table_t;
-
     class Members
     {
         friend class QPDFArgParser;

job.sums

@@ -1,5 +1,5 @@
 # Generated by generate_auto_job
-generate_auto_job 575569edf2ab0036ed7f810bf506968c73c9209a64ec0ae2ed959f0426291447
+generate_auto_job 019081046f1bc19f498134eae00344ecfc65b4e52442ee5f1bc80bff99689443
-job.yml 5de5f1cd3f998274ed4aafa234e61b726a8f96157148ad463970439a96e897bd
+job.yml 8e5b3fe5a6abea64a5a33977c440a7ac9ecc4516d2a131ed38fd4bc1a73445d7
-libqpdf/qpdf/auto_job_decl.hh fca37543c1a2b7f675374e23b1ab34b30a7f5f2d843c53d4bc7e9a12bf4c3615
+libqpdf/qpdf/auto_job_decl.hh 97395ecbe590b23ae04d6cce2080dbd0e998917ff5eeaa5c6aafa91041d3cd6a
-libqpdf/qpdf/auto_job_init.hh 7d7dfe96d4da765b8defb646058027584ea8924c604c558402aa7f2d2e61a005
+libqpdf/qpdf/auto_job_init.hh 465bf46769559ceb77110d1b9d3293ba9b3595850b49848c31aeabd10aadb4ad

job.yml

@@ -51,7 +51,6 @@ choices:
 options:
   - table: help
     bare:
-      - help
       - version
       - copyright
       - json-help

libqpdf/QPDFArgParser.cc

@@ -35,6 +35,9 @@ QPDFArgParser::QPDFArgParser(int argc, char* argv[], char const* progname_env) :
     m(new Members(argc, argv, progname_env))
 {
     selectHelpOptionTable();
+    char const* help_choices[] = {"all", 0};
+    addChoices(
+        "help", bindParam(&QPDFArgParser::argHelp, this), false, help_choices);
     addBare("completion-bash",
             std::bind(std::mem_fn(&QPDFArgParser::argCompletionBash), this));
     addBare("completion-zsh",
@@ -139,13 +142,14 @@ QPDFArgParser::addOptionalParameter(
 }
 
 void
-QPDFArgParser::addRequiredChoices(
+QPDFArgParser::addChoices(
     std::string const& arg,
     param_arg_handler_t handler,
+    bool required,
     char const** choices)
 {
     OptionEntry& oe = registerArg(arg);
-    oe.parameter_needed = true;
+    oe.parameter_needed = required;
     oe.param_arg_handler = handler;
     for (char const** i = choices; *i; ++i)
     {
@@ -253,6 +257,12 @@ QPDFArgParser::argCompletionZsh()
     completionCommon(true);
 }
 
+void
+QPDFArgParser::argHelp(char*)
+{
+    // QXXXQ
+}
+
 void
 QPDFArgParser::handleArgFileArguments()
 {
@@ -624,10 +634,9 @@ QPDFArgParser::parseArgs()
         }
 
         OptionEntry& oe = oep->second;
-        if ((oe.parameter_needed && (0 == parameter)) ||
+        if ((oe.parameter_needed && (nullptr == parameter)) ||
-            ((! oe.choices.empty() &&
+            ((! oe.choices.empty() && (nullptr != parameter) &&
-              ((0 == parameter) ||
+              (0 == oe.choices.count(parameter)))))
-               (0 == oe.choices.count(parameter))))))
         {
             std::string message =
                 "--" + arg_s + " must be given as --" + arg_s + "=";
@@ -708,12 +717,13 @@ QPDFArgParser::doFinalChecks()
 }
 
 void
-QPDFArgParser::addChoicesToCompletions(std::string const& option,
+QPDFArgParser::addChoicesToCompletions(option_table_t& option_table,
+                                       std::string const& option,
                                        std::string const& extra_prefix)
 {
-    if (this->m->option_table->count(option) != 0)
+    if (option_table.count(option) != 0)
     {
-        OptionEntry& oe = (*this->m->option_table)[option];
+        OptionEntry& oe = option_table[option];
         for (std::set<std::string>::iterator iter = oe.choices.begin();
              iter != oe.choices.end(); ++iter)
         {
@@ -724,18 +734,16 @@ QPDFArgParser::addChoicesToCompletions(std::string const& option,
 }
 
 void
-QPDFArgParser::addOptionsToCompletions()
+QPDFArgParser::addOptionsToCompletions(option_table_t& option_table)
 {
-    for (std::map<std::string, OptionEntry>::iterator iter =
+    for (auto& iter: option_table)
-             this->m->option_table->begin();
-         iter != this->m->option_table->end(); ++iter)
     {
-        std::string const& arg = (*iter).first;
+        std::string const& arg = iter.first;
         if (arg == "--")
         {
             continue;
         }
-        OptionEntry& oe = (*iter).second;
+        OptionEntry& oe = iter.second;
         std::string base = "--" + arg;
         if (oe.param_arg_handler)
         {
@@ -743,7 +751,7 @@ QPDFArgParser::addOptionsToCompletions()
             {
                 // zsh doesn't treat = as a word separator, so add all
                 // the options so we don't get a space after the =.
-                addChoicesToCompletions(arg, base + "=");
+                addChoicesToCompletions(option_table, arg, base + "=");
             }
             this->m->completions.insert(base + "=");
         }
@@ -754,6 +762,22 @@ QPDFArgParser::addOptionsToCompletions()
     }
 }
 
+void
+QPDFArgParser::insertCompletions(option_table_t& option_table,
+                                 std::string const& choice_option,
+                                 std::string const& extra_prefix)
+{
+    if (! choice_option.empty())
+    {
+        addChoicesToCompletions(option_table, choice_option, extra_prefix);
+    }
+    else if ((! this->m->bash_cur.empty()) &&
+             (this->m->bash_cur.at(0) == '-'))
+    {
+        addOptionsToCompletions(option_table);
+    }
+}
+
 void
 QPDFArgParser::handleCompletion()
 {
@@ -795,29 +819,17 @@ QPDFArgParser::handleCompletion()
                 }
             }
         }
-        if (! choice_option.empty())
+        if (this->m->zsh_completion && (! choice_option.empty()))
         {
-            if (this->m->zsh_completion)
+            // zsh wants --option=choice rather than just choice
-            {
+            extra_prefix = "--" + choice_option + "=";
-                // zsh wants --option=choice rather than just choice
-                extra_prefix = "--" + choice_option + "=";
-            }
-            addChoicesToCompletions(choice_option, extra_prefix);
         }
-        else if ((! this->m->bash_cur.empty()) &&
+        insertCompletions(*this->m->option_table, choice_option, extra_prefix);
-                 (this->m->bash_cur.at(0) == '-'))
+        if (this->m->argc == 1)
         {
-            addOptionsToCompletions();
+            // Help options are valid only by themselves.
-            if (this->m->argc == 1)
+            insertCompletions(
-            {
+                this->m->help_option_table, choice_option, extra_prefix);
-                // Help options are valid only by themselves.
-                for (std::map<std::string, OptionEntry>::iterator iter =
-                         this->m->help_option_table.begin();
-                     iter != this->m->help_option_table.end(); ++iter)
-                {
-                    this->m->completions.insert("--" + (*iter).first);
-                }
-            }
         }
     }
     std::string prefix = extra_prefix + this->m->bash_cur;

libqpdf/QPDFJob_argv.cc

@@ -127,9 +127,11 @@ ArgParser::argCopyright()
         << std::endl;
 }
 
+#if 0
 void
 ArgParser::argHelp()
 {
+    // QXXXQ
     std::cout
         // 12345678901234567890123456789012345678901234567890123456789012345678901234567890
         << "Usage: qpdf [options] {infile | --empty} [page_selection_options] outfile\n"
@@ -630,6 +632,7 @@ ArgParser::argHelp()
         << "qpdf to completely ignore warnings.  qpdf does not use exit status 1,\n"
         << "since that is used by the shell if it can't execute qpdf.\n";
 }
+#endif
 
 void
 ArgParser::argJsonHelp()

libqpdf/qpdf/auto_job_decl.hh

@@ -12,7 +12,6 @@ static constexpr char const* O_UNDERLAY_OVERLAY = "underlay/overlay";
 static constexpr char const* O_ATTACHMENT = "attachment";
 static constexpr char const* O_COPY_ATTACHMENT = "copy attachment";
 
-void argHelp();
 void argVersion();
 void argCopyright();
 void argJsonHelp();

libqpdf/qpdf/auto_job_init.hh

@@ -22,7 +22,6 @@ char const* print128_choices[] = {"full", "low", "none", 0};
 char const* modify128_choices[] = {"all", "annotate", "form", "assembly", "none", 0};
 
 this->ap.selectHelpOptionTable();
-this->ap.addBare("help", b(&ArgParser::argHelp));
 this->ap.addBare("version", b(&ArgParser::argVersion));
 this->ap.addBare("copyright", b(&ArgParser::argCopyright));
 this->ap.addBare("json-help", b(&ArgParser::argJsonHelp));
@@ -99,38 +98,38 @@ this->ap.addRequiredParameter("remove-attachment", p(&ArgParser::argRemoveAttach
 this->ap.addRequiredParameter("rotate", p(&ArgParser::argRotate), "[+|-]angle");
 this->ap.addRequiredParameter("show-attachment", p(&ArgParser::argShowAttachment), "attachment");
 this->ap.addRequiredParameter("show-object", p(&ArgParser::argShowObject), "trailer");
-this->ap.addRequiredChoices("compress-streams", p(&ArgParser::argCompressStreams), yn_choices);
+this->ap.addChoices("compress-streams", p(&ArgParser::argCompressStreams), true, yn_choices);
-this->ap.addRequiredChoices("decode-level", p(&ArgParser::argDecodeLevel), decode_level_choices);
+this->ap.addChoices("decode-level", p(&ArgParser::argDecodeLevel), true, decode_level_choices);
-this->ap.addRequiredChoices("flatten-annotations", p(&ArgParser::argFlattenAnnotations), flatten_choices);
+this->ap.addChoices("flatten-annotations", p(&ArgParser::argFlattenAnnotations), true, flatten_choices);
-this->ap.addRequiredChoices("json-key", p(&ArgParser::argJsonKey), json_key_choices);
+this->ap.addChoices("json-key", p(&ArgParser::argJsonKey), true, json_key_choices);
-this->ap.addRequiredChoices("keep-files-open", p(&ArgParser::argKeepFilesOpen), yn_choices);
+this->ap.addChoices("keep-files-open", p(&ArgParser::argKeepFilesOpen), true, yn_choices);
-this->ap.addRequiredChoices("normalize-content", p(&ArgParser::argNormalizeContent), yn_choices);
+this->ap.addChoices("normalize-content", p(&ArgParser::argNormalizeContent), true, yn_choices);
-this->ap.addRequiredChoices("object-streams", p(&ArgParser::argObjectStreams), object_streams_choices);
+this->ap.addChoices("object-streams", p(&ArgParser::argObjectStreams), true, object_streams_choices);
-this->ap.addRequiredChoices("password-mode", p(&ArgParser::argPasswordMode), password_mode_choices);
+this->ap.addChoices("password-mode", p(&ArgParser::argPasswordMode), true, password_mode_choices);
-this->ap.addRequiredChoices("remove-unreferenced-resources", p(&ArgParser::argRemoveUnreferencedResources), remove_unref_choices);
+this->ap.addChoices("remove-unreferenced-resources", p(&ArgParser::argRemoveUnreferencedResources), true, remove_unref_choices);
-this->ap.addRequiredChoices("stream-data", p(&ArgParser::argStreamData), stream_data_choices);
+this->ap.addChoices("stream-data", p(&ArgParser::argStreamData), true, stream_data_choices);
 this->ap.registerOptionTable("pages", b(&ArgParser::argEndPages));
 this->ap.addPositional(p(&ArgParser::argPagesPositional));
 this->ap.addRequiredParameter("password", p(&ArgParser::argPagesPassword), "password");
 this->ap.registerOptionTable("encryption", b(&ArgParser::argEndEncryption));
 this->ap.addPositional(p(&ArgParser::argEncPositional));
 this->ap.registerOptionTable("40-bit encryption", b(&ArgParser::argEnd40BitEncryption));
-this->ap.addRequiredChoices("extract", p(&ArgParser::argEnc40Extract), yn_choices);
+this->ap.addChoices("extract", p(&ArgParser::argEnc40Extract), true, yn_choices);
-this->ap.addRequiredChoices("annotate", p(&ArgParser::argEnc40Annotate), yn_choices);
+this->ap.addChoices("annotate", p(&ArgParser::argEnc40Annotate), true, yn_choices);
-this->ap.addRequiredChoices("print", p(&ArgParser::argEnc40Print), yn_choices);
+this->ap.addChoices("print", p(&ArgParser::argEnc40Print), true, yn_choices);
-this->ap.addRequiredChoices("modify", p(&ArgParser::argEnc40Modify), yn_choices);
+this->ap.addChoices("modify", p(&ArgParser::argEnc40Modify), true, yn_choices);
 this->ap.registerOptionTable("128-bit encryption", b(&ArgParser::argEnd128BitEncryption));
 this->ap.addBare("cleartext-metadata", b(&ArgParser::argEnc128CleartextMetadata));
 this->ap.addBare("force-V4", b(&ArgParser::argEnc128ForceV4));
-this->ap.addRequiredChoices("accessibility", p(&ArgParser::argEnc128Accessibility), yn_choices);
+this->ap.addChoices("accessibility", p(&ArgParser::argEnc128Accessibility), true, yn_choices);
-this->ap.addRequiredChoices("extract", p(&ArgParser::argEnc128Extract), yn_choices);
+this->ap.addChoices("extract", p(&ArgParser::argEnc128Extract), true, yn_choices);
-this->ap.addRequiredChoices("print", p(&ArgParser::argEnc128Print), print128_choices);
+this->ap.addChoices("print", p(&ArgParser::argEnc128Print), true, print128_choices);
-this->ap.addRequiredChoices("assemble", p(&ArgParser::argEnc128Assemble), yn_choices);
+this->ap.addChoices("assemble", p(&ArgParser::argEnc128Assemble), true, yn_choices);
-this->ap.addRequiredChoices("annotate", p(&ArgParser::argEnc128Annotate), yn_choices);
+this->ap.addChoices("annotate", p(&ArgParser::argEnc128Annotate), true, yn_choices);
-this->ap.addRequiredChoices("form", p(&ArgParser::argEnc128Form), yn_choices);
+this->ap.addChoices("form", p(&ArgParser::argEnc128Form), true, yn_choices);
-this->ap.addRequiredChoices("modify-other", p(&ArgParser::argEnc128ModifyOther), yn_choices);
+this->ap.addChoices("modify-other", p(&ArgParser::argEnc128ModifyOther), true, yn_choices);
-this->ap.addRequiredChoices("modify", p(&ArgParser::argEnc128Modify), modify128_choices);
+this->ap.addChoices("modify", p(&ArgParser::argEnc128Modify), true, modify128_choices);
-this->ap.addRequiredChoices("use-aes", p(&ArgParser::argEnc128UseAes), yn_choices);
+this->ap.addChoices("use-aes", p(&ArgParser::argEnc128UseAes), true, yn_choices);
 this->ap.registerOptionTable("256-bit encryption", b(&ArgParser::argEnd256BitEncryption));
 this->ap.addBare("force-R5", b(&ArgParser::argEnc256ForceR5));
 this->ap.addBare("allow-insecure", b(&ArgParser::argEnc256AllowInsecure));

libtests/arg_parser.cc

@@ -51,7 +51,7 @@ ArgParser::initOptions()
     ap.addRequiredParameter("salad", p(&ArgParser::handleSalad), "tossed");
     ap.addOptionalParameter("moo", p(&ArgParser::handleMoo));
     char const* choices[] = {"pig", "boar", "sow", 0};
-    ap.addRequiredChoices("oink", p(&ArgParser::handleOink), choices);
+    ap.addChoices("oink", p(&ArgParser::handleOink), true, choices);
     ap.selectHelpOptionTable();
     ap.addBare("version", [this](){ output("3.14159"); });
     ap.selectMainOptionTable();

libtests/qtest/arg_parser/completion-top-arg-zsh.out

@@ -1,5 +1,8 @@
 --baaa
 --completion-zsh
+--help
+--help=
+--help=all
 --moo
 --moo=
 --oink=