Review Request 124934: Fix declaration tooltip flickering

[Maciej Cencora]

> On Aug. 26, 2015, 8:29 p.m., Milian Wolff wrote:
> > language/duchain/duchainutils.cpp, line 343
> > <https://git.reviewboard.kde.org/r/124934/diff/1/?file=398792#file398792line343>
> >
> >     can you give me an example code and explicit instructions to trigger this bug? recursive functions e.g. should work fine - and they do in principal just what you say: finding the function in its own context.
> >     
> >     also, this should be a separate commit. ideally, it should also be covered by a unit test in oldcpp or kdev-clang, depending on what you used for testing.

By function name I meant function identifier in its declaration/definition. I'm using kdev-clang.

example code:
int fun(float param)
{
    bool local;
}

duchainify output:
Context:
  Top-Context "Global" (owner:  0x0 ) 0x122cb90 "" [ "" ] [ (0, 0)  ->  (2147483647, 2147483647) ] "/home/mcencora/projects/test/main2.cpp" 
     Declaration: "int fun (float)" [ "fun" ] 0x1296b30 (internal ctx: 0x12968d0 ) [ (0, 4)  ->  (0, 7) ] , defined,  0 use(s). 
      Definition: [ (0, 4)  ->  (0, 7) ] 
    Context "Function" (owner:  0x1296b30 ) 0x12968d0 "fun" [ "fun" ] [ (0, 0)  ->  (3, 1) ] "" 
       Declaration: "float param" [ "fun::param" ] 0x1296ac0 (internal ctx: 0x0 ) [ (0, 14)  ->  (0, 19) ] , no definition,  0 use(s). 
      Context "Other" (owner:  0x0 ) 0x1296990 "" [ "fun" ] [ (1, 0)  ->  (3, 1) ] "" 
         Declaration: "bool local" [ "fun::local" ] 0x1296a50 (internal ctx: 0x0 ) [ (2, 9)  ->  (2, 14) ] , no definition,  0 use(s).

The algorithm inside itemRangeUnderCursor is "greedy" - it tries to find the most specific context.
When moving mouse over the "fun" identifier, it finds that "Function" context contains the range of this identifier, and later it tries to find a declaration named "fun" inside it which of course fails.

I think the ideal fix would be to store a map of (range, declaration/use ptr) instead of traversing the context tree, but I don't know if that wouldn't break other cases.


> On Aug. 26, 2015, 8:29 p.m., Milian Wolff wrote:
> > util/activetooltip.cpp, line 213
> > <https://git.reviewboard.kde.org/r/124934/diff/1/?file=398798#file398798line213>
> >
> >     if `handleRect_` is invalid, will this affect the union?

No, I've checked with simple app. Union with invalid rect results in same rect as source.


- Maciej


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


On Aug. 26, 2015, 3:01 p.m., Maciej Cencora wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124934/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 3:01 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Actually make use of ActiveToolTip::addExtendedRect (renamed to setHandleRect).
> This functionality was lost with d77a4ecda51c324eec589167da3214d21a1eff42
> 
> Minor change was also needed in itemRangeUnderCursor:
> DUContext::findContextAt for cursor at function name returns the function's context, searching for function's name inside it will fail - we need to search in parent context.
> 
> 
> Diffs
> -----
> 
>   language/duchain/duchainutils.cpp a1ff0f5 
>   plugins/contextbrowser/contextbrowser.cpp 268679f 
>   plugins/patchreview/patchhighlighter.cpp 48dd127 
>   shell/mainwindow_p.cpp 134d48d 
>   shell/workingsets/workingsettoolbutton.cpp 2efadb9 
>   util/activetooltip.h 35b25c0 
>   util/activetooltip.cpp e45116c 
> 
> Diff: https://git.reviewboard.kde.org/r/124934/diff/
> 
> 
> Testing
> -------
> 
> Manual
> 
> 
> Thanks,
> 
> Maciej Cencora
> 
>

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