Review Request 114130: A potential fix for Bug 327959

Milian Wolff mail at milianw.de
Thu Nov 28 11:23:35 UTC 2013


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


For 1.6 you can fixup the small issues I note below and then commit. For 1.7/master, I'd also like to see this function moved to language/duchain/duchainutils.h and be used in more places throughout the codebase.


plugins/contextbrowser/contextbrowser.cpp
<http://git.reviewboard.kde.org/r/114130/#comment31895>

    make static or move to anonymous namespace. 
    
    Also, return an IndexedDeclaration here.
    
    And open the function body on a new line.



plugins/contextbrowser/contextbrowser.cpp
<http://git.reviewboard.kde.org/r/114130/#comment31896>

    please also add braces to one-liner conditionals



plugins/contextbrowser/contextbrowser.cpp
<http://git.reviewboard.kde.org/r/114130/#comment31897>

    just this is enough:
    
    DUChainReadLocker lock;


- Milian Wolff


On Nov. 28, 2013, 9:37 a.m., Jon Mease wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114130/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2013, 9:37 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Bugs: 327959
>     http://bugs.kde.org/show_bug.cgi?id=327959
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Bug Summary: When the "Find uses" action is bound to a keystroke (Such as Ctrl+M), executing that keystroke has no effect even though executing "Find Uses" from the context menu works as expected.
> 
> Problem:  The ContextBrowserPlugin::findUses() function relies on the sender() object being a QAction whose data() is an IndexedDeclaration.  The setData() method for this QAction is only called in the ContextBrowserPlugin::contextMenuExtension() function, however this function is not called when the Find Uses action is invoked with a keyboard shortcut.
> 
> Potential Solution:  As a potential solution, this patch removes the calls to setData() and data() on the QAction.  Instead, I copied the cursorDeclaration() function that is used in quickopenplugin.cpp.  This function returns a Declaration for the item under the cursor, which I then wrap in an IndexedDeclaration and use in place of the prior result of data() on the QAction.
> 
> This solution fixes the bug, and preserves the past behavior of Find Uses from the context menu and the documentation popup.
> 
> Questions: Is there a better way to fix this?  Is there an appropriate place to put the cursorDeclaration() function so that it can be used by multiple plugins without copying it?
> 
> 
> Diffs
> -----
> 
>   plugins/contextbrowser/contextbrowser.cpp a2124f7 
> 
> Diff: http://git.reviewboard.kde.org/r/114130/diff/
> 
> 
> Testing
> -------
> 
> I bound the Find Uses action to Ctrl+M and tested it on a variety of C++ source files.  When the cursor is not on a declaration the shortcut does nothing and when the cursor is on a declaration the find uses window is displayed.  I also tested that executing 'Find Uses' from the context menu and 'Show Uses' from the documentation popup behave as before.
> 
> 
> Thanks,
> 
> Jon Mease
> 
>

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


More information about the KDevelop-devel mailing list