Add ProblemModel::setPlaceHolderText() method
ClosedPublic

Authored by antonanikin on Mar 2 2017, 5:59 AM.

Details

Summary

New method clears current problems and add new "placeholder" item (problem) with selected properties. The method should be used to notify user about some events. For example, analyzer plugin can set placeholders at analysis state changes - started/finished without errors/etc.

The patch is used by D4816 revision, which fixes bug 375557 "No indication of cppcheck being run in background".

Test Plan

Tested with master branch

Diff Detail

Repository
R32 KDevelop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antonanikin created this revision.Mar 2 2017, 5:59 AM
mwolff requested changes to this revision.Mar 5 2017, 7:34 PM
mwolff added a subscriber: mwolff.
mwolff added inline comments.
shell/problemmodel.h
137 ↗(On Diff #12047)

I don't think that this should do many things at once. it should only set the placeholder text, but not clear anything.

when the model is empty, the placeholder should be shown

I also don't get why this needs to pass a source and location here? A single placeholder text should be enough?

This revision now requires changes to proceed.Mar 5 2017, 7:34 PM
antonanikin planned changes to this revision.Mar 6 2017, 2:52 AM
antonanikin edited edge metadata.
  • Update to new version
antonanikin marked an inline comment as done.Mar 13 2017, 12:10 PM
antonanikin added inline comments.
shell/problemmodel.h
137 ↗(On Diff #12047)

Fixed

I also don't get why this needs to pass a source and location here? A single placeholder text should be enough?

This useful for cases when some problem model is used by multiple tools - for example in Valgrind plugin it will be used for providing executed tool name (memcheck/helgrind/DRD).

mwolff requested changes to this revision.Mar 19 2017, 12:56 PM
mwolff added inline comments.
shell/problemmodel.cpp
70 ↗(On Diff #12430)

rename to createPlaceholdreProblem, make const

270 ↗(On Diff #12430)

join lines, follow the surrounding code style

279 ↗(On Diff #12430)
if (d->m_isPlaceholderShown || d->m_problems->count() == 0) {
    // clearing will show/update the new placeholder
    clearProblems();
}
313 ↗(On Diff #12430)

the begin/end reset is not needed anymore, done internally by setProblems already

shell/problemmodel.h
147 ↗(On Diff #12430)

I still don't understand what this location would point to. it's a placeholder, nothing has been found yet, so there cannot be any location?

Can you maybe show a screenshot of this in action?

This revision now requires changes to proceed.Mar 19 2017, 12:56 PM
antonanikin marked an inline comment as done.

Update to master

antonanikin planned changes to this revision.Feb 10 2018, 9:04 AM

Fix comments

antonanikin planned changes to this revision.Feb 10 2018, 10:43 AM

Fix comments

antonanikin marked 4 inline comments as done.Feb 10 2018, 11:02 AM
antonanikin marked an inline comment as done.Feb 10 2018, 11:14 AM

@mwolff:

I still don't understand what this location would point to. it's a placeholder, nothing has been found yet, so there cannot be any location?

Can you maybe show a screenshot of this in action?

This can be useful, for example, to specify the object being checked by the analyzer plugins:


antonanikin edited the summary of this revision. (Show Details)Feb 10 2018, 11:16 AM

but what if the analysis completed for multiple files and no errors where found there?

but what if the analysis completed for multiple files and no errors where found there?

  1. If we start analysis from editor context menu or from main "Code" menu we can choose "Current file" or "Current project", therefore location will be set to the file/project path.
  2. if we start analysis from "Projects" tool view, then location will be set to the selected item path (file/folder/project). If multiple items are selected in the "Projects" tool view, then the context menu does not contain a analysis submenu and ambiguity with the location will simply not occur.

So I cannot start the checker for two files concurrently (assuming both takes a longer time)?

So I cannot start the checker for two files concurrently (assuming both takes a longer time)?

No, current implementation of cppcheck plugin allows you to run check only for single tree item.

mwolff accepted this revision.Feb 19 2018, 8:25 PM

ok, then let's go for it and we can improve this later on if needed

(sorry for the long delay in this review!)

This revision is now accepted and ready to land.Feb 19 2018, 8:25 PM
This revision was automatically updated to reflect the committed changes.