Contextbrowser: Ability to show combined problems and decl tooltip
ClosedPublic

Authored by thomassc on Jan 13 2019, 5:57 PM.

Details

Summary

Without this patch, if both a problem and a declaration are available for a given cursor location,
KDevelop would only show the problem tooltip. This makes the declaration tooltip inaccessible, which
can be an annoyance. This patch makes it show a combined tooltip instead in this case, allowing to access both
the problem(s) and the declaration.

Edit: Instead of hardcoding something like layout->setContentsMargins(2, 2, 2, 2), is there maybe any kind of style property that can be used to define a reasonable margin size?

Test Plan

Tested manually.

Diff Detail

Repository
R32 KDevelop
Branch
show_problem_and_decl_tooltip (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7343
Build 7361: arc lint + arc unit
thomassc created this revision.Jan 13 2019, 5:57 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJan 13 2019, 5:57 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
thomassc requested review of this revision.Jan 13 2019, 5:57 PM
thomassc edited the summary of this revision. (Show Details)
mwolff added a subscriber: mwolff.Jan 15 2019, 1:51 PM

could you show a screenshot of this in action? code-wise I like what I'm seeing, good work! Feature-wise, I also agree that we need to do something about the issue you describe.

thomassc updated this revision to Diff 49938.Jan 20 2019, 2:43 PM

Cosmetic improvement to combined problems and decl tooltip

This is how it looks on my system:

This is how a single tooltip looks (for comparison):

mwolff accepted this revision.Jan 21 2019, 9:51 AM

lgtm, thanks!

unrelated - but potential future work: does keyboard navigation work across tooltips? I.e. 'ALT + ARROW'?

This revision is now accepted and ready to land.Jan 21 2019, 9:51 AM
This revision was automatically updated to reflect the committed changes.

Regarding keyboard navigation, it does not work for combined tooltips unfortunately. However, I wasn't aware of the existence of that feature at all before you asked about it ...

Yeah, the keyboard navigation needs some work anyways - it's buggy even without combined tooltips :) Something for the future to work on!

I'm looking into the keyboard navigation now

Done, keyboard navigation should work now - at least it does according to my testing! Please test this as well and report back if you spot any issues.

Cheers

rjvbb added a subscriber: rjvbb.Feb 22 2019, 11:12 AM

This seems like a big productivity progress (can't test it yet because using the 5.3 branch) but I have a concern that it is only a sufficient solution for those of us with (very) big screens. Esp. problem reports can be long, and I've never seen a scrollbar in them, AFAICR. Another concern is that the combined popup might be so big it's going to obscure parts of the code you'd want to keep visible for context.

More to the point: why show both popups everywhere on the line? Why not simply show the problem report only if there is no context popup? I find that instinctively I "fish" for problem popups on the leading and (esp.) trailing empty parts of a line, probably from a reflex to keep the entire code part unobscured by the popup?

Pity that no link was made to by bug #385012; I'd have noticed this proposition and raised my concerns earlier...

There can be scrollbars in problem reports, see the attached screenshot:

Both popups aren't shown everywhere on the line. The declaration tooltip is only shown if hovering over the respective range. The problem tooltip (on its own) is also shown if the problem is close-by, see findProblemsCloseToCursor() (where I'm not sure whether this region is maybe a bit too large: I think that sometimes this tooltip actually gets in the way accidentally due to this).

I find that instinctively I "fish" for problem popups on the leading and (esp.) trailing empty parts of a line, probably from a reflex to keep the entire code part unobscured by the popup?

For me it is exactly the opposite. I hover the range that is underlined red, because this is where KDevelop marks the problem. I cannot recall having any concerns about code being obscured then (because one can always move the pointer away to have the tooltip disappear, and the tooltip shows up sufficiently far away from the current line). In my opinion, I would actually consider not showing the problem tooltip when hovering anywhere else on the line, because this is where the tooltip is unexpected (since the red marking is somewhere else) and thus it actually gets in the way for me then accidentally. I like Sven's suggestion from the bugreport you linked about an exclamation mark somewhere on the line that can be hovered. I think this would be a good visual indication, instead of simply empty space that happens to be on the same line as a problem.

rjvbb added a comment.Feb 23 2019, 4:44 PM

For me it is exactly the opposite. I hover the range that is underlined red

For it's the entire line (across the full editor view width) that is rendered with a reddish background, no underlining here. I can't remember how I obtained that mode.
So I hover somewhere on that line where there is no code -- if I don't simply glance at the error in the (docked) problem reporter toolview. Most of the time I don't use the majority of the problem report, esp. in situations like when you try to print something not supported by qDebug().

because one can always move the pointer away to have the tooltip disappear

I find that it mostly gets in the way when I start correcting, which is a situation in which I don't want to have to move my hand back to the mouse or trackpad. And there is some form of hysteresis too, where the popup stays up when you move the cursor to a location where it wouldn't yet be triggered.

(since the red marking is somewhere else)

See here:

I put the cursor somewhere on the problem line where I'd expect the popup not to cover code from the code below the problem line (please disregard the fact that it's a different function here).
Error reports only come with a line number so you don't have any additional information to limit the spatial context where you can trigger the popup.

