> On Dec. 18, 2015, 8:11 a.m., Martin Gräßlin wrote:
> > src/platformtheme/kdeplatformtheme.cpp, lines 45-46
> > <https://git.reviewboard.kde.org/r/126198/diff/10/?file=424265#file424265line45>
> >
> >     Q_NULLPTR
> 
> René J.V. Bertin wrote:
>     I recall a relatively recent discussion on a Qt mailing list; IIUC Qt now 
> advises to use `0` and not Q_NULLPTR anymore. It certainly makes the code 
> more compact and (thus) readable; is there a reason KF5 sticks to Q_NULLPTR?
> 
> Aleix Pol Gonzalez wrote:
>     I don't see where you got that impression:
>     
> http://lists.qt-project.org/pipermail/development/2015-December/024040.html

Is that your idea of *relatively* recent? :) I was thinking of a discussion a 
bit longer ago (but frankly, don't ask me to dig it up, I doubt I'd be able to).

I haven't read the full thread above, in fact I stopped reading at 
```
Arguments against:
- it's uglier than "0", and more to type
```

because that sums up exactly how I think about the question. But in the end I 
only care that there are no chances of 32bit/64bit conflicts (I had some of 
those in code of my own, which led me to be almost religious about using NULL 
for pointers).

I do have a bit of an issue with using a *Q_* prefix, as it suggests a 
Qt-specific type that should only be used in calls to Qt functions...


- René J.V.


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


On Dec. 17, 2015, 7:40 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 7:40 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> -------
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -----
> 
>   src/platformtheme/kdemactheme.h PRE-CREATION 
>   src/platformtheme/kdemactheme.mm PRE-CREATION 
>   src/platformtheme/CMakeLists.txt 23f590e 
>   src/platformtheme/kdeplatformtheme.h 97d09df 
>   src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
>   src/platformtheme/kfontsettingsdata.h 4b92c7d 
>   src/platformtheme/kfontsettingsdata.cpp b0a4bbf 
>   src/platformtheme/kfontsettingsdatamac.h PRE-CREATION 
>   src/platformtheme/kfontsettingsdatamac.mm PRE-CREATION 
>   src/platformtheme/khintssettings.h ec064d3 
>   src/platformtheme/khintssettings.cpp 8adf6c5 
>   src/platformtheme/khintssettingsmac.h PRE-CREATION 
>   src/platformtheme/khintssettingsmac.mm PRE-CREATION 
>   src/platformtheme/main_mac.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
> 
> 
> Testing
> -------
> 
> On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.
> 
> I have not verified to what extent my use of a private `QGuiApplication` API 
> links builds to a specific Qt version (I consider that nothing shocking and a 
> minor price to pay).
> >>> Do I need to add some glue to the CMake file so that it will warn if the 
> >>> private headerfiles are not available? Apparently no changes were 
> >>> required to find them.
> 
> 
> File Attachments
> ----------------
> 
> purely native OS X theme
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/650d0da7-52b3-40d1-a1f9-cb610494cf77__Screen_Shot_2015-11-30_at_15.42.31.png
> native theme but with `-style kde`
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/72e2a6aa-2a7c-465b-b404-fc1e52b6fc69__Screen_Shot_2015-11-30_at_15.43.02.png
> using the KDEPlatformTheme
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/309e5995-74fa-42fb-a6f3-936cedbf5246__Screen_Shot_2015-11-30_at_15.43.31.png
> on Linux, using a purely "native" theme
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/eaa1d907-bf05-4ca2-821b-83dc062aea04__QtCreatorNativeLNX.png
> KDEPlatformTheme with the "macintosh" native theme selected
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/01/de55a91f-3500-4db8-8a3b-d252fd7ea169__Screen_Shot_2015-12-01_at_13.52.35.png
> kate built by Christoph Cullman as a completely standalone app bundle
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/12/d5f9a983-a077-4a52-913e-5d980e62daca__kate-Cullmans-appbundle.png
> kate/MacPorts in fully native Mac mode (but using XDG-compliant QSPs).
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/12/02dbfc58-445c-48f5-88ad-33b5ab55571a__kate-MP-fullnative.png
> kate/MacPorts using the QtCurve style (but the native platform plugin)
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/12/c9d1e28e-6684-4ae3-b05d-1fbc6cde39f4__kate-MP-QtCurveStyle.png
> kate/MacPorts using the KdeMacTheme plugin and the macintosh style
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/12/76b1305e-0bf2-4073-a61f-4df59435a198__Kate-MP-plplug-MacStyle.png
> kate/MacPorts using the KdeMacTheme plugin and the QtCurve style
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/12/ad54fdf3-a450-4d0b-8a60-8962f537ed6f__kate-MP-plplug-QtCurve.png
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to