fixed #13990/#13991 - reworked platform lookup #7639
Conversation
| if (!filesdir.empty()) { | ||
| if (filesdir.back() != '/') | ||
| filesdir += '/'; | ||
| // TODO: look in filesdir? |
There was a problem hiding this comment.
If we would also look into that folder it could just be one of the lookup paths above.
| std::string ppath = Path::fromNativeSeparators(path); | ||
| if (ppath.back() != '/') | ||
| ppath += '/'; | ||
| // TODO: look in platforms first? |
There was a problem hiding this comment.
As the platforms folder is the proper location for these files it is weird that we do not look there first.
There was a problem hiding this comment.
I filed https://trac.cppcheck.net/ticket/13992 about this.
| @@ -341,44 +341,40 @@ def test_platform_lookup_builtin(tmpdir): | |||
| ] | |||
|
|
|||
|
|
|||
| @pytest.mark.skip # TODO: performs additional lookups when run via symlink in CI | |||
There was a problem hiding this comment.
The problem are the CWD lookups because if you run the test from the root it contains the configuration folders and it finds them. If the tests are run from the test folder it will look into that first which does not contain them. So it makes me wonder even more if we should remove those lookups.
There was a problem hiding this comment.
I can understand that point.. but it should be possible to put cppcheck configuration files in a project folder. custom cppcheck configurations should not be stored in the cppcheck installation folders.
There was a problem hiding this comment.
I can understand that point.. but it should be possible to put cppcheck configuration files in a project folder.
It still is.
custom cppcheck configurations should not be stored in the cppcheck installation folders.
That is up to the user or the packager.
But that is not the "problem" I am referring to (and it is also not in the scope of this PR). It is about the usage of CWD in the lookup.
It will look in the folder you run Cppcheck from. So on Linux, if you have files in your user folder (like we support in the GUI). And you open a new shell - which by default will start in the user folder. And you run Cppcheck immediately - it might pick up the files in the user folder (because it is the current CWD) before others. In that case supporting the CWD seems rather unexpected and arbitrary.
But making the decision about that is what #7613 is about and I would like to have that discussion there.
firewave
changed the title
|
The platform loading code is now essentially what will be the shared code for all loading. There is still a few things to resolve and I need to check it against the code in the UI before moving to that. |
| # TODO: test with invalid file in project path | ||
| # TODO: test with non-file in project path |
There was a problem hiding this comment.
Will be addressed in a follow-up to not cramp more stuff into this.
Uh oh!
There was an error while loading. Please reload this page.