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
