-
Notifications
You must be signed in to change notification settings - Fork 669
fix(cli): allow exclusive arguments as optional #2770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(cli): allow exclusive arguments as optional #2770
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2770 +/- ##
=======================================
Coverage 96.48% 96.49%
=======================================
Files 90 90
Lines 5866 5874 +8
=======================================
+ Hits 5660 5668 +8
Misses 206 206
Flags with carried forward coverage won't be shown. Click here to find out more.
|
9a16bdd
to
d9d260c
Compare
Would it make more sense to create a mutually exclusive group and add the exclusive args to that? https://docs.python.org/3/library/argparse.html#mutual-exclusion |
I've changed the code to create a mutually exclusive group. I think the data model of RequiredOptional is not ideal. Whether options are exclusive should be perpendicular to whether they are required or optional. Also, it would be possible to have multiple exclusive groups. Currently this is not reflected in the RequiredOptional.exclusive property. It only allows one group, and it is not clear whether this group is required or not. Ideally, you would have something lilke this:
But adding these exclusive options to the CLI argument parser at least makes it functioning again, if not perfect. |
24dc41e
to
a53397d
Compare
The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser. Closes python-gitlab#2769
e435fc9
to
04171d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for this.
The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.
Closes #2769
Changes
Documentation and testing
Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated: