Split up the two parts of cmdlinet::parse into separate functions #5462
Conversation
hannes-steffenhagen-diffblue
requested review from
kroening,
peterschrammel,
pkesseli,
smowton and
tautschnig
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #5462 +/- ##
========================================
Coverage 69.65% 69.65%
========================================
Files 1243 1243
Lines 100843 100847 +4
========================================
+ Hits 70243 70247 +4
Misses 30600 30600
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Your commits have empty message bodies. They should really have a rationale for the changes are being made. |
|
@thomasspriggs @NlightNFotis I have added a bit of documentation, if you would check if that works for you? |
hannes-steffenhagen-diffblue
force-pushed
the
refactor/split-up-cmdline-parse
branch
from
ba9c516 to
8e9c61b
Compare
| /// string at index argc being a terminating null pointer. | ||
| /// This parameter is parsed based on optstring. | ||
| /// \param optstring A specification of allowed command line options. | ||
| /// This is a C string container any number of single |
There was a problem hiding this comment.
⛏️ container -> containing
src/util/cmdline.cpp
Outdated
| } | ||
| void cmdlinet::parse_optstring(const char *optstring) |
There was a problem hiding this comment.
Nit pick: a blank line would be nice to have.
src/util/cmdline.h
Outdated
| @@ -20,6 +20,58 @@ Author: Daniel Kroening, [email protected] | |||
| class cmdlinet | |||
| { | |||
| public: | |||
| /// Parses a commandline according to a specification given in optstring. | |||
There was a problem hiding this comment.
I'd suggest \p optstring to clarify to an extent what this is referring to.
src/util/cmdline.h
Outdated
| /// it was invoked (e.g. /usr/bin/cmake) and is ignored. It is | ||
| /// further assumed the array holds argc+1 elements with the C | ||
| /// string at index argc being a terminating null pointer. | ||
| /// This parameter is parsed based on optstring. |
There was a problem hiding this comment.
Again, \p optstring, and s/parameter/argument/ ("argv" is a parameter, but what gets parsed is the actual argument).
src/util/cmdline.h
Outdated
| /// \param argv An array of C strings. | ||
| /// The 0th element is assumed to be the name of the command as | ||
| /// it was invoked (e.g. /usr/bin/cmake) and is ignored. It is | ||
| /// further assumed the array holds argc+1 elements with the C |
src/util/cmdline.h
Outdated
| /// and ')', both of which can be optionally followed by a | ||
| /// single ':' indicating that the option takes a parameter, | ||
| /// if not present it does not. Parameters must be in the | ||
| /// next array element, except for short options whose |
There was a problem hiding this comment.
next array element in argv
src/util/cmdline.h
Outdated
| /// surrounded by a matching pair of '(' and ')' signifying a | ||
| /// "long" option with the name being the string between '(' | ||
| /// and ')', both of which can be optionally followed by a | ||
| /// single ':' indicating that the option takes a parameter, |
There was a problem hiding this comment.
In my opinion, all further uses of "parameter" should be replaced by "argument" here.
hannes-steffenhagen-diffblue
force-pushed
the
refactor/split-up-cmdline-parse
branch
from
8e9c61b to
48adfaf
Compare
hannes-steffenhagen-diffblue
force-pushed
the
refactor/split-up-cmdline-parse
branch
from
48adfaf to
86baa33
Compare
cmdlinet::parsefirst parses the command line specification, and then the command line arguments themselves. This PR splits those two bits up into their own functions (this just moves code around, the only reason the functions look slightly different is because ofclang-format).