D14436: SwitchDesktop mousewheel options with config dialog added

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> desktop.cpp:172
> +namespace {
> +void switchDesktop(bool next, bool rollover, bool invert) {
>      const int numDesktops = KWindowSystem::numberOfDesktops();

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

> totto wrote in desktop.h:54
> 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.

> 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D14436

To: totto, hein, broulik, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180730/bbc08cbe/attachment.html>