D14436: SwitchDesktop mousewheel options with config dialog added

Thomas Otto noreply at phabricator.kde.org
Mon Jul 30 08:06:54 BST 2018


totto marked an inline comment as done.
totto added a comment.


  > 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 :)

INLINE COMMENTS

> davidedmundson wrote in desktop.cpp:36
> I have this coming: 
> https://phabricator.kde.org/D13034

Great, will keep an eye on that.

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

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.

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/9f6426be/attachment-0001.html>


More information about the Plasma-devel mailing list