[Differential] [Commented On] D1989: Introduce QQuickItem to nest kwin_wayland

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> config-kwin.h.cmake:6
>  #define KWIN_INTERNAL_NAME_X11 "${KWIN_INTERNAL_NAME_X11}"
> +#define KWIN_INTERNAL_NAME_WAYLAND "${KWIN_INTERNAL_NAME_WAYLAND}"
>  #define KWIN_CONFIG "${KWIN_NAME}rc"

best generate the absolute install path of the binary. We don't want a different kwin_wayland to be started just because it's in $PATH.

> kwinqml.cpp:47
> +    m_display = new Display(this);
> +    if (!m_socketName.isNull() && !m_socketName.isEmpty()) {
> +        m_display->setSocketName(m_socketName);

why check isNull and isEmpty?

> kwinqml.cpp:56
> +    m_seat->create();
> +    m_shell = m_display->createShell(m_display);
> +    // output

you also need to call create on m_shell.

> kwinqml.h:38
> +    Q_OBJECT
> +    // socketName to pass to Wayland Server Display.
> +    Q_PROPERTY(QString socketName READ socketName WRITE setSocketName NOTIFY socketNameChanged)

that's a comment and no doxygen documentation. For documentation you need to use

  /**
   * Socket name to pass to Wayland Server Display
   **/

> kwinqml.h:45
> +
> +    void setSocketName(const QString socketName)
> +    {

const QStrig &socketName

> kwinqml.h:46-49
> +    {
> +        m_socketName = socketName;
> +        emit socketNameChanged(socketName);
> +    }

this also emits the changed if it does not change at all.

REPOSITORY
  rKWIN KWin

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: bdhruve, graesslin, #plasma_on_wayland, bshah
Cc: bshah, graesslin, plasma-devel, kwin, hardening, jensreuterberg, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160705/750938a0/attachment-0001.html>