D20193: Expose locked keystates on KModifierKeyInfo when on wayland
apol abandoned this revision. REPOSITORY R273 KGuiAddons REVISION DETAIL https://phabricator.kde.org/D20193 To: apol, romangg, dfaure Cc: alexeymin, zzag, romangg, dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
alexeymin added inline comments. INLINE COMMENTS > dfaure wrote in kmodifierkeyinfoprovider.cpp:90 > Calling this an "abuse" is very arguable, see > https://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/ > I'm not aware of a KF5 policy about this, so it's fine as is. There is a Qt recommendation though: https://wiki.qt.io/Coding_Conventions#auto_Keyword REPOSITORY R273 KGuiAddons REVISION DETAIL https://phabricator.kde.org/D20193 To: apol, romangg, dfaure Cc: alexeymin, zzag, romangg, dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
dfaure added inline comments. INLINE COMMENTS > zzag wrote in kmodifierkeyinfoprovider.cpp:90 > Abuse of `auto`. Calling this an "abuse" is very arguable, see https://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/ I'm not aware of a KF5 policy about this, so it's fine as is. 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
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
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 = 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
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> keystate.xml:22
> +
> +xxx
> +
Interesting description. Does the number of 'x' mean anything? ;)
> kmodifierkeyinfoprovider_wayland.cpp:36
> +
> +KModifierKeyInfoProvider::ModifierState toState(state state)
> +{
static
> kmodifierkeyinfoprovider_wayland.cpp:50
> +
> +Qt::Key toKey(key key)
> +{
static
REPOSITORY
R273 KGuiAddons
REVISION DETAIL
https://phabricator.kde.org/D20193
To: apol, romangg, dfaure
Cc: romangg, dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
romangg accepted this revision. romangg added a comment. This revision is now accepted and ready to land. Please add description, otherwise let's try it out. REPOSITORY R273 KGuiAddons BRANCH wayland_keystate REVISION DETAIL https://phabricator.kde.org/D20193 To: apol, romangg Cc: romangg, dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
apol edited the summary of this revision. apol edited the test plan for this revision. REPOSITORY R273 KGuiAddons REVISION DETAIL https://phabricator.kde.org/D20193 To: apol Cc: dfaure, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
D20193: Expose locked keystates on KModifierKeyInfo when on wayland
apol retitled this revision from "Depends on D20191" to "Expose locked keystates on KModifierKeyInfo when on wayland". apol edited the summary of this revision. apol added a dependency: D20191: Proof of concept of a wayland protocol to allow the keystate dataengine to work. REPOSITORY R273 KGuiAddons REVISION DETAIL https://phabricator.kde.org/D20193 To: apol Cc: dfaure, kde-frameworks-devel, michaelh, ngraham, bruns
