Review Request 116610: Use flag types in NETRootInfo

[Aurélien Gâteau]

> On March 6, 2014, 5:22 p.m., Aurélien Gâteau wrote:
> > Looks good.
> > 
> > One question though, wasn't it possible to merge properties or properties2? I lack extensive knowledge of this domain, but it looks odd from an API point of view.
> 
> Martin Gräßlin wrote:
>     that would be a large SC break which might not be worth the effort even if it is possible.
>     
>     The reason why it is split is AFAIU that NET::Properties consists of 32 flags, thus hitting the bounds of the underlying int back in the days. Given that QFlags still uses either int or unsigned int I guess this border is still valid.

Sounds reasonable to stay this way then.


- Aurélien


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


On March 5, 2014, 10:50 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116610/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 10:50 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> Use flag types in NETRootInfo
> 
> This replaces the usage of the unsigned long[] array with
> NET::Properties, NET::Properties2, NET::WindowTypes, NET::States and
> NET::Actions mixed in - for the fun even in different order
> depending on the ctor to use.
> 
> This includes the following changes:
> * NETRootInfo::event(xcb_generic_event_t*) returns NET::Properties
> * new overload added to NETRootInfo::event taking NET::Properties*
>   and NET::Properties2* as arguments
> * existing overload for NETRootInfo::event taking usinged long* as
>   argument is deprecated and forwards to the new added overload
> * NETRootInfo::passedProperties returns NET::Properties and new
>   methods for Properties2, WindowTypes, States and Actions are added.
>   This is a SC-break, but there is according to lxr no usage except
>   in the unit tests
> * NETRootInfo::supportedProperties returns NET::Properties and new
>   methods for Properties2, WindowTypes, States and Actions are added.
>   This is a SC-break, but there is according to lxr no usage except
>   in the unit tests
> * ctors taking unsigned long* is changed to taking the flags as
>   arguments. This in an SC-break, but the ctor broke already caused
>   by the change from Display* to xcb_connection_t*. The ctor for
>   window manager should only be used by KWin. Other ctor is also
>   mainly only used in kde-workspace.
> 
> 
> Diffs
> -----
> 
>   src/netwm_p.h 1c26d47257a4f3ce71c360c394cc5be16f32b18c 
>   src/netwm.h a5ad46fa1f0255ca6719224079bdea4598b48c51 
>   src/netwm.cpp 288a34348f464e1158640afa1d931d9d05690cc4 
>   src/kwindowsystem_x11.cpp 95c396b65ae0a24db6a276b9b72f4175bb7c14cc 
>   autotests/netrootinfotestwm.cpp 120fbee92d0b22862d8ce746b3b30891ecd9f056 
> 
> Diff: https://git.reviewboard.kde.org/r/116610/diff/
> 
> 
> Testing
> -------
> 
> tests succeed with 116609 applied. KWin and kde-workspace will get adjusted to test.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140310/1049bb23/attachment-0001.html>