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

[Kai Uwe Broulik]

broulik added a comment.


  That DBus stuff looks like it was painful to write :/
  
  It seems the KCM does auto-apply of everything (desktop names, adding, removing them) which is not what we usually do, and neither did the old KCM

INLINE COMMENTS

> desktopsmodel.cpp:92
> +        QStringLiteral("org.kde.KWin.VirtualDesktopManager"),
> +        QStringLiteral("rowsChanged"),
> +        this,

Architectural question: why don't you go the standard `org.freedesktop.DBus.Properties.PropertiesChanged` way?

> desktopsmodel.cpp:105
> +        QStringLiteral("/VirtualDesktopManager"),
> +        QStringLiteral("org.freedesktop.DBus.Properties"),
> +        QStringLiteral("Get"));

Can you put a couple of static `QString`s somewhere to avoid copy-pasting this all over?

> desktopsmodel.cpp:165
> +
> +        const int perRow = std::ceil((qreal)m_desktops.count() / (qreal)m_rows);
> +        return (index.row() / perRow) + 1;

`m_rows` can potentially be `0` here

> main.qml:160
> +
> +        QtControls.Button {
> +            Layout.alignment: Qt.AlignRight

Add `list-add` icon

> virtualdesktops.cpp:40
> +        i18n("Configure Virtual Desktops"),
> +        QStringLiteral("2.0"), QString(), KAboutLicense::GPL);
> +    setAboutData(about);

Copyright headers in the code say LGPL

REPOSITORY
  R108 KWin

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

To: hein, mart
Cc: broulik, plasma-devel, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180802/9b757562/attachment-0001.html>