Move the shortcut management in the settings object
ClosedPublic

Authored by ervin on Fri, Nov 8, 12:00 PM.

Details

Summary

This uses the new KPropertySkeletonItem facility to allow proper
integration with KGlobalAccel while keeping all the nice semantic coming
from KConfigXT.

This required turning the kconfig_compiler code into a base class and
inheriting from it to introduce the property managed via
KPropertySkeletonItem.

Diff Detail

Repository
R133 KScreenLocker
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ervin created this revision.Fri, Nov 8, 12:00 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFri, Nov 8, 12:00 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ervin requested review of this revision.Fri, Nov 8, 12:00 PM
ervin updated this revision to Diff 69435.Fri, Nov 8, 12:27 PM

Remove stale updateState() function

mart accepted this revision.Tue, Nov 12, 9:47 AM
This revision is now accepted and ready to land.Tue, Nov 12, 9:47 AM
kcm/kcm.cpp
198

This part appears to have gone?

kscreensaversettings.cpp
47

given we have defined the defaults at a kconfigskeleton level should we just call ->shortcut() here?

49

Could we handle the alternative shortcut by using a second property?

ervin added inline comments.Tue, Nov 12, 12:32 PM
kcm/kcm.cpp
198

Yes, it was never triggered. It turns out KKeySequenceWidget does that already as soon as you pick a new shortcut, so it could never end up conflicting here since said conflict would have been handled before.

kscreensaversettings.cpp
47

Not sure what you mean here. We're inside the KConfigSkeleton.

Note that KGlobalAccel seems to expect a first call to setShortcut to associate the local QAction with the information from KGlobalAccel. I admit KGlobalAccel looks like an odd beast to me.

49

We could but it's not exposed in the KCM, not sure what the gain would be.

davidedmundson added inline comments.Tue, Nov 12, 4:13 PM
kcm/kcm.cpp
198

win win \o/

kscreensaversettings.cpp
47

Note that KGlobalAccel seems to expect a first call to setShortcut to associate the local.

Oh, of course

I admit KGlobalAccel looks like an odd beast to me.

It is.

49

Fair answer :)

I was getting further ahead in my head and trying to port the other KCMs.

This revision was automatically updated to reflect the committed changes.
ngraham reopened this revision.Thu, Nov 14, 8:55 PM
ngraham added a subscriber: ngraham.

This patch causes a build failure for me:

/home/nate/kde/src/kscreenlocker/kcm/kcm.cpp: In constructor ‘ScreenLockerKcm::ScreenLockerKcm(QWidget*, const QVariantList&)’:
/home/nate/kde/src/kscreenlocker/kcm/kcm.cpp:64:47: error: no matching function for call to ‘KScreenSaverSettings::KScreenSaverSettings(ScreenLockerKcm*)’
   64 |     , m_settings(new KScreenSaverSettings(this))
      |                                               ^
This revision is now accepted and ready to land.Thu, Nov 14, 8:55 PM
ngraham closed this revision.Thu, Nov 14, 8:57 PM

Never mind, it just needed a clean build, my bad.