Review Request 119648: Make "Switch Declaration/Definition" language agnostic

[Milian Wolff]

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

Ship it!


I know, most is not your code, still I think this could be cleaned up further.

Generally a good thing to have it in this plugin, I think. I wonder though how we could enable/disable this action, based on the language of the current active file. We don't want this action to be enabled when we edit a .php/.js/.py file e.g. Maybe we could check whether there are buddies available for the current document and if not, disable the action?


plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44679>

    if (!decl) {
            continue;
        }
        if (auto definition = FunctionDefinition::definition(decl)) {
            return definition;
        }
    
    I'd prefer this. And it makes sure we only call ::definition once.



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44680>

    put the below into a helper function so that you can do
    
    const QString switchCandidate = ...();



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44681>

    this is probably from the old code, but it makes no sense, imo:
    
    we parse it with VisibleDeclarationsAndContexts (maybe a bit quicker), then switch to the editor, there we need uses, which would then trigger another reparse. I'd trigger a reparse with users directly here.



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44685>

    remove the DUChain::lock



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44682>

    const auto url = def->url().toUrl();



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44683>

    he document check here at the beginning is obsolete (after all, you just checked !document || before.



plugins/switchtobuddy/switchtobuddyplugin.cpp
<https://git.reviewboard.kde.org/r/119648/#comment44684>

    I'd prefer
    
    const auto pos = normalizeCursor(targetRange.start());


- Milian Wolff


On Aug. 7, 2014, 2:42 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119648/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 2:42 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Make "Switch Declaration/Definition" language agnostic
> 
> Most of the code originated from oldcpp. It is completely language
> agnostic and hence can be easily reused.
> 
> 
> Diffs
> -----
> 
>   plugins/switchtobuddy/kdevswitchtobuddy.rc PRE-CREATION 
>   plugins/switchtobuddy/switchtobuddyplugin.h e5c183754ae057d8717c90c702fe6e23f1598b40 
>   plugins/switchtobuddy/switchtobuddyplugin.cpp 93bcd09b908b9af46e31490c0d05e24a79de5bca 
> 
> Diff: https://git.reviewboard.kde.org/r/119648/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

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