Review Request 124245: Introduced RefactoringManager

Milian Wolff mail at milianw.de
Sun Jul 12 21:17:48 UTC 2015


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

Ship it!


some nitpicks, otherwise looks good


refactoring/documentcache.cpp (line 81)
<https://git.reviewboard.kde.org/r/124245/#comment56815>

    join line



refactoring/refactoring.cpp (line 24)
<https://git.reviewboard.kde.org/r/124245/#comment56816>

    put member-initialization into its own line, indented by four spaces



refactoring/renamevardeclrefactoring.cpp (line 50)
<https://git.reviewboard.kde.org/r/124245/#comment56817>

    I'd prefer you put the commas below the :, such that you don't need to add/remove a comma to a line when you add/remove a new member



refactoring/renamevardeclrefactoring.cpp (line 77)
<https://git.reviewboard.kde.org/r/124245/#comment56818>

    see above



refactoring/utils.cpp (line 247)
<https://git.reviewboard.kde.org/r/124245/#comment56819>

    here and below: the SourceLocation/Range shoudl be taken by const&, I guess.


- Milian Wolff


On July 10, 2015, 5:26 p.m., Maciej Poleski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124245/
> -----------------------------------------------------------
> 
> (Updated July 10, 2015, 5:26 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Introduced RefactoringManager.
> 
> Class used to prepare list of applicable refactorings "here" and their initialization. This replaces unfinished functionality of `Refactorings::allApplicableRefactorings`.
> 
> This class is coupled to all refactorings implementations. It works by traversing AST looking for node at given localization. Then it makes inspections to decide which refactorings can be used "here". When some refactoring is available - instantiates appropriate classes (derived from `Refactoring`) providing it with additional informations as required.
> 
> List of created instances is used to populate context menu. Action on menu item is to invoke refactoring. All instances are (will be) children of context menu (appropriate `QAction` to be precise).
> 
> This functionality is required for almost any nontrivial refactoring.
> 
> I have problem with multi-pass tooling. I cannot preserve AST nodes between passes. As a result I'm unable to make processing as efficient as it could be. I will work more on it.
> 
> 
> Diffs
> -----
> 
>   refactoring/CMakeLists.txt PRE-CREATION 
>   refactoring/documentcache.h PRE-CREATION 
>   refactoring/documentcache.cpp PRE-CREATION 
>   refactoring/interface.h PRE-CREATION 
>   refactoring/interface.cpp PRE-CREATION 
>   refactoring/kdevrefactorings.h PRE-CREATION 
>   refactoring/kdevrefactorings.cpp PRE-CREATION 
>   refactoring/nooprefactoring.h PRE-CREATION 
>   refactoring/nooprefactoring.cpp PRE-CREATION 
>   refactoring/refactoring.h PRE-CREATION 
>   refactoring/refactoring.cpp PRE-CREATION 
>   refactoring/refactoringcontext.h PRE-CREATION 
>   refactoring/refactoringcontext.cpp PRE-CREATION 
>   refactoring/refactoringinfo.h PRE-CREATION 
>   refactoring/refactoringmanager.h PRE-CREATION 
>   refactoring/refactoringmanager.cpp PRE-CREATION 
>   refactoring/renamevardeclrefactoring.h PRE-CREATION 
>   refactoring/renamevardeclrefactoring.cpp PRE-CREATION 
>   refactoring/utils.h PRE-CREATION 
>   refactoring/utils.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124245/diff/
> 
> 
> Testing
> -------
> 
> Manual testing
> 
> 
> Thanks,
> 
> Maciej Poleski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150712/8cdc98d1/attachment.html>


More information about the KDevelop-devel mailing list