SwitchDesktop mousewheel options with config dialog added
Needs ReviewPublic

Authored by totto on Jul 28 2018, 7:49 AM.


Group Reviewers

By default the mouse wheel action of switchdesktop always
wraps around/rolls over when switching virtual desktops.
Changed to read the corresponding kwinrc setting and behave
Also added the option to invert the direction of the mouse
wheel when switching.

Diff Detail

R120 Plasma Workspace
No Linters Available
No Unit Test Coverage
Build Status
Buildable 3540
Build 3558: arc lint + arc unit
totto created this revision.Jul 28 2018, 7:49 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 28 2018, 7:49 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
totto requested review of this revision.Jul 28 2018, 7:49 AM
totto added inline comments.

The rollover option is from another kde component (kwin), is there a way to get notified whenever this config changes?

davidedmundson requested changes to this revision.Jul 29 2018, 8:59 PM
davidedmundson added a subscriber: davidedmundson.

Showing a disabled checkbox for rollover doesn't make much sense.
IMHO we should either have it as a separate option from kwin and have a checkbox, or not have a checkbox at all.


Unforunately, that's not how our i18n works.

a script greps for i18n("some text here") to generate the english translations.


or just use a regular bool and copy the value in configurationAccepted

Long term, having simpler code tends to be much better than clever code.

This revision now requires changes to proceed.Jul 29 2018, 8:59 PM
totto marked an inline comment as done.Jul 30 2018, 7:06 AM

disabled checkbox for rollover

That was partially meant to overcome the shortcoming that changing the kwin value does not change this value until some settings dialog is opened, but with your patch this is no longer required.

Plus I wanted to avoid two options for the same thing and add some kind of hint where this behavior could be changed. Should I still mention it as plain text (though translating that might be complicated) or just leave users to figure it out themselves? Again, "go to first config gui of this key" would be nice :)


Great, will keep an eye on that.


Or, even simpler: I'll just drop the duplication between the in-class bool (i.e. the the cached value), and the m_options config bool and always look it up in m_options, should be fast enough.

Btw, would a std::shared_ptr have been Ok for once? That does not have the bug prone non-explicit bool problem and would work as expected.


if you turn this into a method you don't need the rollover/invert args


Btw, would a std::shared_ptr have been Ok for once?

TBH, I still would have considered it overkill. It's only two checkboxes, it doesn't need any custom design patterns.

If you did want to reduce the few lines of duplication, KConfigXT is the framework to look at for doing this sort of thing.

totto updated this revision to Diff 42962.Oct 6 2018, 10:40 AM
totto marked an inline comment as done.

new revision

totto added inline comments.Oct 6 2018, 10:50 AM

The m_kwinrcRollOverDesktops variable does not work at runtime because two instances of SwitchDesktop are created:

One when plasma-workspace/KDE is started, a second when the settings window is opened. The actual desktop switching is done by the former, but subsequent changes to the kwinrc/RollOverDesktops setting are only seen and set by the latter. So the desktop rollover setting remains what it was when KDE was started.

Is there a way to sync these without writing another config key? Or wait for "Add mechanism to notify other clients of config changes over DBus" / https://phabricator.kde.org/D13034 ?