davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed KWin.


  Cool approach. One minor thing that won't happen anyway.

INLINE COMMENTS

> dbusinterface.cpp:193
> +{
> +    m_replyQueryWindowInfo = message();
> +    setDelayedReply(true);

can you add:

if (m_replyQuery...) {sendError()}

otherwise if a user makes two calls without selecting a window inbetween the 
first call will just never get a response

> detectwidget.cpp:152
> +                                                          
> QStringLiteral("queryWindowInfo"));
> +    QDBusPendingReply<QVariantMap> async = 
> QDBusConnection::sessionBus().asyncCall(message);
> +

I would explicilty specify a massive timeout for anything which has user 
interaction the other side.

> ruleswidget.cpp:790
> +    CHECKBOX_PREFILL(below, , info.value("keepBelow").toBool());
> +    // noborder is only internal KWin information, so let's guess
> +    CHECKBOX_PREFILL(noborder, , info.value("noBorder").toBool());

This isn't a guess anymore?

REPOSITORY
  R108 KWin

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

To: graesslin, #kwin, #plasma
Cc: davidedmundson, broulik, plasma-devel, kwin, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, iodelay, 
bwowk, hardening

Reply via email to