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.

10 files changed, 104 insertions, 78 deletions

diff --git a/generate_auto_job b/generate_auto_job
index 7e70d29c..2dc51105 100755
--- a/generate_auto_job
+++ b/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', 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', {v}_choices);', file=f)
+                      f', false, {v}_choices);', file=f)
         for o in data['options']:
             table = o['table']
             if 'from_table' not in o:
diff --git a/include/qpdf/QPDFArgParser.hh b/include/qpdf/QPDFArgParser.hh
index 7054a706..ea51ca67 100644
--- a/include/qpdf/QPDFArgParser.hh
+++ b/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(
-        std::string const& arg, param_arg_handler_t, char const** choices);
-    QPDF_DLL
+    void addChoices(
+        std::string const& arg, param_arg_handler_t,
+        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 addChoicesToCompletions(std::string const&, std::string const&);
+    void addOptionsToCompletions(option_table_t&);
+    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;
diff --git a/job.sums b/job.sums
index 4429c717..43c8b8b5 100644
--- a/job.sums
+++ b/job.sums
@@ -1,5 +1,5 @@
 # Generated by generate_auto_job
-generate_auto_job 575569edf2ab0036ed7f810bf506968c73c9209a64ec0ae2ed959f0426291447
-job.yml 5de5f1cd3f998274ed4aafa234e61b726a8f96157148ad463970439a96e897bd
-libqpdf/qpdf/auto_job_decl.hh fca37543c1a2b7f675374e23b1ab34b30a7f5f2d843c53d4bc7e9a12bf4c3615
-libqpdf/qpdf/auto_job_init.hh 7d7dfe96d4da765b8defb646058027584ea8924c604c558402aa7f2d2e61a005
+generate_auto_job 019081046f1bc19f498134eae00344ecfc65b4e52442ee5f1bc80bff99689443
+job.yml 8e5b3fe5a6abea64a5a33977c440a7ac9ecc4516d2a131ed38fd4bc1a73445d7
+libqpdf/qpdf/auto_job_decl.hh 97395ecbe590b23ae04d6cce2080dbd0e998917ff5eeaa5c6aafa91041d3cd6a
+libqpdf/qpdf/auto_job_init.hh 465bf46769559ceb77110d1b9d3293ba9b3595850b49848c31aeabd10aadb4ad
diff --git a/job.yml b/job.yml
index f432bc3d..7acb2d8a 100644
--- a/job.yml
+++ b/job.yml
@@ -51,7 +51,6 @@ choices:
 options:
   - table: help
     bare:
-      - help
       - version
       - copyright
       - json-help
diff --git a/libqpdf/QPDFArgParser.cc b/libqpdf/QPDFArgParser.cc
index a24b00eb..f32b8759 100644
--- a/libqpdf/QPDFArgParser.cc
+++ b/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)
     {
@@ -254,6 +258,12 @@ QPDFArgParser::argCompletionZsh()
 }
 
 void
+QPDFArgParser::argHelp(char*)
+{
+    // QXXXQ
+}
+
+void
 QPDFArgParser::handleArgFileArguments()
 {
     // Support reading arguments from files. Create a new argv. Ensure
@@ -624,10 +634,9 @@ QPDFArgParser::parseArgs()
         }
 
         OptionEntry& oe = oep->second;
-        if ((oe.parameter_needed && (0 == parameter)) ||
-            ((! oe.choices.empty() &&
-              ((0 == parameter) ||
-               (0 == oe.choices.count(parameter))))))
+        if ((oe.parameter_needed && (nullptr == parameter)) ||
+            ((! oe.choices.empty() && (nullptr != 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 =
-             this->m->option_table->begin();
-         iter != this->m->option_table->end(); ++iter)
+    for (auto& iter: option_table)
     {
-        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 + "=");
         }
@@ -755,6 +763,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()
 {
     std::string extra_prefix;
@@ -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 + "=";
-            }
-            addChoicesToCompletions(choice_option, extra_prefix);
+            // zsh wants --option=choice rather than just choice
+            extra_prefix = "--" + choice_option + "=";
         }
-        else if ((! this->m->bash_cur.empty()) &&
-                 (this->m->bash_cur.at(0) == '-'))
+        insertCompletions(*this->m->option_table, choice_option, extra_prefix);
+        if (this->m->argc == 1)
         {
-            addOptionsToCompletions();
-            if (this->m->argc == 1)
-            {
-                // 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);
-                }
-            }
+            // Help options are valid only by themselves.
+            insertCompletions(
+                this->m->help_option_table, choice_option, extra_prefix);
         }
     }
     std::string prefix = extra_prefix + this->m->bash_cur;
