[Patch] Qmake project manager Qt module include paths

[Andreas Pakulat]

On 09.04.09 17:26:18, Sean Harmer wrote:
> Hi,
> 
> On Thursday 09 April 2009 14:22:26 Sean Harmer wrote:
> > Could somebody please review the patch please?
> I have improved the patch a little more so that it now also resolves 
> environment variables in the INCLUDEPATH qmake variable. Updated patch 
> attached for review.

Sorry, but not ok. Envvars need to be read via project config and the
envvar-stuff from kdevplatform. That way we get the standard env in
which kdevelop was started, but overriden by any user-chosen
env configuration via the project config.

> +    // Let's cache the Qt include dir
> +    QProcess qtInc;
> +    qtInc.start( "qmake", QStringList() << "-query" << "QT_INSTALL_HEADERS" );
> +    if ( !qtInc.waitForFinished() )
> +    {
> +        kDebug(9024) << "Failed to query Qt header path using qmake";
> +    } else
> +    {
> +        QByteArray result = qtInc.readAll();
> +        m_qtIncludeDir = result.simplified();
> +    }
> +

This part is ok design-wise, but the conversion from QByteArray to
QString is broken - I suggest to make sure that qmake runs in an UTF8
locale and then use QString::fromUtf8().

> @@ -152,11 +180,57 @@
>      kDebug(9024) << variableValues("QMAKE_INCDIR_QT");
>      if( variableValues("CONFIG").contains("qt") )
>      {
> -        //@TODO add QtCore,QtGui and so on depending on CONFIG values,
> -        //      as QMAKE_INCDIR_QT only contains the include/ dir
> -        foreach( const QString& val, variableValues("QMAKE_INCDIR_QT") )
> +        KUrl url(m_qtIncludeDir);
> +        if( !list.contains( url ) )
> +            list << url;
> +
> +        kDebug(9024) << "QT =" << variableValues("QT");
> +        foreach( const QString& module, variableValues("QT") )
>          {
> -            KUrl url(val);
> +            KUrl url;
> +            if ( module.compare( "core" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtCore");
> +            else if ( module.compare( "gui" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtGui");
> +            else if ( module.compare( "network" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtNetwork");
> +            else if ( module.compare( "opengl" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtOpenGL");
> +            else if ( module.compare( "phonon" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/Phonon");
> +            else if ( module.compare( "script" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtScript");
> +            else if ( module.compare( "scripttools" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtScriptTools");
> +            else if ( module.compare( "sql" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtSql");
> +            else if ( module.compare( "svg" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtSvg");
> +            else if ( module.compare( "webkit" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtWebKit");
> +            else if ( module.compare( "xml" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtXml");
> +            else if ( module.compare( "xmlpatterns" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtXmlPatterns");
> +            else if ( module.compare( "qt3support" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/Qt3Support");
> +            else if ( module.compare( "designer" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtDesigner");
> +            else if ( module.compare( "uitools" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtUiTools");
> +            else if ( module.compare( "help" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtHelp");
> +            else if ( module.compare( "assistant" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtAssistant");
> +            else if ( module.compare( "qtestlib" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtTest");
> +            else if ( module.compare( "qaxcontainer" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/ActiveQt");
> +            else if ( module.compare( "qaxserver" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/ActiveQt");
> +            else if ( module.compare( "dbus" ) == 0 )
> +                url.setUrl(m_qtIncludeDir + "/QtDBus");

I like if( module == "XXX" )  more than the compare() version (looks so
old-style c-like ;), so I'd appreciate it if you could change that.
Other than that, this part looks fine.

Oh and please check for extra debug output, that shouldn't be comitted.

Andreas

-- 
Increased knowledge will help you now.  Have mate's phone bugged.