Skip Menu |

This queue is for tickets about the Getopt-ArgParse CPAN distribution.

Report information
The Basics
Id: 102497
Status: resolved
Priority: 0/
Queue: Getopt-ArgParse

People
Owner: MYTRAM [...] cpan.org
Requestors: apfeiffe [...] Brocade.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 1.0.5



CC: "mytram2 [...] gmail.com" <mytram2 [...] gmail.com>, Adam Pfeiffer <apfeiffe [...] Brocade.com>
Subject: Split out the required from the optional parameters in Getopt::ArgParse
Date: Tue, 3 Mar 2015 23:18:25 +0000
To: "bug-Getopt-ArgParse [...] rt.cpan.org" <bug-Getopt-ArgParse [...] rt.cpan.org>
From: Adam Pfeiffer <apfeiffe [...] Brocade.com>
Hello, Right now all arguments are listed as optional in the usage statement, even the ones that are required. Here is an example of the output before any changes: 20_fwdlBatch.pl: usage: 20_fwdlBatch.pl [--help|-h] --fabric_name --firmware_version [--load_type] [--reboot_type] [--sleep_before_data_gather] [--sleep_between_fabrics] [--login_id] Upgrades all switches in one fabric in parallel. If multiple fabrics are specified, it will upgrade one fabric at a time. You can choose between hcl or -sf. If using -sf, you can specify the reboot type: none, single, multiple. none = don't reboot after -sf single = reboot one switch at a time (for chassis, reboots one cp at a time) multiple = reboot all switches at the same time (for chassis, reboots both cps at the same time) optional arguments: --help, -h ? show this help message and exit --fabric_name FABRIC_NAME comma seperated list of fabrics --firmware_version FIRMWARE_VERSION short name (v7.3.0a_bld40) or full path --load_type LOAD_TYPE ? -sf = single user, hcl = hotcodeload Choices: [-sf, hcl], case insensitive Default: hcl --reboot_type REBOOT_TYPE ? type of reboot to perform (only valid for load_type: -sf) Choices: [none, single, multiple], case insensitive Default: none --sleep_before_data_gather SLEEP_BEFORE_DATA_GATHER ? seconds to sleep after firmware upgrade is complete Default: 240 --sleep_between_fabrics SLEEP_BETWEEN_FABRICS ? seconds to sleep between fabrics Default: 300 --login_id LOGIN_ID ? userid to use when logging into the switches Default: root I suggest modifying _format_usage_by_spec and _format_group_usage so that the output looks like this: ./20_fwdlBatch: usage: ./20_fwdlBatch [--help|-h] --tbc --fabric_name --firmware_version [--load_type] [--reboot_type] [--fw_host] [--fw_user] [--fw_pass] [--email] [--product] [--release] [--phase] [--loops] [--debug] Upgrades all switches in one fabric in parallel. If multiple fabrics are specified, it will upgrade one fabric at a time. You can choose between hcl or -sf. If using -sf, you can specify the reboot type: none, single, multiple. none = don't reboot after -sf single = reboot one switch at a time (for chassis, reboots one cp at a time) multiple = reboot all switches at the same time (for chassis, reboots both cps at the same time) required arguments: --tbc TBC path to the tbc file --fabric_name FABRIC_NAME comma seperated list of fabrics --firmware_version FIRMWARE_VERSION short name (v7.3.0a_bld40) or full path optional arguments: --help, -h ? show this help message and exit --load_type LOAD_TYPE ? -sf = single user, hcl = hotcodeload Choices: [-sf, hcl], case insensitive Default: hcl --reboot_type REBOOT_TYPE ? type of reboot to perform (only valid for load_type: -sf) Choices: [none, single, multiple], case insensitive Default: none --fw_host FW_HOST ? IP/hostname of the firmware server Default: 10.38.2.25 --fw_user FW_USER ? username for the firmware server Default: anonymous --fw_pass FW_PASS ? password for the firmware server Default: email@email.com --email EMAIL ? list of users to receive email results for this test run Default: apfeiffe --product PRODUCT ? product you are testing against Default: none --release RELEASE ? release you are testing against Default: -- --phase PHASE ? phase you are testing against Default: -- --loops LOOPS ? number of time to perform this test Default: 1 --debug ? run this script in the debugger I have updated my local code to produce this new output. Here are my changes: 1154 if (@option_specs) { 1155 push @usage, 'required arguments:'; 1156 push @usage, @{ $self->_format_usage_by_spec(\@option_specs, 1) }; 1157 } 1158 1159 if (@option_specs) { 1160 push @usage, 'optional arguments:'; 1161 push @usage, @{ $self->_format_usage_by_spec(\@option_specs, 0) }; 1162 } In the above code, I have added a 2nd call to _format_usage_by_spec and I am passing a 2nd parameter which says if the function should print required arguments (when set to 1) or optional arguments (when set to 0). Then, in _format_usage_by_spec: 1169 sub _format_usage_by_spec { 1170 my $self = shift; 1171 my $specs = shift; 1172 my $required = shift; And 1180 for my $spec ( @$specs ) { 1181 if ($required and !$spec->{'required'}) 1182 { 1183 next; 1184 } 1185 if (!$required and $spec->{'required'}) 1186 { 1187 next; 1188 } Thanks much Adam Pfeiffer SQA Team Brocade 4 Brocade Parkway, Broomfield, CO 80021 (720) 558-3681 www.brocade.com<http://www.brocade.com/> [Brocade Logo]

Message body is not shown because it is too large.

Download image001.gif
image/gif 2.8k
image001.gif
Subject: Re: [rt.cpan.org #102497] Split out the required from the optional parameters in Getopt::ArgParse
Date: Wed, 4 Mar 2015 13:28:27 +1100
To: bug-Getopt-ArgParse [...] rt.cpan.org
From: Marty Ma <mytram2 [...] gmail.com>
Thanks Adam! I am going to have them looked at soon. -Marty On 4 March 2015 at 10:18, Adam Pfeiffer via RT < bug-Getopt-ArgParse@rt.cpan.org> wrote: Show quoted text
> Tue Mar 03 18:18:44 2015: Request 102497 was acted upon. > Transaction: Ticket created by apfeiffe@Brocade.com > Queue: Getopt-ArgParse > Subject: Split out the required from the optional parameters in > Getopt::ArgParse > Broken in: (no value) > Severity: (no value) > Owner: Nobody > Requestors: apfeiffe@Brocade.com > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=102497 > > > > Hello, > Right now all arguments are listed as optional in the usage statement, > even the ones that are required. Here is an example of the output before > any changes: > > 20_fwdlBatch.pl: > usage: 20_fwdlBatch.pl [--help|-h] --fabric_name --firmware_version > [--load_type] [--reboot_type] [--sleep_before_data_gather] > [--sleep_between_fabrics] [--login_id] > > Upgrades all switches in one fabric in parallel. If multiple fabrics are > specified, it will upgrade one fabric at a time. You can choose between > hcl or > -sf. If using -sf, you can specify the reboot type: none, single, > multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp > at a > time) > > multiple = reboot all switches at the same time (for chassis, > reboots > both cps at the same time) > > > > optional arguments: > --help, -h ? show this > help message and exit > --fabric_name FABRIC_NAME comma > seperated list of fabrics > --firmware_version FIRMWARE_VERSION short name > (v7.3.0a_bld40) or full path > --load_type LOAD_TYPE ? -sf = single > user, hcl = hotcodeload > Choices: > [-sf, hcl], case insensitive > Default: hcl > --reboot_type REBOOT_TYPE ? type of > reboot to perform (only valid for load_type: -sf) > Choices: > [none, single, multiple], case insensitive > Default: none > --sleep_before_data_gather SLEEP_BEFORE_DATA_GATHER ? seconds to > sleep after firmware upgrade is complete > Default: 240 > --sleep_between_fabrics SLEEP_BETWEEN_FABRICS ? seconds to > sleep between fabrics > Default: 300 > --login_id LOGIN_ID ? userid to use > when logging into the switches > Default: root > > > I suggest modifying _format_usage_by_spec and _format_group_usage so that > the output looks like this: > ./20_fwdlBatch: > usage: ./20_fwdlBatch [--help|-h] --tbc --fabric_name --firmware_version > [--load_type] [--reboot_type] [--fw_host] [--fw_user] [--fw_pass] [--email] > [--product] [--release] [--phase] [--loops] [--debug] > > Upgrades all switches in one fabric in parallel. If multiple fabrics are > specified, it will upgrade one fabric at a time. You can choose between > hcl or > -sf. If using -sf, you can specify the reboot type: none, single, > multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp > at a > time) > > multiple = reboot all switches at the same time (for chassis, > reboots > both cps at the same time) > > > > required arguments: > --tbc TBC path to the tbc file > --fabric_name FABRIC_NAME comma seperated list of > fabrics > --firmware_version FIRMWARE_VERSION short name (v7.3.0a_bld40) or > full path > optional arguments: > --help, -h ? show this help message and exit > --load_type LOAD_TYPE ? -sf = single user, hcl = hotcodeload > Choices: [-sf, hcl], case insensitive > Default: hcl > --reboot_type REBOOT_TYPE ? type of reboot to perform (only valid > for load_type: -sf) > Choices: [none, single, multiple], case > insensitive > Default: none > --fw_host FW_HOST ? IP/hostname of the firmware server > Default: 10.38.2.25 > --fw_user FW_USER ? username for the firmware server > Default: anonymous > --fw_pass FW_PASS ? password for the firmware server > Default: email@email.com > --email EMAIL ? list of users to receive email results > for this test run > Default: apfeiffe > --product PRODUCT ? product you are testing against > Default: none > --release RELEASE ? release you are testing against > Default: -- > --phase PHASE ? phase you are testing against > Default: -- > --loops LOOPS ? number of time to perform this test > Default: 1 > --debug ? run this script in the debugger > > > I have updated my local code to produce this new output. Here are my > changes: > 1154 if (@option_specs) { > 1155 push @usage, 'required arguments:'; > 1156 push @usage, @{ $self->_format_usage_by_spec(\@option_specs, > 1) }; > 1157 } > 1158 > 1159 if (@option_specs) { > 1160 push @usage, 'optional arguments:'; > 1161 push @usage, @{ $self->_format_usage_by_spec(\@option_specs, > 0) }; > 1162 } > > In the above code, I have added a 2nd call to _format_usage_by_spec and I > am passing a 2nd parameter which says if the function should print required > arguments (when set to 1) or optional arguments (when set to 0). > > Then, in _format_usage_by_spec: > 1169 sub _format_usage_by_spec { > 1170 my $self = shift; > 1171 my $specs = shift; > 1172 my $required = shift; > > And > > 1180 for my $spec ( @$specs ) { > 1181 if ($required and !$spec->{'required'}) > 1182 { > 1183 next; > 1184 } > 1185 if (!$required and $spec->{'required'}) > 1186 { > 1187 next; > 1188 } > > Thanks much > Adam Pfeiffer > SQA Team > Brocade > 4 Brocade Parkway, Broomfield, CO 80021 > (720) 558-3681 > www.brocade.com<http://www.brocade.com/> > [Brocade Logo] > > >
CC: Adam Pfeiffer <apfeiffe [...] Brocade.com>
Subject: RE: [rt.cpan.org #102497] Split out the required from the optional parameters in Getopt::ArgParse
Date: Wed, 4 Mar 2015 15:34:05 +0000
To: "bug-Getopt-ArgParse [...] rt.cpan.org" <bug-Getopt-ArgParse [...] rt.cpan.org>
From: Adam Pfeiffer <apfeiffe [...] Brocade.com>
Thanks much! I also forked this project on github and posted my changes. I then created a pull request so you could easily see the changes. Thanks Adam Pfeiffer
Hi Adam, I see where the confusion is. I was using the terms: `optional arguments` (-option) vs. positional arguments (arguments without an option name). The "optional arguments" words you mentioned were in that context. However, this's confusing as using the same words to mean two things in different context. I decided to use "named arguments" vs. "positional arguments", and keep "optional" vs. "required" as most will expect. And I used the '?' to indicate if an argument wasn't required but then that also meant that required and optional arguments' help text were intermixed, and I myself have run into trouble with that ;-). Building on top of your change, I use the following new sections for argument help text: required positional arguments: if any optional positional arguments: if any required named arguments: if any optional named arguments: if any. I hope that way the module will suit your needs too. I am packing up a new cpan release. And will have these two RT closed once that's done. Once again, thanks so much for your ideas and pull request! Marty = On Tue Mar 03 18:18:44 2015, apfeiffe@Brocade.com wrote: Show quoted text
> Hello, > Right now all arguments are listed as optional in the usage statement, > even the ones that are required. Here is an example of the output > before any changes: > > 20_fwdlBatch.pl: > usage: 20_fwdlBatch.pl [--help|-h] --fabric_name --firmware_version > [--load_type] [--reboot_type] [--sleep_before_data_gather] > [--sleep_between_fabrics] [--login_id] > > Upgrades all switches in one fabric in parallel. If multiple fabrics > are > specified, it will upgrade one fabric at a time. You can choose > between hcl or > -sf. If using -sf, you can specify the reboot type: none, single, > multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp at a > time) > > multiple = reboot all switches at the same time (for chassis, reboots > both cps at the same time) > > > > optional arguments: > --help, -h ? show this > help message and exit > --fabric_name FABRIC_NAME comma > seperated list of fabrics > --firmware_version FIRMWARE_VERSION short > name (v7.3.0a_bld40) or full path > --load_type LOAD_TYPE ? -sf = > single user, hcl = hotcodeload > Choices: > [-sf, hcl], case insensitive > Default: > hcl > --reboot_type REBOOT_TYPE ? type of > reboot to perform (only valid for load_type: -sf) > Choices: > [none, single, multiple], case insensitive > Default: > none > --sleep_before_data_gather SLEEP_BEFORE_DATA_GATHER ? seconds > to sleep after firmware upgrade is complete > Default: > 240 > --sleep_between_fabrics SLEEP_BETWEEN_FABRICS ? seconds > to sleep between fabrics > Default: > 300 > --login_id LOGIN_ID ? userid to > use when logging into the switches > Default: > root > > > I suggest modifying _format_usage_by_spec and _format_group_usage so > that the output looks like this: > ./20_fwdlBatch: > usage: ./20_fwdlBatch [--help|-h] --tbc --fabric_name > --firmware_version > [--load_type] [--reboot_type] [--fw_host] [--fw_user] [--fw_pass] [-- > email] > [--product] [--release] [--phase] [--loops] [--debug] > > Upgrades all switches in one fabric in parallel. If multiple fabrics > are > specified, it will upgrade one fabric at a time. You can choose > between hcl or > -sf. If using -sf, you can specify the reboot type: none, single, > multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp at a > time) > > multiple = reboot all switches at the same time (for chassis, reboots > both cps at the same time) > > > > required arguments: > --tbc TBC path to the tbc file > --fabric_name FABRIC_NAME comma seperated list of > fabrics > --firmware_version FIRMWARE_VERSION short name > (v7.3.0a_bld40) or full path > optional arguments: > --help, -h ? show this help message and exit > --load_type LOAD_TYPE ? -sf = single user, hcl = > hotcodeload > Choices: [-sf, hcl], case > insensitive > Default: hcl > --reboot_type REBOOT_TYPE ? type of reboot to perform (only > valid for load_type: -sf) > Choices: [none, single, multiple], > case insensitive > Default: none > --fw_host FW_HOST ? IP/hostname of the firmware server > Default: 10.38.2.25 > --fw_user FW_USER ? username for the firmware server > Default: anonymous > --fw_pass FW_PASS ? password for the firmware server > Default: email@email.com > --email EMAIL ? list of users to receive email > results for this test run > Default: apfeiffe > --product PRODUCT ? product you are testing against > Default: none > --release RELEASE ? release you are testing against > Default: -- > --phase PHASE ? phase you are testing against > Default: -- > --loops LOOPS ? number of time to perform this test > Default: 1 > --debug ? run this script in the debugger > > > I have updated my local code to produce this new output. Here are my > changes: > 1154 if (@option_specs) { > 1155 push @usage, 'required arguments:'; > 1156 push @usage, @{ $self-
> >_format_usage_by_spec(\@option_specs, 1) };
> 1157 } > 1158 > 1159 if (@option_specs) { > 1160 push @usage, 'optional arguments:'; > 1161 push @usage, @{ $self-
> >_format_usage_by_spec(\@option_specs, 0) };
> 1162 } > > In the above code, I have added a 2nd call to _format_usage_by_spec > and I am passing a 2nd parameter which says if the function should > print required arguments (when set to 1) or optional arguments (when > set to 0). > > Then, in _format_usage_by_spec: > 1169 sub _format_usage_by_spec { > 1170 my $self = shift; > 1171 my $specs = shift; > 1172 my $required = shift; > > And > > 1180 for my $spec ( @$specs ) { > 1181 if ($required and !$spec->{'required'}) > 1182 { > 1183 next; > 1184 } > 1185 if (!$required and $spec->{'required'}) > 1186 { > 1187 next; > 1188 } > > Thanks much > Adam Pfeiffer > SQA Team > Brocade > 4 Brocade Parkway, Broomfield, CO 80021 > (720) 558-3681 > www.brocade.com<http://www.brocade.com/> > [Brocade Logo]
CC: Adam Pfeiffer <apfeiffe [...] Brocade.com>
Subject: RE: [rt.cpan.org #102497] Split out the required from the optional parameters in Getopt::ArgParse
Date: Fri, 6 Mar 2015 15:50:28 +0000
To: "bug-Getopt-ArgParse [...] rt.cpan.org" <bug-Getopt-ArgParse [...] rt.cpan.org>
From: Adam Pfeiffer <apfeiffe [...] Brocade.com>
Hi Marty, I think the changes you proposed sound great. I will test out the changes once released. Thanks for your time and effort to help make this module better! Adam Pfeiffer P.S. I am working on making this the standard way to read in arguments in our automation framework. I love the clear cut interface and the easy to use syntax.   Show quoted text
-----Original Message----- From: M ytraM via RT [mailto:bug-Getopt-ArgParse@rt.cpan.org] Sent: Friday, March 06, 2015 3:44 AM To: Adam Pfeiffer Subject: [rt.cpan.org #102497] Split out the required from the optional parameters in Getopt::ArgParse <URL: https://rt.cpan.org/Ticket/Display.html?id=102497 > Hi Adam, I see where the confusion is. I was using the terms: `optional arguments` (-option) vs. positional arguments (arguments without an option name). The "optional arguments" words you mentioned were in that context. However, this's confusing as using the same words to mean two things in different context. I decided to use "named arguments" vs. "positional arguments", and keep "optional" vs. "required" as most will expect. And I used the '?' to indicate if an argument wasn't required but then that also meant that required and optional arguments' help text were intermixed, and I myself have run into trouble with that ;-). Building on top of your change, I use the following new sections for argument help text: required positional arguments: if any optional positional arguments: if any required named arguments: if any optional named arguments: if any. I hope that way the module will suit your needs too. I am packing up a new cpan release. And will have these two RT closed once that's done. Once again, thanks so much for your ideas and pull request! Marty = On Tue Mar 03 18:18:44 2015, apfeiffe@Brocade.com wrote:
> Hello, > Right now all arguments are listed as optional in the usage statement, > even the ones that are required. Here is an example of the output > before any changes: > > 20_fwdlBatch.pl: > usage: 20_fwdlBatch.pl [--help|-h] --fabric_name --firmware_version > [--load_type] [--reboot_type] [--sleep_before_data_gather] > [--sleep_between_fabrics] [--login_id] > > Upgrades all switches in one fabric in parallel. If multiple fabrics > are specified, it will upgrade one fabric at a time. You can choose > between hcl or -sf. If using -sf, you can specify the reboot type: > none, single, multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp at a > time) > > multiple = reboot all switches at the same time (for chassis, reboots > both cps at the same time) > > > > optional arguments: > --help, -h ? show this > help message and exit > --fabric_name FABRIC_NAME comma > seperated list of fabrics > --firmware_version FIRMWARE_VERSION short > name (v7.3.0a_bld40) or full path > --load_type LOAD_TYPE ? -sf = > single user, hcl = hotcodeload > Choices: > [-sf, hcl], case insensitive > Default: > hcl > --reboot_type REBOOT_TYPE ? type of > reboot to perform (only valid for load_type: -sf) > Choices: > [none, single, multiple], case insensitive > Default: > none > --sleep_before_data_gather SLEEP_BEFORE_DATA_GATHER ? seconds > to sleep after firmware upgrade is complete > Default: > 240 > --sleep_between_fabrics SLEEP_BETWEEN_FABRICS ? seconds > to sleep between fabrics > Default: > 300 > --login_id LOGIN_ID ? userid to > use when logging into the switches > Default: > root > > > I suggest modifying _format_usage_by_spec and _format_group_usage so > that the output looks like this: > ./20_fwdlBatch: > usage: ./20_fwdlBatch [--help|-h] --tbc --fabric_name > --firmware_version [--load_type] [--reboot_type] [--fw_host] > [--fw_user] [--fw_pass] [-- email] [--product] [--release] [--phase] > [--loops] [--debug] > > Upgrades all switches in one fabric in parallel. If multiple fabrics > are specified, it will upgrade one fabric at a time. You can choose > between hcl or -sf. If using -sf, you can specify the reboot type: > none, single, multiple. > > none = don't reboot after -sf > > single = reboot one switch at a time (for chassis, reboots one cp at a > time) > > multiple = reboot all switches at the same time (for chassis, reboots > both cps at the same time) > > > > required arguments: > --tbc TBC path to the tbc file > --fabric_name FABRIC_NAME comma seperated list of > fabrics > --firmware_version FIRMWARE_VERSION short name > (v7.3.0a_bld40) or full path > optional arguments: > --help, -h ? show this help message and exit > --load_type LOAD_TYPE ? -sf = single user, hcl = > hotcodeload > Choices: [-sf, hcl], case > insensitive > Default: hcl > --reboot_type REBOOT_TYPE ? type of reboot to perform (only > valid for load_type: -sf) > Choices: [none, single, multiple], > case insensitive > Default: none > --fw_host FW_HOST ? IP/hostname of the firmware server > Default: 10.38.2.25 > --fw_user FW_USER ? username for the firmware server > Default: anonymous > --fw_pass FW_PASS ? password for the firmware server > Default: email@email.com > --email EMAIL ? list of users to receive email > results for this test run > Default: apfeiffe > --product PRODUCT ? product you are testing against > Default: none > --release RELEASE ? release you are testing against > Default: -- > --phase PHASE ? phase you are testing against > Default: -- > --loops LOOPS ? number of time to perform this test > Default: 1 > --debug ? run this script in the debugger > > > I have updated my local code to produce this new output. Here are my > changes: > 1154 if (@option_specs) { > 1155 push @usage, 'required arguments:'; > 1156 push @usage, @{ $self-
> >_format_usage_by_spec(\@option_specs, 1) };
> 1157 } > 1158 > 1159 if (@option_specs) { > 1160 push @usage, 'optional arguments:'; > 1161 push @usage, @{ $self-
> >_format_usage_by_spec(\@option_specs, 0) };
> 1162 } > > In the above code, I have added a 2nd call to _format_usage_by_spec > and I am passing a 2nd parameter which says if the function should > print required arguments (when set to 1) or optional arguments (when > set to 0). > > Then, in _format_usage_by_spec: > 1169 sub _format_usage_by_spec { > 1170 my $self = shift; > 1171 my $specs = shift; > 1172 my $required = shift; > > And > > 1180 for my $spec ( @$specs ) { > 1181 if ($required and !$spec->{'required'}) > 1182 { > 1183 next; > 1184 } > 1185 if (!$required and $spec->{'required'}) > 1186 { > 1187 next; > 1188 } > > Thanks much > Adam Pfeiffer > SQA Team > Brocade > 4 Brocade Parkway, Broomfield, CO 80021 > (720) 558-3681 > www.brocade.com<http://www.brocade.com/> > [Brocade Logo]