Cast to AbstractClient instead of Client in Toplevel::setReadyForPainting
ClosedPublic

Authored by graesslin on Jan 1 2019, 1:50 PM.

Details

Summary

TabGroup is nowadays in AbstractClient so the cast to Client is
incorrect.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin created this revision.Jan 1 2019, 1:50 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 1 2019, 1:50 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
graesslin requested review of this revision.Jan 1 2019, 1:50 PM
zzag added a subscriber: zzag.EditedJan 1 2019, 2:02 PM

Off-topic: what should we prefer

auto foo = qobject_cast<Foo*>(bar);

or

auto foo = qobject_cast<Foo *>(bar);

?

toplevel.cpp
265

While we're on this, would it be feasible to use qobject_cast instead?

zzag accepted this revision.Jan 1 2019, 2:23 PM
This revision is now accepted and ready to land.Jan 1 2019, 2:23 PM
In D17898#384616, @zzag wrote:

Off-topic: what should we prefer

auto foo = qobject_cast<Foo*>(bar);

or

auto foo = qobject_cast<Foo *>(bar);

?

I tend to prefer the latter case with whitespace. But our code is so messed up in that area that I think both is fine. A little bit on KWin coding style history. KWin used to have it's own coding style, the Lubos style. You can still find it for example in ksmserver, which was also maintained by Lubos. The coding style was fine, the only problem was that nobody really understood it and Lubos didn't demand it to be correct. So the code was everything but not consistent. When I became maintainer I reformatted the code with astyle to follow kdelibs style. The conversion with such rules is not perfect, e.g. such cases are not covered. Nowadays tooling might be better. I could imagine that something clang based would be able to cover such cases. So in case such a tool exists I would be all for reformat once again.

toplevel.cpp
265

Is there any real advantage of qobject_cast?

davidedmundson added inline comments.
toplevel.cpp
265

Supposedly it's faster for QObjects.

Though not really enough that it'll make a noticeable difference, especially here.

zzag added a comment.EditedJan 1 2019, 5:58 PM

I could imagine that something clang based would be able to cover such cases. So in case such a tool exists I would be all for reformat once again.

Yes, there's a tool called clang-format[1]. Basically, we would need to create a style configuration file and then run

find . -name '*.h' -o -name '*.cpp' -exec clang-format -i -style=file {} \;

FWIW, it's quite easy to integrate clang-format with Vim[2], Emacs, KDevelop, etc.

[1] https://clang.llvm.org/docs/ClangFormat.html
[2] http://clang.llvm.org/docs/ClangFormat.html#vim-integration
[3] Qt Creator's .clang-format. https://github.com/qt-creator/qt-creator/blob/master/.clang-format
[4] Qt's .clang-format. https://github.com/qt/qt5/blob/5.12/_clang-format
[5] Style options. https://clang.llvm.org/docs/ClangFormatStyleOptions.html

This revision was automatically updated to reflect the committed changes.