Checked into 'master':

• 611419fcd8a19c06ca651add93deb66bdd0c55d5 - CLI argument parsing and bad return codes
• ce5a07f06f361040b14ed5b1622d816c7044a7ec - Development tool to aid verification

[root@sigma dogtag]# pki user-cert-show --help
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--help Show help options

[root@sigma dogtag]# pki user-cert-show
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--encoded Base-64 encoded
--help Show help options
--output <file> Output file
--pretty Pretty print

Replying to [comment:6 rpattath]:

[root@sigma dogtag]# pki user-cert-show --help
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--help Show help options

Note, however, that this command invocation correctly returns "0" which implies success.

[root@sigma dogtag]# pki user-cert-show
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--encoded Base-64 encoded
--help Show help options
--output <file> Output file
--pretty Pretty print

Similarly, this command invocation correctly returns "1" implying failure, since a usage is being supplied as a result of improper use of the command. That being said, I believe that in this case, there should have also been an error message which stated that insufficient parameters were being applied.

The fix to the proper "--help" usage statement is to move the (Arrays.asList(args).contains("--help")) check to be after all "Options" for that command have been initialized; this fix needs to be applied to several commands.

Additionally, this particular command (one amongst many) needs to contain option.setRequired(true); since this command requires additional arguments.

However, on certain commands, this could cause a side-effect that if the "--help" command-line option is not looked for initially (as it is in various commands), it would not work correctly since an exception would be thrown prior to the check for "--help".

To fix all of these issues and to help avoid future mishaps, I propose the following logic be systematically applied to all "usage"-based CLI objects (as opposed to "Command"-based CLI objects which need to allow the construction of a potential command prior to processing the "--help" command-line option):

• Perform initialization of all command-line "Options"
  • insert option.setRequired(true); as needed
• Regardless of the number of parameters, always check for the presence of "--help" on the command-line using (Arrays.asList(args).contains("--help")), echo the usage statement for that command, and return '0' (SUCCESS)
• Parse the command-line throwing an exception when needed, echo an error message followed by the usage statement for that command, and return '1' (FAILURE)
• Remove any checks for the "--help" option both within and after the exception as these will have now always been handled previously
• Correct all number of argument checks that occur after the parse statement, adding/removing any new checks as required due to the addition/removal of any option.setRequired(true); statements above

Replying to [comment:7 mharmsen]:

Replying to [comment:6 rpattath]:

[root@sigma dogtag]# pki user-cert-show --help
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--help Show help options

Note, however, that this command invocation correctly returns "0" which implies success.

[root@sigma dogtag]# pki user-cert-show
usage: user-cert-show <User ID> <Cert ID> [OPTIONS...]
--encoded Base-64 encoded
--help Show help options
--output <file> Output file
--pretty Pretty print

Similarly, this command invocation correctly returns "1" implying failure, since a usage is being supplied as a result of improper use of the command. That being said, I believe that in this case, there should have also been an error message which stated that insufficient parameters were being applied.

The fix to the proper "--help" usage statement is to move the (Arrays.asList(args).contains("--help")) check to be after all "Options" for that command have been initialized; this fix needs to be applied to several commands.

Additionally, this particular command (one amongst many) needs to contain option.setRequired(true); since this command requires additional arguments.

However, on certain commands, this could cause a side-effect that if the "--help" command-line option is not looked for initially (as it is in various commands), it would not work correctly since an exception would be thrown prior to the check for "--help".

To fix all of these issues and to help avoid future mishaps, I propose the following logic be systematically applied to all "usage"-based CLI objects (as opposed to "Command"-based CLI objects which need to allow the construction of a potential command prior to processing the "--help" command-line option):
Perform initialization of all command-line "Options"
* insert option.setRequired(true); as needed
Regardless of the number of parameters, always check for the presence of "--help" on the command-line using (Arrays.asList(args).contains("--help")), echo the usage statement for that command, and return '0' (SUCCESS)
Parse the command-line throwing an exception when needed, echo an error message followed by the usage statement for that command, and return '1' (FAILURE)
Remove any checks for the "--help" option both within and after the exception as these will have now always been handled previously
* Correct all number of argument checks that occur after the parse statement, adding/removing any new checks as required due to the addition/removal of any option.setRequired(true); statements above

This proposal was discussed with edewata over IRC, and we agreed upon the following revised plan:

• Create a createOptions() method for each command as necessary:
  • populate this method by moving the option settings from each command's execute() method into the command's createOptions() method
  • place a call to createOptions() in each command's constructor
• Each command's execute() method will contain a (Arrays.asList(args).contains("--help")) check as its first statement which will display a usage() statement and exit with an exit code of "0"; this check will now always occur after all of each command's options have been set and prior to the parse() invocation
• All other references to "help" which occur after the parse() invocation in the execute() method will be deleted
• As needed, an args.length() check will be provided after the parse() invocation in the execute() method, and an error message such as "Missing <argument>.", "No <argument> specified.", or "Insufficient number of arguments specified." will be displayed prior to a usage() statement and exit with an exit code of "-1"
• For consistency, change all error code exit codes to "-1"

NOTE:  As it was explained that args and options are two different entities,
       new "option.setRequired(true);" statements will not be utilized
       as they only pertain to "options" and not to "args".

Checked in to 'master' branch:

• 7b6b60b7d8d26799ea1bda48e6e51fa05854c80e

Requested changes to parsing involving "arguments" and "options" will be addressed in PKI TRAC Ticket #982 - CLI commands does not give an error message only for missing required options and not for required arguments.

Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new
issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.

This issue has been cloned to GitHub and is available here:
https://github.com/dogtagpki/pki/issues/1410

If you want to receive further updates on the issue, please navigate to the
GitHub issue and click on Subscribe button.

Thank you for understanding, and we apologize for any inconvenience.