[Bug 43557] New: Add declarative mutually-exclusive option support to Koha::Script->describe_options
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Bug ID: 43557 Summary: Add declarative mutually-exclusive option support to Koha::Script->describe_options Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: jake.deery@openfifth.co.uk, robin@catalyst.net.nz Target Milestone: --- Bug 43546 added Koha::Script->describe_options, a thin wrapper round Getopt::Long::Descriptive establishing a declarative convention for command-line option parsing: options can be marked required, and a positional-argument minimum can be declared, both enforced consistently before script logic runs, instead of every script rolling its own post-parse check and error text. Several cron scripts have their own hand-rolled "these two options are mutually exclusive" check, each with slightly different wording and mechanism: - misc/cronjobs/longoverdue.pl: --category/--skip-category, --library/--skip-library, --itemtype/--skip-itemtype (three separate pairs) - misc/cronjobs/membership_expiry.pl: --active/--inactive - misc/cronjobs/update_totalissues.pl: --since/--interval, --use-items/--incremental - misc/cronjobs/process_message_queue.pl: --code/--exclude-code (already converted to describe_options on bug 43551, using a one-off $usage->die() check since this feature didn't exist yet) That is seven pairs across four scripts, all reimplementing the same check. Getopt::Long::Descriptive's own one_of constraint looks like a fit at first glance, but testing shows it does not enforce exclusivity at all when the sub-options are array/repeatable types (e.g. our --code/--exclude-code, which both take multiple values): passing both options together with values is silently accepted, which would be a regression, not a fix. This bug is to add a declarative "exclusive" key to the trailing options hashref of Koha::Script->describe_options, alongside the existing "args" key, e.g.: my $opt = Koha::Script->describe_options( '%c %o', [ 'category|c=s@', 'category codes to include' ], [ 'skip-category|C=s@', 'category codes to exclude' ], { exclusive => [ [qw(category skip_category)] ] }, ); When more than one option in a declared exclusive group is supplied, the script should die with a clear, consistent error and the usage text, the same as a missing required option does today, instead of each script writing its own pod2usage/die/$usage->die variant. Test plan: 1. prove t/Koha/Script.t 2. Add regression tests covering: neither option given, one option given, both options given (dies with a clear message naming both options), and the fact that a bare optional-value flag with no value (see bug 37075) is not treated as "given" for this purpose. 3. koha-qa.pl passes for Koha/Script.pm Sponsored-by: OpenFifth <https://openfifth.co.uk/> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43546 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43546 [Bug 43546] Standardize command-line scripts on Getopt::Long::Descriptive for safer, more consistent option handling -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43551 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43551 [Bug 43551] Convert misc/cronjobs/process_message_queue.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43558 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43558 [Bug 43558] Convert misc/cronjobs/longoverdue.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43559 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43559 [Bug 43559] Convert misc/cronjobs/membership_expiry.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43560 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43560 [Bug 43560] Convert misc/cronjobs/update_totalissues.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206174 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206174&action=edit Bug 43557: Add declarative mutually-exclusive option support to Koha::Script->describe_options Adds an exclusive key to the trailing options hashref of Koha::Script->describe_options, alongside the existing required and args support. Declaring exclusive => [ [qw(code exclude-code)], ... ] makes a group of options mutually exclusive: if more than one is actually given a value, the script dies with a clear, consistent error and the usage text, the same as a missing required option already does, instead of every script rolling its own pod2usage/die check. Group members must be written using each option's first-declared spec name, hyphens and all (e.g. exclude-code, not the accessor's exclude_code) -- confirmed by testing that Getopt::Long::Descriptive's _specified() introspection method needs that exact form, not the canonicalised accessor name. An option only counts as "given" if it was specified on the command line and resolved to a non-empty value, so a bare optional-value (:s) flag left over as an empty string (bug 37075) does not by itself trigger the conflict -- this was tested directly against both scalar and repeatable (:s@) option types. Getopt::Long::Descriptive's own one_of constraint was considered as an alternative, since it already provides exclusivity checking for a group of options. Testing showed it does not enforce exclusivity at all when the group's options are array/repeatable types: passing two such options together with real values is silently accepted, which would have been a regression for scripts like process_message_queue.pl (bug 43551) whose --code/--exclude-code both take multiple values. Test plan: 1. prove t/Koha/Script.t 2. koha-qa.pl passes for Koha/Script.pm -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43549 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43549 [Bug 43549] Convert misc/cronjobs/cleanup_database.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org