[Differential] [Changed Subscribers] D3089: WIP: Restore global menu support

[graesslin (Martin Gräßlin)]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161018/94708adb/attachment.html>