D14542: KCM using new virtual desktops DBus interface

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  I understand what you're doing with the syncing now, it makes sense in principle.
  Can you copy-paste what you typed to me into the code.

INLINE COMMENTS

> desktopsmodel.cpp:202
> +
> +    m_desktops[desktopIndex] = id;
> +    m_names[id] = name;

this is setting it to the value it already is

> desktopsmodel.cpp:251
> +            s_virtualDesktopsInterface,
> +            QStringLiteral("removeDesktop"));
> +

If I have 3 desktops with 3 IDs
id1 -> desktop1
id2 -> desktop2
id3 -> desktop3

and I delete desktop2

Here I delete 3 and then sync the names,  leaving me with:
id1 -> desktop1
id2 -> desktop3

it looks fine within the confines of this KCM, but as soon as we  rely on those IDs for external use (even just the fact that a user might have his windows on id3 and none on id2) it'll fall apart.

----------

When you make the change sending insert/remove instead of renaming we'll need to make sure we  do removal before insertion.

desktopCreated relies on the index of the newly created desktop to be in the same place as m_desktops has it.

If we insert first, the position will be off as the server will still have the about-to-be-deleted entries

REPOSITORY
  R108 KWin

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

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