Code Analysis: Show list of file extensions on which the tool runs on
ClosedPublic

Authored by gregormi on Dec 2 2018, 5:57 PM.

Details

Summary

Reason: a long time I wondered how exactly this Code Analysis works and wasn't even aware of that for "cppcheck" .h files are _not_ scanned.

Plus:

  • Rearrange to GUI to make the label better fit (at least for my eye): move selector and start

button from the lower right to the upper left.

Before:

After:

Ideas for later:

  • Make it possible to run a tool for individual files
  • Another analyser which derives from the cppcheck class could be created: it specifically sets the language to C++ (in the original class the caption should be renamed to "cppcheck (C/C++) and also includes .h and .hpp files
  • Show the number of files the analyser was run on

Diff Detail

Repository
R40 Kate
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
gregormi created this revision.Dec 2 2018, 5:57 PM
Restricted Application added a project: Kate. · View Herald TranscriptDec 2 2018, 5:57 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
gregormi requested review of this revision.Dec 2 2018, 5:57 PM
gregormi edited the summary of this revision. (Show Details)Dec 2 2018, 6:00 PM
gregormi added a reviewer: Kate.
gregormi edited the summary of this revision. (Show Details)
gregormi retitled this revision from Code Analysis: Show file extentions where the tool runs on to WIP: Code Analysis: Show file extentions where the tool runs on.Dec 2 2018, 6:02 PM
ngraham added a subscriber: ngraham.Dec 2 2018, 7:17 PM

+1, I like it!

gregormi edited the summary of this revision. (Show Details)Dec 2 2018, 7:41 PM

I think this is fine. A minor comment from my side would be: I am not sure adding these kind of labels are alwas a good solution. I can see that it's useful to know which files are affected. Then again, once you know this, the label is just visual noise.

addons/project/kateprojectcodeanalysistool.h
62

Why not return a QStringList? The current version just forces parsing in other places. :)

...

Later: ok, now I see it's used in the regular expression. Still, I don't like how an implementation detail in a specific location (here: regex later) imposes design decisions here. And also in the UI, a label would rather be comma separated instead of | separated.

addons/project/kateprojectinfoviewcodeanalysis.cpp
111

Why are there leading spaces in the label? Usually, spacing is up to the QStyle.

addons/project/kateprojectinfoviewcodeanalysis.h
66

Everything in this file has comments. This function should have one as well.

cullmann accepted this revision.Dec 7 2018, 6:45 PM
cullmann added a subscriber: cullmann.

I think it improves the current state, please commit it, thanks!

This revision is now accepted and ready to land.Dec 7 2018, 6:45 PM
gregormi updated this revision to Diff 47109.Dec 8 2018, 1:29 PM
gregormi marked 2 inline comments as done.
  • Replace info label with a much smaller button which shows a tooltip on click
  • comments
gregormi added inline comments.Dec 8 2018, 1:29 PM
addons/project/kateprojectcodeanalysistool.h
62

I added a comment about this.

addons/project/kateprojectinfoviewcodeanalysis.cpp
111

Fixed by using a button which is much less obstrusive

gregormi retitled this revision from WIP: Code Analysis: Show file extentions where the tool runs on to Code Analysis: Show list of file extensions on which the tool runs on.Dec 8 2018, 1:33 PM
gregormi edited the summary of this revision. (Show Details)
gregormi updated this revision to Diff 47110.Dec 8 2018, 1:37 PM
  • minor comment change
This revision was automatically updated to reflect the committed changes.