Note how the red background is also apparent in the scrollbar/outline view, which I find extremely practical.

Here's my version of the patch, adapted for the 5.3 branch:

It still uses your combined view, but only if the height is not more than 1/3 of the current screen height. If higher, it inverses the logic and will display the declaration instead of the problem popup when both are available. I think this is a logical thing to do because the problem popup will be available for the entire line.
Note that the window in that screenshot taken on my notebook is maximised vertically and takes up most of the horizontal screen estate too. Just about every combined popup that I have (not :)) seen gets rejected by my 1/3 rule. On my Mac with its 1080p screen the smaller ones get through but there too they are often too big because problem reports can be so verbose.

I was under the impression that these popups were handled by 2 distinct plugins; had I known that they're created in this unique function I would have presented that inversed priority approach long ago.
In combined popups I'd probably put the declaration widget on top because it is more likely to be the shortest of the two, and you probably lose less if the lower part of a problem report falls off the bottom of the screen.

Interesting, I didn't know about the mode which highlights problematic lines. Thanks a lot for pointing it out. I had to look for it, and it turned out that this setting is at: Settings dialog -> Language Support -> Semantic Code Highlighting block -> Highlight problematic lines. I can't remember deactivating this, so it's probably off by default? It seems potentially very useful though, given that the underlines are sometimes easy to overlook, and the red line highlighting also shows up in the scrollbar minimap as you write. When this setting is active, the region where the problem tooltip shows up indeed makes more sense to me. It still doesn't match up though, for example if there are some whitespace lines before the line with the error, then the tooltip will also show when hovering somewhere over these whitespace lines.

Error reports only come with a line number so you don't have any additional information to limit the spatial context where you can trigger the popup.

Actually in your screenshot the error underlining is also visible which is confined to the precise error location ("NewDoc" is underlined in both error lines). The underlining is just hard to see here. This might be due to this KTextEditor bug: https://bugs.kde.org/show_bug.cgi?id=403868

Omitting the problem tooltip if it's large makes it a bit unpredictable to the user. He/she then won't know in advance whether hovering over a location that might show a combined tooltip will actually show it or not.

rjvbb added a comment.Feb 24 2019, 8:33 AM
Omitting the problem tooltip if it's large makes it a bit unpredictable to the user. He/she then won't know in advance whether hovering over a location that might show a combined tooltip will actually show it or not.

True, which is why I wouldn't have thought of combined popups myself (and planned to remove those initially). But I'm not sure what's worse, often getting a wall-of-text-in-your-face popup or not being entirely certain what you'll get. You'd always get the information that changes as a function of where on the line you hover (the decl. popup), and I think this is a feature that users learn about while using the product anyway.

I'd have to use my version a bit longer on my large screen to see how I react to this aspect; it could be an additional argument in favour of putting the decl. popup in top (because it's the part that changes).

rjvbb added a comment.Feb 24 2019, 8:44 AM

About the combined popups: could you downsize the 2 components so they always have the same (reasonable) size which could be coupled to screen height?

rjvbb added a comment.Feb 25 2019, 8:41 AM

Here's a very good example of an overly large problem report:

Note how the problem reporter toolview collapses the error message, leaving just the bit that is useful here. Would it be possible to do the same thing in the popup (= where the user can expand the message)?

(Not that the problem toolview is so convenient here if you do want to go through the list of all candidates, because it doesn't wrap - but that's a different issue).

Personally, I don't think that I share the concerns about large tooltips. Even in your screenshot it looks fine to me. Since as written before, it can be very easily hidden if desired (e.g. just by clicking in the editor text area). I'd much rather have as much relevant information at once instead of having to click another "expand" button each time to see more. But that's just my personal opinion. Maybe others can comment too.

rjvbb added a comment.Feb 25 2019, 2:24 PM

Even in your screenshot it looks fine to me.

Well, in the last screenshot the popup is too large in a functional sense (screen junk) and thus gets in the way more than necessary, but not in an absolute sense, indeed. I haven't created a single example where that is the case, in part because that means I have to revert my patch ;)

Since as written before, it can be very easily hidden if desired (e.g. just by clicking in the editor text area).

I have a very strong impression that popups can also be triggered if the cursor is moved while editing text. I do NOT want to have to stop my typing in that case, nor should I have to hit Esc. ... and I might be looking at my keyboard because entering a sequence of characters I cannot type blind reliably.

Which reminds me that maybe there is (or should be) a mode where the cursor is hidden while typing which also deactivates all popups except the code completion ones (which very often get in the way too).

Rene, instead of thinking about what-ifs, maybe try it out first? Most notably, the tooltips only show up when you press ALT and keep it pressed. Otherwise, the tooltips don't show up - unless you hover code with your mouse cursor. Moving the keyboard edit cursor won't ever trigger tooltips. Or are you somehow moving your mouse cursor while typing?! Don't do that :)