The "included_files" variable should contain the include directive arguments
for the base classes of the created class.
Details
- Reviewers
mwolff kfunk - Group Reviewers
KDevelop - Commits
- R32:05cc5e8d6c93: Port ClangClassHelper to also fill the include_files var
Diff Detail
- Repository
- R32 KDevelop
- Branch
- portIncludePathCreationClangClassHelper
- Lint
No Linters Available - Unit
No Unit Test Coverage
lgtm in principle
languages/clang/codegen/clangclasshelper.cpp | ||
---|---|---|
279–280 | while at it: join line | |
languages/clang/duchain/unknowndeclarationproblem.cpp | ||
335 | fitIt?! what? :P here and below: remove excess KDevelop:: qualification (see above, namespace is used) | |
541 | needs a unit test, missing so far if I'm not mistaken util stuff is usually put into duchainutils | |
547 | look it up yourself :) potentially there is a fallback, alternatively do the fallback yourself. afaik the project managers will usually return $something for a folder as well. | |
560 | that is for finding an include from the name of a class (i.e. an identifier) - doesn't seem to be relevant here | |
567 | wrong comment |
Small update which picks up some first comments + adds minor improvements
more, like moving code around + tests, to come in the next days
only export UnknownDeclarationProblem::findMatchingIncludeFiles,
keep custom logic for not-existing files specific to classhelper
Still no matching unit tests added for now. There are two things that could see tests:
- the exposed UnknownDeclarationProblem::findMatchingIncludeFiles(...) method
- the included_files variable properly filled with data
Both things will need me to invest some more time, as I yet have to grasp things around DUChain enough to write a proper test for the exposed method.
And classhelper code is without any testing approach so far, from what I found (also nothing seen in oldcpp), so that needs some more thinking how to do this properly.
Given 5.1 is planned to be released this week and me lacking enough time for the above now, I propose to have the current patch in already without dedicated unit tests, as it fixes a regression for cpp->clang when it comes to code generation from file templates (missing included_files variable content). Given my dissatisfaction with the current code generation abilities, you should see me working on tests in the near future at least, as part of other development.
Rest LGTM, patch should go into 5.1.
The new code introduced in this patch will only be executed while creating from file templates, so there can't be potential regression in the Clang navigation / code completion support. But we'll fix generated code, which is good.
languages/clang/codegen/clangclasshelper.cpp | ||
---|---|---|
103 | Could turn that into a simple function, or even inline as lambda at the call-site | |
181 | auto i -> const auto& fileItem | |
192 | includepaths -> includePaths | |
201 | includefiles -> includeFiles |