Review Request: Make sig assistant rename

[Milian Wolff]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103817/#review10815
-----------------------------------------------------------


could you please elaborate on the "assistant stacking"?

and this code really could need some love, style-wise. Please, I'd really appreciate if you could clean the code up while you are at it. Also some more comments would certainly help! I don't understand e.g. what the three new functions in IAssistant are supposed to do... And please please please, don't inline all code, separate it cleanly between header (with apidox please?) and implementation...


languages/cpp/codegen/renameaction.h
<http://git.reviewboard.kde.org/r/103817/#comment8762>

    please move the method-implementations into a .cpp file, and add a "using namespace KDevelop;" there to shorten the code below



languages/cpp/codegen/renameaction.h
<http://git.reviewboard.kde.org/r/103817/#comment8765>

    const&



languages/cpp/codegen/renameaction.h
<http://git.reviewboard.kde.org/r/103817/#comment8763>

    new code shouldn't pass anything to the lock's ctor. i.e.:
    
    DUChainReadLocker lock;



languages/cpp/codegen/renameaction.h
<http://git.reviewboard.kde.org/r/103817/#comment8764>

    const&



languages/cpp/codegen/signatureassistant.cpp
<http://git.reviewboard.kde.org/r/103817/#comment8766>

    could it happen that this is never called? yes, or - if the assistant is not applied? then we'd leak the rename actions.
    
    so add a qDeleteAll to the ctor of IAssistantAction


- Milian Wolff


On Feb. 17, 2012, 10:50 a.m., Olivier Jean de Gaalon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103817/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2012, 10:50 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> After editing function declaration, when updating the definition to match, rename uses of renamed declarations.
> 
> Note:
> Removed the assistant stacking since the sig assist depends on being destroyed (and it was hacking where change is needed). Shouldn't be much work to fix that, but time I don't have now...
> 
> 
> This addresses bug 255706.
>     http://bugs.kde.org/show_bug.cgi?id=255706
> 
> 
> Diffs
> -----
> 
>   languages/cpp/codegen/renameaction.h PRE-CREATION 
>   languages/cpp/codegen/renameassistant.cpp 9bbdce7 
>   languages/cpp/codegen/signatureassistant.h 2c6853d 
>   languages/cpp/codegen/signatureassistant.cpp 0231e90 
> 
> Diff: http://git.reviewboard.kde.org/r/103817/diff/
> 
> 
> Testing
> -------
> 
> Using the signature assistant.
> 
> 
> Thanks,
> 
> Olivier Jean de Gaalon
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120221/5944feb8/attachment.html>