zzag added inline comments.
INLINE COMMENTS
> kmodifierkeyinfo.cpp:34-44
> +#ifdef WITH_XCB
> + if (qGuiApp->platformName() == QLatin1String("xcb"))
> + return new KModifierKeyInfoProviderXcb;
> + else
> +#endif
> +#ifdef WITH_WAYLAND
> + if (qGuiApp->platformName() == QLatin1String("wayland"))
You don't need `else`s, e.g.
static KModifierKeyInfoProvider *createProvider()
{
#ifdef WITH_XCB
if (qGuiApp->platformName() == QLatin1String("xcb")) {
return new KModifierKeyInfoProviderXcb;
}
#endif
#ifdef WITH_WAYLAND
if (qGuiApp->platformName() == QLatin1String("wayland")) {
return new KModifierKeyInfoProviderWayland;
}
#endif
return new KModifierKeyInfoProvider;
}
> kmodifierkeyinfoprovider.cpp:90
> +{
> + auto &state = m_modifierStates[key];
> + if (newState != state) {
Abuse of `auto`.
> kmodifierkeyinfoprovider.cpp:92
> + if (newState != state) {
> + const auto difference = (newState ^ state);
> + state = newState;
Same here.
> dfaure wrote in kmodifierkeyinfoprovider_wayland.cpp:36
> static
Needs to be a static function instead.
> kmodifierkeyinfoprovider_wayland.cpp:38-45
> + switch(state) {
> + case state_unlocked:
> + return KModifierKeyInfoProvider::Nothing;
> + case state_latched:
> + return KModifierKeyInfoProvider::Latched;
> + case state_locked:
> + return KModifierKeyInfoProvider::Locked;
https://techbase.kde.org/Policies/Frameworks_Coding_Style#Switch_Statements
> kmodifierkeyinfoprovider_wayland.h:29
> {
> -}
> +Q_OBJECT
> +public:
Indent it.
REPOSITORY
R273 KGuiAddons
REVISION DETAIL
https://phabricator.kde.org/D20193
To: apol, romangg, dfaure
Cc: zzag, romangg, dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham,
bruns