diff --git a/libqpdf/QPDFJob_argv.cc b/libqpdf/QPDFJob_argv.cc
index 0cf12cd4..9b678257 100644
--- a/libqpdf/QPDFJob_argv.cc
+++ b/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()
diff --git a/libqpdf/qpdf/auto_job_decl.hh b/libqpdf/qpdf/auto_job_decl.hh
index 076a8f68..1f2b4263 100644
--- a/libqpdf/qpdf/auto_job_decl.hh
+++ b/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();
diff --git a/libqpdf/qpdf/auto_job_init.hh b/libqpdf/qpdf/auto_job_init.hh
index 8566312f..3d7cdd7b 100644
--- a/libqpdf/qpdf/auto_job_init.hh
+++ b/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.addRequiredChoices("decode-level", p(&ArgParser::argDecodeLevel), decode_level_choices);
-this->ap.addRequiredChoices("flatten-annotations", p(&ArgParser::argFlattenAnnotations), flatten_choices);
-this->ap.addRequiredChoices("json-key", p(&ArgParser::argJsonKey), json_key_choices);
-this->ap.addRequiredChoices("keep-files-open", p(&ArgParser::argKeepFilesOpen), yn_choices);
-this->ap.addRequiredChoices("normalize-content", p(&ArgParser::argNormalizeContent), yn_choices);
-this->ap.addRequiredChoices("object-streams", p(&ArgParser::argObjectStreams), object_streams_choices);
-this->ap.addRequiredChoices("password-mode", p(&ArgParser::argPasswordMode), password_mode_choices);
-this->ap.addRequiredChoices("remove-unreferenced-resources", p(&ArgParser::argRemoveUnreferencedResources), remove_unref_choices);
-this->ap.addRequiredChoices("stream-data", p(&ArgParser::argStreamData), stream_data_choices);
+this->ap.addChoices("compress-streams", p(&ArgParser::argCompressStreams), true, yn_choices);
+this->ap.addChoices("decode-level", p(&ArgParser::argDecodeLevel), true, decode_level_choices);
+this->ap.addChoices("flatten-annotations", p(&ArgParser::argFlattenAnnotations), true, flatten_choices);
+this->ap.addChoices("json-key", p(&ArgParser::argJsonKey), true, json_key_choices);
+this->ap.addChoices("keep-files-open", p(&ArgParser::argKeepFilesOpen), true, yn_choices);
+this->ap.addChoices("normalize-content", p(&ArgParser::argNormalizeContent), true, yn_choices);
+this->ap.addChoices("object-streams", p(&ArgParser::argObjectStreams), true, object_streams_choices);
+this->ap.addChoices("password-mode", p(&ArgParser::argPasswordMode), true, password_mode_choices);
+this->ap.addChoices("remove-unreferenced-resources", p(&ArgParser::argRemoveUnreferencedResources), true, remove_unref_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.addRequiredChoices("annotate", p(&ArgParser::argEnc40Annotate), yn_choices);
-this->ap.addRequiredChoices("print", p(&ArgParser::argEnc40Print), yn_choices);
-this->ap.addRequiredChoices("modify", p(&ArgParser::argEnc40Modify), yn_choices);
+this->ap.addChoices("extract", p(&ArgParser::argEnc40Extract), true, yn_choices);
+this->ap.addChoices("annotate", p(&ArgParser::argEnc40Annotate), true, yn_choices);
+this->ap.addChoices("print", p(&ArgParser::argEnc40Print), true, 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.addRequiredChoices("extract", p(&ArgParser::argEnc128Extract), yn_choices);
-this->ap.addRequiredChoices("print", p(&ArgParser::argEnc128Print), print128_choices);
-this->ap.addRequiredChoices("assemble", p(&ArgParser::argEnc128Assemble), yn_choices);
-this->ap.addRequiredChoices("annotate", p(&ArgParser::argEnc128Annotate), yn_choices);
-this->ap.addRequiredChoices("form", p(&ArgParser::argEnc128Form), yn_choices);
-this->ap.addRequiredChoices("modify-other", p(&ArgParser::argEnc128ModifyOther), yn_choices);
-this->ap.addRequiredChoices("modify", p(&ArgParser::argEnc128Modify), modify128_choices);
-this->ap.addRequiredChoices("use-aes", p(&ArgParser::argEnc128UseAes), yn_choices);
+this->ap.addChoices("accessibility", p(&ArgParser::argEnc128Accessibility), true, yn_choices);
+this->ap.addChoices("extract", p(&ArgParser::argEnc128Extract), true, yn_choices);
+this->ap.addChoices("print", p(&ArgParser::argEnc128Print), true, print128_choices);
+this->ap.addChoices("assemble", p(&ArgParser::argEnc128Assemble), true, yn_choices);
+this->ap.addChoices("annotate", p(&ArgParser::argEnc128Annotate), true, yn_choices);
+this->ap.addChoices("form", p(&ArgParser::argEnc128Form), true, yn_choices);
+this->ap.addChoices("modify-other", p(&ArgParser::argEnc128ModifyOther), true, yn_choices);
+this->ap.addChoices("modify", p(&ArgParser::argEnc128Modify), true, modify128_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));
diff --git a/libtests/arg_parser.cc b/libtests/arg_parser.cc
index a57b9c66..3da0206e 100644
--- a/libtests/arg_parser.cc
+++ b/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();
diff --git a/libtests/qtest/arg_parser/completion-top-arg-zsh.out b/libtests/qtest/arg_parser/completion-top-arg-zsh.out
index 11bcb3b6..5a500d38 100644
--- a/libtests/qtest/arg_parser/completion-top-arg-zsh.out
+++ b/libtests/qtest/arg_parser/completion-top-arg-zsh.out
@@ -1,5 +1,8 @@
 --baaa
 --completion-zsh
+--help
+--help=
+--help=all
 --moo
 --moo=
 --oink=