graesslin added inline comments.
INLINE COMMENTS
> CMakeLists.txt:276-278
> +option(KWIN_BUILD_KAPPMENU "Enable building of KWin with application menu
> support" ON)
> +# cmake_dependent_option(KWIN_BUILD_KAPPMENU "Build without appmenu support"
> ON "KWIN_BUILD_DECORATIONS" FALSE)
> +
I'm for not having an option, just always build enable.
> CMakeLists.txt:453-454
> + # FIXME TODO figure out all the cmake magic
> +
> /home/kaiuwe/Projekte/kf5/plasma-workspace/appmenu/org.kde.kappmenu.xml
> appmenu_interface)
> + #${CMAKE_SOURCE_DIR}/plasma-workspace/appmenu/org.kde.kappmenu.xml
> appmenu_interface)
> +endif()
we won't be able to depend on plasma-workspace as plasma-workspace already
depends on KWin. So either the org.kde.kappmenu.xml needs to be copied to KWin
or kappmenu needs to go into an own repo which both kwin and plasma-workspace
can depend on.
> appmenu.h:45
> + bool hasMenu(xcb_window_t window);
> + void showApplicationMenu(const QPoint &pos, const xcb_window_t window);
> +
Even if it's just bringing the code back: an xcb_window_t in the api is a bad
idea considering wayland
> appmenu.h:48-50
> + void slotShowRequest(qulonglong wid);
> + void slotMenuAvailable(qulonglong wid);
> + void slotMenuHidden(qulonglong wid);
same here, wid is evil
> appmenu.h:54
> +private:
> + QList<xcb_window_t> m_windowsMenu;
> + OrgKdeKappmenuInterface *m_appmenuInterface;
and of course this one is also bad
REPOSITORY
rKWIN KWin
REVISION DETAIL
https://phabricator.kde.org/D3089
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: broulik, #plasma
Cc: graesslin, plasma-devel, kwin, lesliezhai, ali-mohamed, hardening,
jensreuterberg, abetts, sebas
