Details
- Reviewers
hein graesslin - Commits
- R127:10b00a219e9f: Add "SkipSwitcher" to API
Diff Detail
- Repository
- R127 KWayland
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Following discussion in BUG 375921, it was decided to add SkipSwitcher to the API. This is my first submission to the KWin/KWayland/KWindowSystem group, and @hein has graciously offered to help me out.
As Martin wrote in the bug report, this should be straightforward, following the implementation of SkipTaskbar. For the most part, it is - the autotest results include
PASS : TestWindowManagement::testRequestsBoolean(skipSwitcher)
However, autotest #6 (kwayland-testWaylandSurface) is failing with an exception. I got the same exception when testing KWayland without my changes. I'm attaching the test output, in case it helps figure out what's wrong. All the other tests succeed.
| src/client/plasmawindowmodel.h | ||
|---|---|---|
| 110 | I normally update those just before pushing to the next frameworks release number. | |
| src/client/protocols/plasma-shell.xml | ||
| 339 | The since is wrong. It must be higher than the current version. You also need to increase the version number and I think this should be added as last request and not in between. | |
| src/client/protocols/plasma-window-management.xml | ||
| 49 | Here also the since is wrong | |
There is one more file which need changes. In src/client/registry.cpp you need to increase the version numbers of the two changed protocols. Adding a new request can be really cumbersome.
| src/server/plasmashell_interface.cpp | ||
|---|---|---|
| 53 | you need to increase the version number. | |
| 168 | This is at the wrong position. It needs to be the last entry in the list. That might be your compile problem you mentioned in the KWin request. | |
| src/server/plasmawindowmanagement_interface.cpp | ||
| 122 | you need to increase the version number. | |
The adjustments in registry.cpp are still missing.
| src/server/plasmawindowmanagement_interface.cpp | ||
|---|---|---|
| 122 | this needs to be 9 | |
I apologize, but I don't know what adjustments to make in registry.cpp. In all other cases, I followed the templates for SkipPager or SkipTaskbar. I don't find any reference to those two functions in registry.cpp.
D11924 and D11926 have been pushed, so kwin master branch builds are now broken with:
14:49:26 /home/jenkins/workspace/Plasma kwin kf5-qt5 SUSEQt5.10/abstract_client.cpp: In member function 'void KWin::AbstractClient::setupWindowManagementInterface()':
14:49:26 /home/jenkins/workspace/Plasma kwin kf5-qt5 SUSEQt5.10/abstract_client.cpp:712:8: error: 'class KWayland::Server::PlasmaWindowInterface' has no member named 'setSkipSwitcher'; did you mean 'setSkipTaskbar'?
14:49:26 w->setSkipSwitcher(skipSwitcher());
14:49:26 ^~~~~~~~~~~~~~~
14:49:26 setSkipTaskbar
14:49:26 /home/jenkins/workspace/Plasma kwin kf5-qt5 SUSEQt5.10/abstract_client.cpp: In lambda function:
14:49:26 /home/jenkins/workspace/Plasma kwin kf5-qt5 SUSEQt5.10/abstract_client.cpp:728:16: error: 'class KWayland::Server::PlasmaWindowInterface' has no member named 'setSkipSwitcher'; did you mean 'setSkipTaskbar'?
14:49:26 w->setSkipSwitcher(skipSwitcher());
14:49:26 ^~~~~~~~~~~~~~~
14:49:26 setSkipTaskbar
https://build.kde.org/job/Plasma%20kwin%20kf5-qt5%20SUSEQt5.10/2/console
Observed this on the openSUSE OBS as well. Please fix the compilation issue or revert.
For now I reverted the commit in kwin to allow building. Once this review is merged, it can be re-instated.
Thank you for reverting the commit. The one - and only - reason I committed it was because it was flagged as approved and ready to land. I apologize for the chaos. I'll work with my reviewers to straighten the rest of this out.
any update on this? It's just the version increase in registry.cpp which is missing...
wonderful! Thanks to you two :-) And yes as soon as you push this change you can push the kwin change.