[Differential] [Requested Changes To] D1989: Introduce QQuickItem to nest kwin_wayland

[graesslin (Martin Gräßlin)]

graesslin requested changes to this revision.
graesslin added a reviewer: graesslin.
graesslin added a comment.
This revision now requires changes to proceed.


  I would like to see a test case which verifies that it starts the kwin_wayland instance and terminates it again.

INLINE COMMENTS

> kwinplugin.cpp:24-25
> +{
> +    Q_ASSERT(uri == QLatin1String("org.kde.kwin.app"));
> +    qmlRegisterType<KWinQml>(uri, 1, 0,"KWinApp");
> +}

Also here I dislike the name KWinApp and kwin.app - that's confusing due to kwinApp() already having a meaning.

> kwinqml.cpp:34-35
> +
> +KWinQml::KWinQml()
> +{
> +}

the call to the parent class is missing

> kwinqml.cpp:79-81
> +    QStringList arguments;
> +    arguments << "--xwayland";
> +    kwinWayland->start(QStringLiteral(KWIN_INTERNAL_NAME_WAYLAND), arguments);

we also need to pass at least --socket with a useful name. Otherwise it cannot be started in a Wayland session.

> kwinqml.cpp:81
> +    arguments << "--xwayland";
> +    kwinWayland->start(QStringLiteral(KWIN_INTERNAL_NAME_WAYLAND), arguments);
> +}

how is this instance being terminated again?

> kwinqml.h:37
> +    Q_OBJECT
> +    Q_PROPERTY(QString socketName READ socketName WRITE setSocketName NOTIFY socketNameChanged)
> +

please add documentation

> kwinqml.h:40
> +public:
> +    KWinQml();
> +    ~KWinQml() override;

QQuickItem *parent = nullptr

REPOSITORY
  rKWIN KWin

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

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

To: bdhruve, bshah, #plasma_on_wayland, graesslin
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/20160701/ccb253f4/attachment.html>