-
Notifications
You must be signed in to change notification settings - Fork 7
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
[MISC] Allow options for subcommands #244
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #244 +/- ##
==========================================
- Coverage 95.13% 95.10% -0.04%
==========================================
Files 18 18
Lines 1728 1735 +7
==========================================
+ Hits 1644 1650 +6
- Misses 84 85 +1 ☔ View full report in Codecov by Sentry. |
820e72d
to
e6ef526
Compare
e6ef526
to
b0acb3e
Compare
b0acb3e
to
35c537c
Compare
35c537c
to
700591e
Compare
700591e
to
9cb2421
Compare
9cb2421
to
3b9fee0
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.
I see two things:
std::vector<std::string> options
is used as a set, so maybestd::unordered_set<std::string>
is the appropriate data structure.- I personally prefer 'unknown subcommand' over 'misspelled subcommand'
d89995c
to
8f755cc
Compare
8f755cc
to
a236f78
Compare
Agreed. I ended up refactoring |
a236f78
to
d44f6a1
Compare
Follow-up of #238
Main Problem
The subcommand is determined by just checking whether any argument matches a subcommand.
To allow options for the top-level-parser, we need to know whether a potential subcommand is actually an option value.
E.g.,
Currently, it would use
build
as subcommand and then fail onparse()
of the top-level-parser.At this point of parsing, we don't know whether
-o
is an option or flag, so just checking for-
is not enough.My first intuition is that the
format_parse
needs to know about (and) handle the subcommands. Which would also mean that it needs a pointer to the parent.However, special formats also must know to which command they apply.
Since
format_parse
has functions to check if an option or flag is set, those might be reused.Test cases
Test case 1
Test case 2
Test case 3
Test case 4
Test case 5
Test case 6
Test case 7