D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81491. bport added a comment. Simplify code arround QHash and QPair REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81386=81491 REVISION DETAIL https://phabricator.kde.org/D29231 AFFECTED FILES

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

zzag added a comment. Thanks, this patch looks good to me. Although it would be nice to see the kwin side before leaving a +1. :) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D29231 To: bport, zzag, davidedmundson, apol Cc: romangg, crossi, kde-frameworks-devel,

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

apol added inline comments. INLINE COMMENTS > keyboard_shortcuts_inhibit_interface.cpp:144 > +QPair key(surface, seat); > +if (m_inhibitors.contains(key)) { > +return m_inhibitors[key]; Just `return m_inhibitors.value({ surface, seat })` to save a look-up and a few lines of

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81386. bport added a comment. fix qobject macro indentation REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81385=81386 REVISION DETAIL https://phabricator.kde.org/D29231 AFFECTED FILES

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81385. bport added a comment. fix test, and take in consideration zzag feedback REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81361=81385 REVISION DETAIL https://phabricator.kde.org/D29231 AFFECTED FILES

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

romangg added inline comments. INLINE COMMENTS > zzag wrote in keyboard_shortcuts_inhibit_interface.cpp:129 > We probably need to ask folks in `#wayland` whether inhibitors outlive the > manager object. In general objects created through some interface can outlive the interface bind if not

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

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 >

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81361. bport added a comment. Use client not display for KeyboardShortcutsInhibitorInterface REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81358=81361 REVISION DETAIL https://phabricator.kde.org/D29231

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

apol added a comment. Code looks good overall, I'd say you'll get to polish the weird bits when developing the kwin side. In fact, this probably could be implemented within kwin considering what we discussed last week. We could remove the weirdness we have to keep its ABI. INLINE

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81358. bport marked an inline comment as done. bport added a comment. added const to wrong line REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81355=81358 REVISION DETAIL https://phabricator.kde.org/D29231

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport updated this revision to Diff 81355. bport added a comment. Cyril feedback REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29231?vs=81351=81355 REVISION DETAIL https://phabricator.kde.org/D29231 AFFECTED FILES autotests/server/CMakeLists.txt

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport edited the summary of this revision. 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

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

crossi added inline comments. INLINE COMMENTS > keyboard_shortcuts_inhibit_interface.cpp:92 > +{ > +return d->m_surface; > +} indentation is missing > keyboard_shortcuts_inhibit_interface.h:39 > +void setActive(bool active); > +bool isActive(); > +private: Should be const REPOSITORY

D29231: [WIP] Add keyboard_shortcuts_inhibit protocol

bport created this revision. bport added reviewers: zzag, davidedmundson, apol. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. bport requested review of this revision. REVISION SUMMARY Depends on D29054 . REPOSITORY