D17265: Ensure consistency when the layout is changed via the dbus protocol

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dbusinterface.cpp:475
>      m_manager->createVirtualDesktop(position + 1, name);
> +    m_manager->save();
>  }

VirtualDesktopManager::createVirtualDesktop

calls save on line 464

> virtualdesktops.cpp:246
> +        for (auto *vd : m_desktops) {
> +            connect(vd, &VirtualDesktop::nameChanged, this,
> +                [this, vd]() {

we can get rid of this connect if we move the statement at ~468 to be outside if (m_rootInfo)

> virtualdesktops.cpp:629
>  {
>      return grid().height();
>  }

so is m_rows the canonical source or this?

> virtualdesktops.cpp:770
>  
> +    for (const int i : m_deskopsToRemoveFromConfig) {
> +        group.deleteEntry(QStringLiteral("Id_%1").arg(i));

I don't see why it would be an issue given we only load entries up to the value in "Number" anyway?
It wasn't an issue before?

If you do want to remove them, I'd rather something that uses the group as the data source. Then we don't need other changes and managing a second set of data. I think the current code breaks if you remove + add without calling save inbetween.

Something along the lines of:

  for (int i = count() + 1;  group.hasEntry("Id_" + i; i++) {
    group.deleteEntry("Id_" + i);
  }

REPOSITORY
  R108 KWin

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

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