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
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
Diff Detail
- Repository
- R33 KDevPlatform
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
This comment was removed by antonanikin.
Comment Actions
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.
Comment Actions
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 comment was removed by kfunk.
Comment Actions
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 | |
Comment Actions
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. | |
Comment Actions
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.
Comment Actions
@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.
Comment Actions
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 | |
Comment Actions
inline comments fixes
| shell/problemmodel.h | ||
|---|---|---|
| 153 |
Done. | |
Comment Actions
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.