This patch fixes FilteredProblemStore - now it uses the value of current scope during problems filtration. Old version ignores scope value, which makes scope filtration GUI elements (provided by ProblemsView) completely useless.
Details
Tested with standard ProblemReporterPlugin (uses own ProblemReporterModel) and with kdev-cppcheck plugin (uses standard ProblemModel). For both cases everybody works fine: grouping, scope filtration, 'show imports' filter.
Diff Detail
- Repository
- R33 KDevPlatform
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
Just tested. This breaks the combination of "Show Imports" + "Scope: Current Document" in the C++ problem view. The problem view no longer shows issues in the includes of the current doc.
New version implements 'show imports' components directly inside WatchedDocumentSet (sub)classes. This allows to use this filtration with any problem models without additional code. Currently it was implemented only for ProblemReporterModel.
For ProblemsView class also 'show imports' action fixed, which has wrong state after changing from one problem model to another and back to first. Old code don't reset action's state to correct value, which can leads to situation when model has 'show imports' property equals to false but action has true.
this is the wrong direction, we want to get rid of foreach in the long run:
plugins/problemreporter/problemreportermodel.cpp | ||
---|---|---|
15 | this is wrong, docs is const, and range-based-for on a const container does not detach. please revert this stuff | |
shell/watcheddocumentset.cpp | ||
198 | qAsConst, keep range-based-for |
General notice: I feel like this patch is way too invasive for this fix.
I need to review in detail, don't have time for this at the moment.
shell/watcheddocumentset.cpp | ||
---|---|---|
198 | Quick note: We can't use that, qAsConst is Qt 5.7 material. |
Yes, current patch is not so small as first version. But I tried to fix scope and "show imports" filtrations by "right way", which allows to use such abilities with any problem models.
@kfunk, I add some small fixes. Main change - now for Project-sets we adds project root to document set. It is useful for cppcheck plugin - it's generate some errors without location field and I set project root as location for them.
My testing don't show any problems and regressions from original code.
Sorry for the late review.
Also needs a rebase on current master.
shell/problemstore.cpp | ||
---|---|---|
206 | Should the getter/setter just relay to functions from d->m_documents (so you don't need to have m_showImports in ProblemStore)? | |
shell/problemstore.h | ||
122 | Typo | |
123 | Make 'const' | |
shell/watcheddocumentset.cpp | ||
39 | Missing Q_OBJECT | |
70 | Here and below: Introduce a flag for doUpdate & doEmit? | |
74 | Factor out the next four lines into a separate doUpdate(MyFlag) function? | |
170 | Initialize | |
shell/watcheddocumentset.h | ||
52 | Add getter, too, for symmetry | |
143 | Style: Add newline before |
inline comments fixes
shell/problemmodel.h | ||
---|---|---|
153 |
Done. |
Causes issues in unit tests, please fix:
https://build.kde.org/view/Kdevelop/job/kdevplatform%20master%20kf5-qt5/165/PLATFORM=Linux,compiler=gcc/testReport/junit/(root)/TestSuite/test_filteredproblemstore/
QFATAL : TestFilteredProblemStore::testNoGrouping() ASSERT: "d->m_documents" in file /home/jenkins/sources/kdevplatform/kf5-qt5/shell/problemstore.cpp, line 228
FAIL! : TestFilteredProblemStore::testNoGrouping() Received a fatal error.