HAVE_X11 usage in KIO/core

[Aleix Pol]

On Fri, Feb 7, 2014 at 9:51 AM, Martin Gräßlin <mgraesslin at kde.org> wrote:

> On Friday 07 February 2014 09:38:41 Kevin Krammer wrote:
> > 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.
>
> See the comment I pasted, it's explicitly saying it's the windowing system
> and
> not the OS...
>
> Cheers
> Martin
> _______________________________________________
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel at kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
>
>
Sounds to me like we want to use QGuiApplication::platformName() there, the
only problem is that the strings differ.

Maybe we should just deprecate this function and advise users to use
::platformName().

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