[FIX] Throw missing subcommand argument on parse() and adapt misleading tutorial. #2179
Conversation
smehringer
force-pushed
the
argument_parser_fix_subcommand_parsing
branch
from
4d1c645
to
eeed52e
Compare
eseiler
changed the title
eseiler
requested review from
a team and
rrahn
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## master #2179 +/- ##
==========================================
+ Coverage 97.95% 98.07% +0.12%
==========================================
Files 262 262
Lines 10047 10675 +628
==========================================
+ Hits 9842 10470 +628
Misses 205 205
Continue to review full report at Codecov.
|
smehringer
force-pushed
the
argument_parser_fix_subcommand_parsing
branch
from
5c4a654
to
3cde399
Compare
| throw design_error{"The application name must only contain alpha-numeric characters or '_' and '-' " | ||
| "(regex: \"^[a-zA-Z0-9_-]+$\")."}; | ||
|
|
||
| for (auto & sub : this->subcommands) | ||
| if (!std::regex_match(sub, std::regex{"^[a-zA-Z0-9_]+$"})) |
There was a problem hiding this comment.
What is the difference between the app_name_regex and this one? And why does it have different requirements then the app name? For Example no - sign?
There was a problem hiding this comment.
Let me check..
So the subcommand parser name is automatically build with appname``-``subcommand, that's why the -. I don't remember adding the restriction but it was most probably me. I currently see no reason why this shouldn't be allowed.
I will fix that in a follow up PR though
There was a problem hiding this comment.
So the design is that the subcommand parser needs to be called foo-bar insteaf of foo bar?
There was a problem hiding this comment.
no the app is still called with ./foo bar on the command line. But the app name internally subparser.info.app_name is called foo-bar. A name is required and for the subparser I set the name internally, I guess I chose to add the top level parser to it 🤷 could disambiguate stuff in the future or with multiple nested parsers.
There was a problem hiding this comment.
I added a follow up issue to track this seqan/product_backlog#234
|
@rrahn polite ping |
smehringer
force-pushed
the
argument_parser_fix_subcommand_parsing
branch
from
85fe614
to
36190d3
Compare
Resolves #2172