----------------------------------------------------------- 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel