D18458: Rewrite the KWin decorations settings as a ConfigModule.

zzag added inline comments.

INLINE COMMENTS

> previewbridge.cpp:133
> +    const auto offers = KPluginTrader::self()->query(s_pluginName, s_pluginName);
> +    auto item = std::find_if(offers.cbegin(), offers.cend(), [this](const auto &plugin) { return plugin.pluginName() == m_plugin; });
> +    if (item != offers.cend()) {

In KWin, we tend to use constBegin/constEnd.

> previewbutton.cpp:135
>      }
> -    m_button->paint(painter, QRect(0, 0, width(), height()));
> +    QRect size { 0, 0, (int)width(), (int)height() };
> +

Doesn't QQuickItem have size() method?

> previewbutton.h:62
> +    QColor color() const { return m_color; }
> +    void setColor(QColor color);
> +

const QColor &color

> decorationmodel.cpp:163
>                                                Q_ARG(QString, d.themeName));
> +
>                      m_plugins.emplace_back(std::move(d));

Unrelated whitespace change.

> decorationmodel.cpp:173
>          data.pluginName = info.pluginName();
> -        data.visibleName = info.name().isEmpty() ? info.pluginName() : info.name();
>          data.configuration = config;

It seems like that's unrelated change.

> decorationmodel.h:36
> +    enum DecorationRole {
> +        PluginNameRole = Qt::UserRole + 4,
> +        ThemeNameRole = Qt::UserRole + 5,

Can't we start from Qt::UserRole +1?

> kcm.cpp:52
> +    static const QString s_configBorderSize { QStringLiteral("BorderSize") };
> +    static const QString s_configCloseOnDoubleClickOnMenu { QStringLiteral("CloseOnDoubleClickOnMenu") };
> +    static const QString s_configDecoButtonsOnLeft { QStringLiteral("ButtonsOnLeft") };

Given that this is anonymous namespace you don't need `static`, also please don't indent contents of the namespace.

> kcm.cpp:89
>  {
> -    setupUi(this);
> +    KAboutData* about = new KAboutData(QStringLiteral("kcm_kwindecoration"),
> +                                       i18n("Configure Window Decorations"),

Use `auto`.

> kcm.h:32
>  {
> -class PreviewBridge;
> -class ButtonsModel;
> +    class DownloadDialog;
>  }

Please don't indent content of the namespace.

> kcm.h:37-46
> +    enum class BorderSize;
> +
> +    namespace Preview
> +    {
> +        class ButtonsModel;
> +    }
> +    namespace Configuration

Same here.

> utils.cpp:24
> +
> +namespace {
> +    static

Function implementations, class, struct and namespace declarations always have the opening brace on the start of a line.

> utils.cpp:25-26
> +namespace {
> +    static
> +    const QMap<QString, KDecoration2::BorderSize> s_borderSizes {
> +        { QStringLiteral("None"),      KDecoration2::BorderSize::None },

We don't need `static`. Same comments about indentation.

> utils.h:29
> +
> +class Utils
> +{

Please use namespace instead.

REPOSITORY
  R108 KWin

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

To: vpilo, #vdg, abetts, davidedmundson, #kwin
Cc: zzag, GB_2, ngraham, broulik, kwin, jraleigh, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, alexeymin, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart