Review Request 115225: Add runtime platform support to KWindowInfo

[Martin Gräßlin]

> On Jan. 22, 2014, 4:32 p.m., David Edmundson wrote:
> > src/kwindowinfo.cpp, line 191
> > <https://git.reviewboard.kde.org/r/115225/diff/2/?file=235203#file235203line191>
> >
> >     This seems wrong.
> >     
> >     KWindowInfo cheese;
> >     cheese.state(); //CRASH
> >

I kept the ctor as it's documented as "makes QList happy". I don't really get why it is there and it looks wrong to me and I think it's a stupid idea to put KWindowInfo into a QList at all.

If other people agree that we can do an API break (hello Kevin or David F.) I'll happily remove the ctor and document the API break.


- Martin


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


On Jan. 22, 2014, 3:03 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115225/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 3:03 p.m.)
> 
> 
> Review request for KDE Frameworks and kdewin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> Add runtime platform support to KWindowInfo
> 
> Main idea of this change is to only pick the X11 implementation in case
> that the application is running on the X11 platform. So far it was a
> compile time switch which meant that if compiled with X11 support but
> not running on the X11 platform it would have caused runtime errors.
> 
> To make this possible a KWindowInfoPrivate class with a dummy
> implementation is provided. This is used as d-ptr for KWindowInfo.
> Thus there is one generic implementation and the implementation of
> KWindowInfo is no longer ifdefed for the supported platforms.
> 
> The platform specific code can inherit from KWindowInfoPrivate and
> overwrite the dummy method implementation. KWindowInfoPrivate provides
> a factory method where the platform specific implementation can be
> hooked into. There we can have both compile time and runtime checking.
> If there is no platfom specific implementation available the dummy
> implementation is used.
> 
> NOTE: THIS CHANGE BREAKS THE WINDOWS AND MAC BACKEND!
> 
> Windows and Mac is excluded from build. At the moment they get the
> dummy implementation. Unfortunately I don't have the possibility to
> compile the changes and thus don't dare to touch the code. Fixes from
> the teams are highly appreciated.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt e32a1150a2c190f23ad456ca8218b012c5d71507 
>   src/kwindowinfo.h 171f441ff329a5356ccf560341272199e20c837a 
>   src/kwindowinfo.cpp PRE-CREATION 
>   src/kwindowinfo_p.h PRE-CREATION 
>   src/kwindowinfo_p_x11.h PRE-CREATION 
>   src/kwindowinfo_x11.cpp 865d8bed085e987f97f479ea8aa0e6de8567586f 
> 
> Diff: https://git.reviewboard.kde.org/r/115225/diff/
> 
> 
> Testing
> -------
> 
> Unit test from https://git.reviewboard.kde.org/r/115190/ is still working. Now you can guess why I wrote that test ;-)
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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