Skip to content

Fix convert help subcommand error for #210#311

Open
heathdutton wants to merge 1 commit intousnistgov:developfrom
heathdutton:210-fix-convert-help-command
Open

Fix convert help subcommand error for #210#311
heathdutton wants to merge 1 commit intousnistgov:developfrom
heathdutton:210-fix-convert-help-command

Conversation

@heathdutton
Copy link

Committer Notes

Closes #210.

The convert subcommand was throwing an error when invoked with --help, instead of displaying the help message and returning exit code 0. This occurred because the parent class marked the --to option as required, causing parsing to fail before help could be displayed.

This fix overrides gatherOptions() to define the --to option without marking it as required, and implements validateOptions() to skip validation when --help or --version is present. For normal convert operations, --to is still validated manually to ensure it's provided.

All Submissions:

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)? N/A
  • Have you updated all website and readme documentation affected by the changes you made? N/A

@iMichaela
Copy link

Thank you, @heathdutton. @selenaxiao-nist - please review the PR when you have a chance.

Copy link

@selenaxiao-nist selenaxiao-nist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heathdutton
Thanks for your contribution. This removes the error when running oscal-cli catalog convert --help but prints the error twice when running oscal-cli catalog convert

Please revise.

@heathdutton heathdutton marked this pull request as draft December 30, 2025 15:47
@heathdutton heathdutton force-pushed the 210-fix-convert-help-command branch from 325ca86 to ed83b63 Compare December 30, 2025 15:50
@heathdutton heathdutton marked this pull request as ready for review December 30, 2025 15:52
@selenaxiao-nist selenaxiao-nist self-requested a review December 30, 2025 18:18
Copy link

@selenaxiao-nist selenaxiao-nist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is still printed twice, but after more testing I found this also happens with other subcommands, and isn't directly caused by these new changes. LGTM otherwise. I'll approve this PR and open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants