Differential D17318

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

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

Details

Reviewers

cullmann

Group Reviewers

Kate

Commits

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

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

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 5646
Build 5664: arc lint + arc unit

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

Harbormaster completed remote builds in B5646: Diff 46727.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

+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
105	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.

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

Replace info label with a much smaller button which shows a tooltip on click
comments

Harbormaster completed remote builds in B5812: Diff 47109.Dec 8 2018, 1:29 PM

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
105	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)

minor comment change

Harbormaster completed remote builds in B5813: Diff 47110.Dec 8 2018, 1:37 PM

Closed by commit R40:dcbed2c39f08: Code Analysis: Show list of file extensions on which the tool runs on (authored by gregormi). · Explain WhyDec 8 2018, 1:40 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			addons/project/kateprojectcodeanalysistool.h (7 lines)
M			addons/project/kateprojectinfoviewcodeanalysis.h (9 lines)
M			addons/project/kateprojectinfoviewcodeanalysis.cpp (28 lines)
M			addons/project/tools/kateprojectcodeanalysistoolcppcheck.h (2 lines)
M			addons/project/tools/kateprojectcodeanalysistoolcppcheck.cpp (9 lines)
M			addons/project/tools/kateprojectcodeanalysistoolflake8.h (2 lines)
M			addons/project/tools/kateprojectcodeanalysistoolflake8.cpp (7 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	46727	5fda417		Dec 2 2018, 5:57 PM	★	★
Diff 2	47109	5fda417	- Replace info label with a much smaller button which shows a tooltip on click	Dec 8 2018, 1:29 PM	★	★
Diff 3	47110	5fda417	- minor comment change	Dec 8 2018, 1:37 PM	★	★
Diff 4	47112	bfdd000	R40:dcbed2c39f08db730df406ff151deef536aeceee	Dec 8 2018, 1:40 PM	★	★