D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-10 Thread Vlad Zahorodnii 
zzag added a comment.


  In D27464#625403 , @broulik wrote:
  
  > Didn't we have a dedicated protocol for window menu in plasma-integration? 
Or is this just for reading? We surely don't want all apps to use plasma 
surface interface for announcing the global menu?
  
  
  Okay, let's step back. We have a proprietary protocol that is used to 
announce application menus. The problem is that only kwin knows that a 
particular window has that particular application menu. We need a way to share 
this information with a global menu applet.
  
  > We surely don't want all apps to use plasma surface interface for 
announcing the global menu?
  
  Could you please explain why we need to announce global menus?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: broulik, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-10 Thread Kai Uwe Broulik 
broulik added a comment.


  In D27464#625405 , @zzag wrote:
  
  > Could you please clarify what you mean by "plasma surface interface"?
  
  
  The thing we use in plasmashell to position panels and everything absolutely

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: broulik, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-10 Thread Vlad Zahorodnii 
zzag added a comment.


  In D27464#625403 , @broulik wrote:
  
  > Didn't we have a dedicated protocol for window menu in plasma-integration?
  
  
  That protocol is for announcing global menus.
  
  > Or is this just for reading?
  
  Yes, it's for reading.
  
  > We surely don't want all apps to use plasma surface interface for 
announcing the global menu?
  
  Could you please clarify what you mean by "plasma surface interface"?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: broulik, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-10 Thread Kai Uwe Broulik 
broulik added a comment.


  Didn't we have a dedicated protocol for window menu in plasma-integration? Or 
is this just for reading? We surely don't want all apps to use plasma surface 
interface for announcing the global menu?
  
  
https://cgit.kde.org/plasma-integration.git/tree/src/platformtheme/kwaylandintegration.cpp
  
  Or was that still using that custom Qt surface protocol that is being 
discontinued?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: broulik, davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-06 Thread Carson Black 
This revision was automatically updated to reflect the committed changes.
Closed by commit R127:dddf9d1ac3f2: Add application menu dbus paths to 
org_kde_plasma_window interface (authored by cblack).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76886=77147

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Carson Black 
cblack added a dependent revision: D27818: [wayland/AbstractClient] Broadcast 
application menu events.

REPOSITORY
  R127 KWayland

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Vlad Zahorodnii 
zzag accepted this revision.
zzag added a comment.
This revision is now accepted and ready to land.


  Thanks. Even though the patch is accepted, please do not land it yet. 
https://phabricator.kde.org/D27464#621521

REPOSITORY
  R127 KWayland

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Carson Black 
cblack updated this revision to Diff 76886.
cblack added a comment.


  Add missing documentation

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76885=76886

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Vlad Zahorodnii 
zzag added a comment.


  Looks good to me, but please don't land without corresponding KWin patch.

INLINE COMMENTS

> plasmawindowmanagement.h:700
> +/**
> + *  TODO: Documentation.
> + **/

You missed this one.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Carson Black 
cblack updated this revision to Diff 76885.
cblack added a comment.


  Address feedback

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76811=76885

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> plasmawindowmanagement.cpp:372
> +
> +Q_UNUSED(window);
> +}

Please move it to the top of the method and remove the semicolon.

> plasmawindowmanagement.h:526-527
>  
> +QString applicationMenuServiceName() const;
> +QString applicationMenuObjectPath() const;
> +

Please add documentation.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread Carson Black 
cblack added a comment.


  @zzag does anything else look off to you?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-03 Thread David Edmundson 
davidedmundson accepted this revision.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack updated this revision to Diff 76811.
cblack added a comment.


  Coding style

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76805=76811

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack marked an inline comment as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> plasmawindowmanagement_interface.cpp:85
>  void setGeometry(const QRect );
> +void setApplicationMenuPaths(const QString& service, const QString& 
> object);
>  wl_resource *resourceForParent(PlasmaWindowInterface *parent, 
> wl_resource *child) const;

