CmdLineParser: various refactorings and cleanups as well as testing improvements#5676
CmdLineParser: various refactorings and cleanups as well as testing improvements#5676firewave merged 6 commits intocppcheck-opensource:mainfrom
Conversation
|
Still requires a couple of more tests and also requires #5672 to be merged first. |
| else if (std::strcmp(argv[i], "-h") == 0 || std::strcmp(argv[i], "--help") == 0) { | ||
| mPathNames.clear(); | ||
| mShowHelp = true; | ||
| // TODO: make this an exclusive option |
There was a problem hiding this comment.
"exclusive" option means that it should be processed separately so e.g. --library=missing-lib --help would not bailout with a library error but simply shows the help text. So we could scan for those before before handling all the others.
I tried a few different commands and the behavior is not consistent. Some just do the "left-to-right" until they hit the bail out options and others treat them as truly exclusive. I would like to go with the latter but that will be done in an upcoming PR.
| ASSERT_EQUALS("The Product\n", logger->str()); // TODO: include version? | ||
| } | ||
|
|
||
| // TODO: test extraVersion |
There was a problem hiding this comment.
That appears to be unused. Does premium make use of that?
There was a problem hiding this comment.
No premium does not use extraVersion. I don't remember why we added it maybe it's for "dev" or sha or something..
| "}\n"); | ||
| const char * const argv[] = {"cppcheck", "--version"}; | ||
| ASSERT(parser->parseFromArgs(2, argv)); | ||
| ASSERT_EQUALS("The Product\n", logger->str()); // TODO: include version? |
There was a problem hiding this comment.
The product name should not contain the version number since it is being use as simply the product name is other places. I think we should include the version here.
As different builds might also make use of a different version we should have a way to specify this easily. I do not think the cppcheck.cfg would be the way to specify that.
| if (getShowErrorMessages()) { | ||
| XMLErrorMessagesLogger xmlLogger; | ||
| std::cout << ErrorMessage::getXMLHeader(mSettings.cppcheckCfgProductName); | ||
| ErrorLogger& log = xmlLogger; |
There was a problem hiding this comment.
I chose this so we keep using the interface. The actual implementation is private and I would like to keep it that way.
| // print all possible error messages.. | ||
| else if (std::strcmp(argv[i], "--errorlist") == 0) { | ||
| mShowErrorMessages = true; | ||
| mSettings.xml = true; |
There was a problem hiding this comment.
This was unnecessary and might have interfered with other options since we proceed with "left-to-right" rpocessing.
| CppCheck::getErrorMessages(xmlLogger); | ||
| std::cout << ErrorMessage::getXMLFooter() << std::endl; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
We can return here now since we no longer rely on other options. --verbose never had an effect since we print it as XML which always includes the verbose message.
|
|
||
| // TODO: test --errorlist with product name | ||
|
|
||
| void errorlistverbose1() { |
There was a problem hiding this comment.
No longer necessary since we no longer rely on --verbose which actually never had any effect at all. Possibly a very old left-over. I did not check the history when this changed.
firewave
force-pushed
the
cmd-xxy
branch
5 times, most recently
from
332861d to
e567fcf
Compare
firewave
changed the title
…cessary handling for `--errorlist` with `--verbose`
| "}\n"); | ||
| const char * const argv[] = {"cppcheck", "--version"}; | ||
| ASSERT(parser->parseFromArgs(2, argv)); | ||
| // TODO: somehow the config is not loaded on some systems |
There was a problem hiding this comment.
I will look into this in the PR which adds proper error reporting to the config loading function.
2 participants
No description provided.