Expose locked keystates on KModifierKeyInfo when on wayland
AbandonedPublic

Authored by apol on Apr 1 2019, 11:53 PM.

Details

Reviewers
romangg
dfaure
Summary

Instead of deciding at build time which backend to use, see which is
used upon construction.
This will make it possible to have an alternative wayland
implementation.

Test Plan

See D20442

Diff Detail

Repository
R273 KGuiAddons
Branch
wayland_keystate
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10355
Build 10373: arc lint + arc unit
apol created this revision.Apr 1 2019, 11:53 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 1 2019, 11:53 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
apol requested review of this revision.Apr 1 2019, 11:53 PM
apol updated this revision to Diff 55250.Apr 1 2019, 11:54 PM

description

dfaure added a subscriber: dfaure.Apr 2 2019, 12:34 PM

Still no description visible here :)

apol retitled this revision from Depends on D20191 to Expose locked keystates on KModifierKeyInfo when on wayland.Apr 2 2019, 4:00 PM
apol edited the summary of this revision. (Show Details)
apol edited the summary of this revision. (Show Details)Jun 20 2019, 2:42 PM
apol edited the test plan for this revision. (Show Details)
romangg accepted this revision.Jun 20 2019, 2:48 PM
romangg added a subscriber: romangg.

Please add description, otherwise let's try it out.

This revision is now accepted and ready to land.Jun 20 2019, 2:48 PM
dfaure requested changes to this revision.Jun 22 2019, 7:56 AM
dfaure added inline comments.
src/keystate.xml
22

Interesting description. Does the number of 'x' mean anything? ;)

src/util/kmodifierkeyinfoprovider_wayland.cpp
36

static

50

static

This revision now requires changes to proceed.Jun 22 2019, 7:56 AM
zzag added a subscriber: zzag.Jun 22 2019, 9:24 AM
zzag added inline comments.
src/util/kmodifierkeyinfo.cpp
34–44

You don't need elses, 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;
}
src/util/kmodifierkeyinfoprovider.cpp
90

Abuse of auto.

92

Same here.

src/util/kmodifierkeyinfoprovider_wayland.cpp
36

Needs to be a static function instead.

38–45
src/util/kmodifierkeyinfoprovider_wayland.h
29

Indent it.

dfaure added inline comments.Jun 22 2019, 10:31 PM
src/util/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.

alexeymin added inline comments.
src/util/kmodifierkeyinfoprovider.cpp
90

There is a Qt recommendation though: https://wiki.qt.io/Coding_Conventions#auto_Keyword

apol abandoned this revision.Jun 23 2019, 8:42 AM