D14542: WIP: Basic KCM using new virtual desktops DBus interface

davidedmundson added a comment.


  QML and the rest is all fine.
  
  I don't understand why desktopmodel is the way it is.
  
  There are 2 DBus patterns one could do here.
  
  - we buffer all changes in a model locally, when the user clicks save we apply them on the server and recall initialise.
  - we do things async-realtime. The model is always in sync with remote changes. When the user clicks create/remove we send a request to the server; the model only updates when it gets the callback.
  
  This seems to be doing both patterns at once.

INLINE COMMENTS

> desktopsmodel.cpp:402
> +    // If the user didn't make any changes, we can just stay in sync.
> +    if (!m_userModified) {
> +        beginInsertRows(QModelIndex(), data.position, data.position);

but if it is userModified don't we need to update the ID to be the non-dummy value in case that entry is then later removed?

> desktopsmodel.cpp:416
> +{
> +    const int desktopIndex = m_serverSideDesktops.indexOf(id);
> +

needs a guard.

could be emitted before the first load finishes

> org.kde.kwin.virtualdesktopmanager.xml:1
> +<interface name="org.kde.KWin.VirtualDesktopManager">
> +    <signal name="desktopCreated">

You're not using this file?

I would strongly suggest generating the iface, it makes the C++ a lot nicer and you get compile errors if the iface ever changes.

REPOSITORY
  R108 KWin

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

To: hein, mart, davidedmundson, ltoscano
Cc: zzag, davidedmundson, broulik, plasma-devel, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart