-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 #13021: cmdline: validate option --std #6723
Conversation
The tests seem to fail because they use '--std=cplusplus11'. Should I change the parsing to accept the 'cplusplus' prefix in addition to 'c++'? |
No, just turn the |
So this line now results in |
sorry for late reply. |
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 basically like this so make the CI happy.
For your info you can run a specific test class like this:
testrunner TestClass
You can run a specific test function like this:
testrunner TestClass::testcase
Ok, seems to work now. I'm still unsure about what to do with the 'gnu++11' options generated by CMake in the self check though. Would it be appropriate to sed it to c++11 in the CI script as a quick fix? |
I think that users might have to sed sometimes. however it wouldn't hurt to handle gnu++11 in the importproject directly. using |
Shouldn't we be consistent here, i.e. handle |
Perhaps handle it if a certain cli option is set (e.g. —allow-gnu)? Adds flexibility but prevents any misunderstandings. |
my thinking was that why would you really want to use in a compile_commands.json file it's a different story. That is not a cppcheck option but a gcc option. And I assume it's not very uncommon. |
I have a commit that handles gnu++XX as c++XX in command files, should I push? |
That's fine with me. |
Sorry for the late reply but I have been out sick and still not fully recovered. This is possibly all wrong. The whole knowledge about what is a valid standard already exists in simplecpp and that needs to be leveraged. Since it is handled differently in Cppcheck I added the TODO. I cannot really tell if I just didn't have the time to implement it (and then forgot) or if there was something more problematic preventing this. This also needs tests for the project import. |
hmm you mean to utilize |
Yes, at least the knowledge. I did not have a look yet. IIRC there is also some inconsistencies with the later on usage. I might not have slapped a TODO on everything. I think this should done incrementally. Start with the CLI parameter bailing out based on the existing logic. Then plumping in the simplecpp logic and afterwards address the project import part (as already mentioned that needs some test coverage - and maybe quite some of it). |
yes that is acceptable to me also. we don't have to change the standards.h or importproject at all it's enough with cmdlineparser and testcmdlineparser.. |
I suppose this should be done in a new pull request? |
That is up to you. You can keep this and just remove the stuff that is deemed out of scope for the first step. |
I created a new pull request. |
Command line parsing fails with an error on commands such as