Review Request 126241: [OS X] adapting KStyle (WIP)

[Hugo Pereira Da Costa]

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


I honestly have some doubt on the approach (but no counter proposal either)
Basically the file kstyle.mm is a *copy* of the .cpp file, with a couple of lines changed. (which I had to track down by 
- downloading the patch
- making an explicit diff of the two files
(which makes reviewing quite difficult/useless)

The mm file has the name, but different extension, and is compiled only for OSX
That means that any future fix (by me, or others) applied to one on a different platform will either
- not be ported to the other platform
- be ported blindly without testing (I don't even know how to compile mm on my linux box)
Is this really what we want to do ? 

Of course one could also add a common parent class to both the linux and osx implementation, that makes 99% of the job (some KStyleBase class), 
but then, wont that break binary compatibility badly for all widget styles that inherits these ? (oxygen, breeze)

Finally, the changes applied here only work for kstyle derived styles (oxygen, breeze), and not the QStyle based one. (QtCurve ?)

... not convinced. (but not strong objection either)

- Hugo Pereira Da Costa


On Dec. 4, 2015, 12:27 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126241/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2015, 12:27 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Hugo Pereira Da Costa.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> -------
> 
> This is a split-off from RR https://git.reviewboard.kde.org/r/126198/, as suggested there.
> 
> The proposed change (which is a work in progress) contains a few modifications mirroring those proposed for the KdePlatformTheme plugin, aiming to adapt the library to Mac OS X.
> 
> These modifications should probably be implemented by subclassing KStyle rather than duplicating all code.
> 
> I have been focussing on the platform theme modifications, without really looking into the extent to which KStyle is used or potentially useful on OS X. A separate RR should support discussion about that more easily.
> 
> Would it for instance be possible to use KStyle to create a Qt *style* plugin that does nothing more than extending the native theme/style with support for KDE's font roles/types, colour palettes and icon themes? This could be preferable for users or developers who are not interested in providing a consistent cross-platform look (which presumable requires a platform *theme*) and/or who do not want to depend on a theme that makes explicit use of a private Qt API (cf. `KdeMacTheme` in the RR above).
> 
> 
> Diffs
> -----
> 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126241/diff/
> 
> 
> Testing
> -------
> 
> Builds on OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 ; "source" modifications are tested in the platform theme.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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