Review Request 120213: Add support for protocols to NETWinInfo

[Thomas Lübking]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120213/#review66605
-----------------------------------------------------------

Ship it!


Minor notes only.

However, I assume you also want to add protocol invocation (like exists for pinging the window) and therefore a preemtive warning:
I'll *strongly* veto exposing the sync protocol to random clients (since we know the effects of a double sync ;-)


src/netwm.cpp
<https://git.reviewboard.kde.org/r/120213/#comment46415>

    "p->protocols = NoProtocol;"?
    (ie. have an enum for the empty case and also "why operate on a temp variable?")



src/netwm.cpp
<https://git.reviewboard.kde.org/r/120213/#comment46417>

    "switch(*it) { ..." might be more readable, but i've no preference ;-)



src/netwm.cpp
<https://git.reviewboard.kde.org/r/120213/#comment46416>

    Tarzan speech. Consider:
    - TakeFocusProtocol
    - Protocol::TakeFocus



src/netwm_def.h
<https://git.reviewboard.kde.org/r/120213/#comment46414>

    "by the client"?


- Thomas Lübking


On Sept. 15, 2014, 7:29 vorm., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120213/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 7:29 vorm.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> NETWinInfo is now able to read all known protocols set on WM_PROTOCOLS
> property and provides the set protocols as flags.
> 
> So far the following protocols are supported:
> * WM_TAKE_FOCUS (ICCCM 4.1.2.7)
> * WM_DELETE_WINDOW (ICCCM 4.1.2.7)
> * _NET_WM_SYNC_REQUEST (EWMH)
> * _NET_WM_PING (EWMH)
> * _NET_WM_CONTEXT_HELP (KDE specific extension to EWMH)
> 
> CHANGELOG: NETWinInfo provides convenient wrapper for WM_PROTOCOLS.
> 
> 
> Diffs
> -----
> 
>   autotests/netwininfotestclient.cpp e35ab8d83b1e0c1af42dacc7f330cbc3b9b899aa 
>   src/netwm.h 436ad2fd449b48a36d3bb15754f7c8c64f52c8dd 
>   src/netwm.cpp 1431cf6bae605050e2e86f7ca60357ddb7736d32 
>   src/netwm_def.h 12abfff03af5b81a60ec03a5366b6ff0b95d35d0 
>   src/netwm_p.h a586450211846af464c9014f586e14b0e808cd75 
> 
> Diff: https://git.reviewboard.kde.org/r/120213/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140915/c0081749/attachment.html>