Review Request 121777: Make it possible to change language standard for the parser

Milian Wolff mail at milianw.de
Tue Feb 3 11:06:13 UTC 2015


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


some nitpicks from my side, I haven't tested it though - but I think it's an improvement


languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52077>

    maybe also store the supportedStandards in a temporary? simplifies reading a bit, I think.
    
    also, the other strings could/should be made QStringLiterals



languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52078>

    same as above



languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52079>

    QStringLiteral



languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52080>

    a comment might help simplify reading this code:
    
        // find compiler configured to a path closest to the requested item, or fallback to ...



languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52081>

    const



languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52082>

    style: put the { behind the `cast ...Role:` and the } in a new line before the `default:`:
    
        case Foo: {
            ...
        }
        default:
    
    or better yset - can't you just do a
    
        case Foo:
            return QVariant::fromValue(pathConfig.compiler);
        default:
            ...



languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52083>

    it is just *an* added path



languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp
<https://git.reviewboard.kde.org/r/121777/#comment52084>

    style: QVariant v = QVariant::fromValue(currentCompiler());


- Milian Wolff


On Jan. 24, 2015, 11:10 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121777/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2015, 11:10 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> Now we can choose different language standards for computing standard include directories/defined macros. 
> Also this feature is very useful for the kdev-clang plugin (Now it's possible to parse plain C projects).
> 
> 
> Diffs
> -----
> 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompilerfactory.h 3849bcb 
>   languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.h a59a6b7 
>   languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.cpp fcd9db7 
>   languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.h 5cb3ca0 
>   languages/plugins/custom-definesandincludes/compilerprovider/settingsmanager.cpp 229f456 
>   languages/plugins/custom-definesandincludes/compilerprovider/tests/CMakeLists.txt 8898c86 
>   languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.h dbd9a9c 
>   languages/plugins/custom-definesandincludes/compilerprovider/tests/test_compilerprovider.cpp 70b9919 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.cpp aebbf5a 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.ui 0c90cee 
>   languages/plugins/custom-definesandincludes/kcm_widget/definesandincludesconfigpage.cpp e1d6dda 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.h fe9124c 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.cpp 7d4347a 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.h 446ddb5 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp 5b34e75 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui 06a6c51 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.h 00042fb 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp 8d47690 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h 139b33c 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp 24d711d 
>   languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.h 079b78d 
>   languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp a9feab8 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompiler.h 3627ebd 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompiler.cpp 0bf6a44 
> 
> Diff: https://git.reviewboard.kde.org/r/121777/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150203/44940bbc/attachment.html>


More information about the KDevelop-devel mailing list