Review Request 120241: Add include path completion

[Milian Wolff]

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


Nice, very clean code and the comments below are again mostly nitpicks. Thanks Sergey!

What's missing though are unit tests. Pretty please add some for that stuff.


codecompletion/CMakeLists.txt
<https://git.reviewboard.kde.org/r/120241/#comment46510>

    this looks wrong. link against duchain where necessary (but not here).



codecompletion/includepathcompletioncontext.h
<https://git.reviewboard.kde.org/r/120241/#comment46511>

    indent is off and dtor is missing



codecompletion/includepathcompletioncontext.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46513>

    '\n'



codecompletion/includepathcompletioncontext.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46514>

    QStringLiteral



codecompletion/includepathcompletioncontext.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46515>

    instead of this (which requires a temporary allocation), check whether slashIdx <= 1 and add a comment. should be the same, no? instead of .left below then use .mid
    
    otoh, this is only called rarely, so maybe just ignore this optimization. probably not required.



codecompletion/includepathcompletioncontext.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46516>

    I prefer this: `Path(url).parent()`



codecompletion/model.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46517>

    rename to `shouldDoIncludePathCompletion` or similar



codecompletion/model.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46512>

    indent



duchain/clangfilehelpers.cpp
<https://git.reviewboard.kde.org/r/120241/#comment46518>

    missing copyright header


- Milian Wolff


On Sept. 17, 2014, 9:28 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120241/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 9:28 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> While the general idea borrowed from oldcpp, the code is mostly rewritten anew.
> 
> 
> Diffs
> -----
> 
>   codecompletion/CMakeLists.txt 6d6f9a8 
>   codecompletion/includepathcompletioncontext.h PRE-CREATION 
>   codecompletion/includepathcompletioncontext.cpp PRE-CREATION 
>   codecompletion/model.cpp 114f885 
>   duchain/clangfilehelpers.cpp PRE-CREATION 
>   duchain/clanghelpers.cpp 93f4298 
>   duchain/navigationwidget.h 8283bb6 
>   duchain/navigationwidget.cpp 97010c1 
> 
> Diff: https://git.reviewboard.kde.org/r/120241/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

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