D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

[Vlad Zahorodnii]

zzag requested changes to this revision.
zzag added a comment.
This revision now requires changes to proceed.


  I don't want to be selfish, but I'm not really used to the coding style in this patch. Could you please move method definitions outside class declarations?

INLINE COMMENTS

> keyboard_shortcuts_inhibit_interface.cpp:25
> +    Private(wl_client *client, int id, SurfaceInterface *surface, SeatInterface *seat, KeyboardShortcutsInhibitorInterface* q)
> +        : zwp_keyboard_shortcuts_inhibitor_v1(client, id, s_version)
> +        , q(q)

What if the client requested version < s_version? You need to create the resource manually.

> keyboard_shortcuts_inhibit_interface.cpp:129
> +        Q_UNUSED(resource)
> +        // TODO ensure we delete all inhibitors
> +        delete q;

We probably need to ask folks in `#wayland` whether inhibitors outlive the manager object.

> keyboard_shortcuts_inhibit_interface.h:32
> +{
> +Q_OBJECT
> +public:

Indentation. Also, it's probably a minor thing, but it's really nice when Q_OBJECT is separated from code below by a single empty line (cosmetics).

> keyboard_shortcuts_inhibit_interface.h:60
> +     */
> +    KeyboardShortcutsInhibitorInterface *getShortcutsInhibitor(SurfaceInterface *surface, SeatInterface *seat) const;
> +

We probably don't need it to be public. Only emit inhibitorCreated.

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson, apol
Cc: crossi, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200427/66e4951f/attachment.html>