HAVE_X11 usage in KIO/core

[Kevin Krammer]

On Friday, 2014-02-07, 08:53:54, Martin Gräßlin wrote:
> Hi,
> 
> I found some HAVE_X11 not defined warnings in KIO and had a look at them.
> One of them is in core/kprotocolmanager.cpp in the following snippet.
> 
> // This is not the OS, but the windowing system, e.g. X11 on Unix/Linux.
> static QString platform()
> {
> #if HAVE_X11
>     return QL1S("X11");
> #elif defined(Q_OS_MAC)
>     return QL1S("Macintosh");
> #elif defined(Q_OS_WIN)
>     return QL1S("Windows");
> #else
>     return QL1S("Unknown");
> #endif
> }
> 
> I'm wondering what to do about it. The best would be to use
> QGuiApplication::platformName, but it's a core app. Also finding X11 in
> CMakeLists to get the HAVE_X11 defined looks very wrong to me and not future
> safe (Wayland).

My guess is that platform() in this context means operating system, not 
windowing/display system.
Hinted also by the use of Q_OS_ instead of Q_WS_

IMHO the correct change is something like this

#if defined(Q_OS_UNIX)
    #if  defined(Q_OS_MAC)
        return QL1S("Macintosh")
    #elif defined(Q_OS_LINUX)
        return QL1S("Linux")
    #else
        return QL1S("Unix")
    #endif
#elfi defined(Q_OS_WINDOWS)
      return QL1S("Windows")
#else
      return QL1S("Unknown")
#endif

Cheers,
Kevin
-- 
Kevin Krammer, KDE developer, xdg-utils developer
KDE user support, developer mentoring
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140207/f7a43fd8/attachment.sig>