From 53ba65eb59d0bced37e73d8bf96a0d7a7285f662 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Fri, 7 Jan 2022 15:29:27 -0500 Subject: [PATCH] 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 | 14 +++- include/qpdf/QPDFArgParser.hh | 19 +++-- job.sums | 8 +- job.yml | 1 - libqpdf/QPDFArgParser.cc | 84 +++++++++++-------- libqpdf/QPDFJob_argv.cc | 3 + libqpdf/qpdf/auto_job_decl.hh | 1 - libqpdf/qpdf/auto_job_init.hh | 47 +++++------ libtests/arg_parser.cc | 2 +- .../arg_parser/completion-top-arg-zsh.out | 3 + 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', {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: 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 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 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) { @@ -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)) || - ((! 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::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::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 + "="); } @@ -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 + "="; - } - 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::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=