You forgot to fix coding style here ;-)

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack marked 3 inline comments as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack updated this revision to Diff 76805.
cblack added a comment.


  Tick up PWM version

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76804=76805

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack marked 6 inline comments as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Carson Black 
cblack updated this revision to Diff 76804.
cblack added a comment.


  Address feedback

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=76035=76804

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Vlad Zahorodnii 
zzag requested changes to this revision.
zzag added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> plasmawindowmanagement.cpp:363
>  
> +void PlasmaWindow::Private::appmenuChangedCallback(void *data, 
> org_kde_plasma_window *window, const char* service_name, const char* 
> object_path)
> +{

Coding style: put whitespace before `*` and keep using snake_case.

  void PlasmaWindow::Private::appmenuChangedCallback(void *data, 
org_kde_plasma_window *window, const char *service_name, const char 
*object_path)

> zzag wrote in plasma-window-management.xml:312
> and this one should be
> 
>   
>   
>   

Also, please drop "_changed".

> plasmawindowmanagement_interface.h:234
> + */
> +void setApplicationMenuPaths(const QString& service_name, const QString& 
> object_path);
> +

Coding style: put whitespace before `&` and use camelCase.

  void setApplicationMenuPaths(const QString , const QString 
);

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-02 Thread Vlad Zahorodnii 
zzag added inline comments.

INLINE COMMENTS

> plasma-window-management.xml:20
>  
>
>  

version="10"

> plasma-window-management.xml:87
>  
> -  
> +  
>  

version="10"

> plasma-window-management.xml:312
>  
> +
> +  

and this one should be

  
  
  

> davidedmundson wrote in plasmawindowmanagement_interface.cpp:129
> how is this 9 already?

Urgh, it seems like versioning in the plasma window management protocol is 
messed up. For some reason, `org_kde_plasma_window_management` and 
`org_kde_plasma_window` have different versions.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-03-01 Thread Carson Black 
cblack added a comment.


  Would the version number already being 9 in old versions of KWayland cause 
any issues? I have a feeling old KWayland versions may not like it when they're 
sent events they don't recognize, unless libwayland/KWayland handle that 
usecase.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-24 Thread Carson Black 
cblack added inline comments.

INLINE COMMENTS

> davidedmundson wrote in plasmawindowmanagement_interface.cpp:129
> how is this 9 already?

Git blame shows that it's been 9 since 2018. Looks like someone made a mistake 
a while back?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-21 Thread David Edmundson 
davidedmundson added a comment.


  This is a nice simple example of something with a version bump: 
https://phabricator.kde.org/D27535 for where to change registry.
  
  I was wrong about display, there is a number you need to change, but 
apparently it's on v9 already, so you don't need to do anything.

INLINE COMMENTS

> plasmawindowmanagement_interface.cpp:129
>  
>  const quint32 PlasmaWindowManagementInterface::Private::s_version = 9;
>  

how is this 9 already?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-21 Thread Carson Black 
cblack added a comment.


  In D27464#614479 , @davidedmundson 
wrote:
  
  > Good start. The direction makes sense.
  >
  > display.cpp and registry.cpp need updating to the new version number. 
Otherwise we won't be able to use this new method.
  >
  > It'd be great if you could update the unit test too.
  
  
  Pardon, but I don't see anything in display/registry.cpp that looks like it 
takes a version number for the org_kde_plasma_window interface. I only see 
okp_windowmanagement in registry.cpp. Am I missing something?

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-19 Thread Carson Black 
cblack marked an inline comment as done.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-19 Thread Carson Black 
cblack updated this revision to Diff 76035.
cblack added a comment.


  Use utf8 encoding on client side

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=75855=76035

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-19 Thread David Edmundson 
davidedmundson added a comment.


  Good start. The direction makes sense.
  
  display.cpp and registry.cpp need updating to the new version number. 
Otherwise we won't be able to use this new method.
  
  It'd be great if you could update the unit test too.

INLINE COMMENTS

> plasmawindowmanagement_interface.cpp:599
> +}
> +org_kde_plasma_window_send_application_menu_changed(resource, 
> qUtf8Printable(service), qUtf8Printable(object));
> +}

We're mixing up our local 8bit and utf8 on different sides of this.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-17 Thread Carson Black 
cblack updated this revision to Diff 75855.
cblack added a comment.


  Figure out how to use arc correctly

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27464?vs=75854=75855

BRANCH
  cblack/appmenu-listener

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/protocols/plasma-window-management.xml
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h

To: cblack, #kwin, zzag
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-17 Thread Carson Black 
cblack planned changes to this revision.
cblack added a comment.


  whoops, I seem to have forgotten how to use arc

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27464

To: cblack, #kwin, zzag
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27464: Add application menu dbus paths to org_kde_plasma_window interface

2020-02-17 Thread Carson Black 
cblack created this revision.
cblack added reviewers: KWin, zzag.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
cblack requested review of this revision.

REVISION SUMMARY
  This patch adds an interface allowing a compositor to send
  the service name and object path of a PlasmaWindow's application menu
  to the client.

REPOSITORY
  R127 KWayland

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27464

AFFECTED FILES
  .gitignore
  src/CMakeLists.txt
  src/client/CMakeLists.txt
  src/org_kde_kwayland.categories
  src/server/CMakeLists.txt

To: cblack, #kwin, zzag
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns