Review Request 118542: Move assistant infrastructure to kdevplatform

[Milian Wolff]

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


looks cool to me, some style issues and of course the missing pimpl stuff.

did you try it out with non-c++ language plugins like PHP with its $foo-like variables?


language/assistant/renameaction.h
<https://git.reviewboard.kde.org/r/118542/#comment41286>

    nitpick: * and & go next to type in kdev land



language/assistant/renameaction.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41288>

    auto, also below



language/assistant/renameaction.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41289>

    spaces



language/assistant/renameaction.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41290>

    indent level is off



language/assistant/renameaction.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41291>

    braces



language/assistant/renameassistant.h
<https://git.reviewboard.kde.org/r/118542/#comment41292>

    what for?



language/assistant/renameassistant.h
<https://git.reviewboard.kde.org/r/118542/#comment41293>

    indent



language/assistant/renameassistant.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41294>

    spaces



language/assistant/renameassistant.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41295>

    return !firstRange.instersect(secondRange + Range(0, -1, 0, +1)).isEmpty());
    
    achieves the same, no?



language/assistant/renameassistant.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41296>

    auto cursor = changed.start();



language/assistant/renameassistant.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41297>

    here and below: braces



language/assistant/staticassistantsmanager.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41304>

    yay in principle.
    
    but: should this be a list or something? could an assistant not support multiple languages or should we keep it as-is and let multiple assistants be registered for every language?
    
    not opening an issue, just want to know your input on that



language/assistant/staticassistantsmanager.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41305>

    here and below: braces



language/assistant/staticassistantsmanager.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41298>

    here and below: space after if



language/assistant/staticassistantsmanager.cpp
<https://git.reviewboard.kde.org/r/118542/#comment41299>

    braces


- Milian Wolff


On June 4, 2014, 8:47 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118542/
> -----------------------------------------------------------
> 
> (Updated June 4, 2014, 8:47 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Move assistant infrastructure to kdevplatform
> 
> Noteworthy changes:
> * StaticCodeAssistant (from CPP plugin) is now StaticAssistantsManager,
>   now lives in  kdevplatform, so all languages can benefit from it
> * RenameAssistant was moved to kdevplatform, as it it useful for all
>   languages
> * BasicRefactoring now has new virtual methods that control the behavior
>   for renaming actions
>   LanguageSupport got a new property: refactoring (so languages can expose
>   their custom BaseRefactoring implementation)
> 
> New API:
> * New base class for "static" assistants: StaticAssistant
>   Static assistants exist during the whole session
>   They are notified when documents change and create solutions based on
>   that. Current implementations: AdaptDefinitionSignatureAssistant (CPP
>   language) and RenameAssistant (kdevplatform)
> * New manager class: StaticAssistantsManager:
>   Tracks the StaticAssistant instances, takes care of notifying them
>   about changes in the editors. Single entry point for registering
>   StaticAssistants.
>   Entry-point: LanguageController::staticAssistantsManager
> 
> 
> Diffs
> -----
> 
>   interfaces/ilanguagecontroller.h 57f8651f8e46685c7155db28b9c4277713635a88 
>   language/CMakeLists.txt 0c3af97f733b2c088798f3ee3fb9f6499c968da2 
>   language/assistant/renameaction.h PRE-CREATION 
>   language/assistant/renameaction.cpp PRE-CREATION 
>   language/assistant/renameassistant.h PRE-CREATION 
>   language/assistant/renameassistant.cpp PRE-CREATION 
>   language/assistant/renamefileaction.h PRE-CREATION 
>   language/assistant/renamefileaction.cpp PRE-CREATION 
>   language/assistant/staticassistant.h PRE-CREATION 
>   language/assistant/staticassistant.cpp PRE-CREATION 
>   language/assistant/staticassistantsmanager.h PRE-CREATION 
>   language/assistant/staticassistantsmanager.cpp PRE-CREATION 
>   language/codegen/basicrefactoring.h 8c7f782bdc1f6af9d7d45c7cb7e30aba85f9547e 
>   language/codegen/basicrefactoring.cpp 2b537f329dfacdba1a2890c9cfddd72b2ac60ef8 
>   language/interfaces/ilanguagesupport.h 5c8747b0cf1aee78ed84c8f07989a09e1bd6cb10 
>   language/interfaces/ilanguagesupport.cpp 5797511f19f39ebf1fbcf7662549e698b330bdf6 
>   shell/languagecontroller.h a4c192603a62dcbb00539004a46cc674df26b998 
>   shell/languagecontroller.cpp 308ec20a6e7f41a271069993010fc46ff696ef56 
> 
> Diff: https://git.reviewboard.kde.org/r/118542/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

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