D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 44671. hein added a comment. - Sync ids before syncing names. - Remove the dummy id swap, it's no longer needed given the above. - Add a class description comment that illuminates the sync/detach behavior. - Reindent headers. REPOSITORY R108 KWin

D14542: KCM using new virtual desktops DBus interface

zzag added inline comments. INLINE COMMENTS > desktopsmodel.cpp:253 > + > +call.setArguments({m_desktops.last()}); > + Is m_desktops.last() the right one? It can be already deleted. For example, If user has two virtual desktops: - Desktop 1 - Pizza and he or she wants to delete

D14542: KCM using new virtual desktops DBus interface

zzag requested changes to this revision. zzag added inline comments. INLINE COMMENTS > desktopsmodel.cpp:413 > +// so we can determine when we are in sync. > +const QString = m_desktops.at(data.position); > +m_names.remove(dummyId); Is this right? If I remove a desktop

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 >

D14542: KCM using new virtual desktops DBus interface

hein added a comment. In D14542#352260 , @zzag wrote: > Please wait for others. > > When you're about to push, please: > > - re-title this patch to "[kcmkwin/desktop] Use new ..."; > - shift access modifiers and everything that goes

D14542: KCM using new virtual desktops DBus interface

zzag accepted this revision. zzag added a comment. Please wait for others. When you're about to push, please: - re-title this patch to "[kcmkwin/desktop] Use new ..."; - shift access modifiers and everything that goes below 4 spaces to the left (that's one of my inline comments).

D14542: KCM using new virtual desktops DBus interface

hein added a comment. Should be OK now 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, lesliezhai,

D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 44498. hein added a comment. Try to remove Marco's revision REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14542?vs=44095=44498 BRANCH arcpatch-D14542_1 REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED

D14542: KCM using new virtual desktops DBus interface

zzag added a comment. It looks like this revision now also includes Marco's patch. 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,

D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 44095. hein added a comment. - Fix Exec - Clean up KCM titles REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14542?vs=43814=44095 BRANCH arcpatch-D14542_1 REVISION DETAIL https://phabricator.kde.org/D14542 AFFECTED

D14542: KCM using new virtual desktops DBus interface

zzag added inline comments. INLINE COMMENTS > kcm_kwin_virtualdesktops.desktop:2 > [Desktop Entry] > +Exec=kcmshell5 kwn_virtualdesktops > +Icon=preferences-desktop Is it a typo? > main.qml:30 > + > +ConfigModule.quickHelp: i18n("Virtual Desktops NG") > + I suppose we don't need NG

D14542: KCM using new virtual desktops DBus interface

hein added a comment. Ping? 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, lesliezhai, ali-mohamed,

D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 43814. hein edited the summary of this revision. hein added a comment. Drop the TODO note from the message. REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14542?vs=43315=43814 BRANCH arcpatch-D14542 REVISION DETAIL

D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 43315. hein added a comment. Drop FIXME from spacer on Marco's advice. REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14542?vs=43181=43315 BRANCH arcpatch-D13887_2 REVISION DETAIL https://phabricator.kde.org/D14542

D14542: KCM using new virtual desktops DBus interface

mart added a comment. In D14542#339584 , @hein wrote: > @mart There's a last FIXME in the QML code for a spacing hack (mentioned as a todo in the description). Could you give me a hand with that one? to me is not an hack, it's how

D14542: KCM using new virtual desktops DBus interface

hein updated this revision to Diff 43181. hein retitled this revision from "WIP: Basic KCM using new virtual desktops DBus interface" to "KCM using new virtual desktops DBus interface". hein added a reviewer: zzag. hein removed a subscriber: zzag. hein added a comment. - Remove hack around

D14542: KCM using new virtual desktops DBus interface

hein added a comment. @mart There's a last FIXME in the QML code for a spacing hack. Could you give me a hand with that one? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D14542 To: hein, mart, davidedmundson, ltoscano, zzag Cc: davidedmundson, broulik, plasma-devel,