Review Request 117098: Add #include-suggestion to kdev-clang

[Kevin Funk]

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



duchain/missinginclude.h
<https://git.reviewboard.kde.org/r/117098/#comment37955>

    Why is that not just returning void?
    And use const& for the parameters



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37957>

    "= QString()"



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37948>

    Call that 'isBlacklistedIncludeDir', like before?



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37946>

    What is 'acc' why is it not const&?
    
    Please document the function



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37947>

    Style: No spaces in QVector<HeaderPath> (there are more similar issues below)



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37949>

    Style: Trim spaces (more issues below)



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37950>

    Style: Make that:
    if ( ... ) {
        ...
    }
    
    (more issues below, please fix them, too)



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37951>

    Can't you move these two lines to the for-condition?
    
    => "for(auto x = a.begin(), y = b.begin(); x != a.end() && y != b.end(); ++x, ++y ) {"



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37952>

    Remove empty line



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37953>

    Style: Again, insert braces and newline



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37954>

    Style: Make that:
    UnknownDeclarationFixit{ "class " + identifier + ";", range };



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37958>

    Style: Always use braces around for-loops.



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37959>

    Style: 
    UnknownDeclarationProblem::UnknownDeclarationProblem( KDevelop::Identifier id, KDevelop::Path path )
        : m_identifer( id )
        , m_file( path )



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37960>

    That line is pretty much obvious, you don't need to state that.



duchain/missinginclude.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37961>

    Style: Always indent with 4 spaces.



duchain/parsesession.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37964>

    Rather use QList::value() here, for safety.



duchain/parsesession.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37962>

    Can't you just re-use the original message? This looks likes unnecessary code + something that would need to be adapted quite often
    
    Also see 'prettyDiagnosticSpelling'



duchain/parsesession.cpp
<https://git.reviewboard.kde.org/r/117098/#comment37965>

    Check if symbol is non-empty


- Kevin Funk


On March 26, 2014, 7:48 p.m., Jørgen Kvalsvik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117098/
> -----------------------------------------------------------
> 
> (Updated March 26, 2014, 7:48 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Add #include-suggestion features to kdev-clang
> 
> 
> Diffs
> -----
> 
>   duchain/CMakeLists.txt d21011a 
>   duchain/missinginclude.h PRE-CREATION 
>   duchain/missinginclude.cpp PRE-CREATION 
>   duchain/parsesession.cpp eee9fb1 
> 
> Diff: https://git.reviewboard.kde.org/r/117098/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jørgen Kvalsvik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140327/f169615f/attachment-0001.html>