Automatic code linting using clang-tidy.
ClosedPublic

Authored by abika on Oct 6 2018, 7:19 PM.

Details

Summary

Fixes some 1000+ code style and performance warnings.

The drawback is that it pollutes the 'git annotate' views.

All changes:
Replaced all null pointer constants with C++11 'nullptr' keyword
Use the 'auto' specifier for variable declaration
Replace 'virtual' by 'override' keyword
Replace default constructor/destructor bodies with 'default' keyword
Replace old-style for loops by range-based loops
Use const reference as parameter and std::move() if possible
Replace static by dynamic casts for downcasts
Use const reference instead of copy initialization where possible
Use const reference for loop variables where possible
Remove redundant member initializations in constructors

Test Plan

Tested basic Krusader functionality (start/stop, browsing,
moving/copy/rename/files, search, etc.

Diff Detail

Repository
R167 Krusader
Branch
my-clang-fixes-arc
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3551
Build 3569: arc lint + arc unit
abika requested review of this revision.Oct 6 2018, 7:19 PM
abika created this revision.
yurchor added a subscriber: yurchor.Oct 6 2018, 7:26 PM
yurchor added inline comments.
CMakeLists.txt
10 โ†—(On Diff #42978)

This part seems to be unrelated.

131 โ†—(On Diff #42978)

Is this needed to remove this part?

abika updated this revision to Diff 42982.Oct 6 2018, 7:33 PM
  • fixup! Automatic code linting using clang-tidy.
yurchor accepted this revision as: yurchor.Oct 6 2018, 7:34 PM

Thanks.

This revision is now accepted and ready to land.Oct 6 2018, 7:34 PM
abika added a comment.Oct 6 2018, 7:36 PM

Thanks Yuri! That should have not been included in here.

abika marked 2 inline comments as done.Oct 6 2018, 7:37 PM
nmel requested changes to this revision.Oct 16 2018, 7:13 AM
nmel added a subscriber: nmel.

Other than minor things noted inline, looks good! Thanks Alex.

krArc/krarc.cpp
74

Q_DECL_OVERRIDE here and other places or we are moving to direct override across the board as we are in C++11 world already?

krusader/Panel/krsearchbar.cpp
219

Why? Can you elaborate in the comment? IMO, we should use dynamic cast here as we haven't checked event type yet.

This revision now requires changes to proceed.Oct 16 2018, 7:13 AM
abika updated this revision to Diff 51697.Feb 14 2019, 4:59 PM
  • Panel: Simplify handling escape key to close search bar when panel is focused
abika marked an inline comment as done.Feb 14 2019, 5:02 PM

Finally ...

Please check again.

krArc/krarc.cpp
74

Maybe I do not understand the question but why not? C++11 is mandatory for a long time now and you added override to krusader/icon.cpp yourself.

krusader/Panel/krsearchbar.cpp
219

Well, I do not know anymore why I have this code here to begin with.
Handling the escape key while the panel is focused and the searchbar is open is handled later one by handleKeyPressEvent() anyway so I removed this here.

nmel accepted this revision.Feb 18 2019, 7:29 AM

Thanks for improving the code quality, Alex!

krArc/krarc.cpp
74

Just wanted to confirm. I forgot that I switched to plain override myself. ;)

krusader/Panel/krsearchbar.cpp
219

Indeed. I checked that it works fine. Thanks for cleaning.

This revision is now accepted and ready to land.Feb 18 2019, 7:29 AM
Closed by commit R167:641b9b49582a: Merge branch 'my-clang-fixes' (authored by Alexander Bikadorov <alex.bikadorov@kdemail.net>). ยท Explain WhyFeb 24 2019, 4:45 PM
This revision was automatically updated to reflect the committed changes.
abika marked an inline comment as done.Feb 24 2019, 4:46 PM

Thanks for the review!