WIP: AssistantPopups NG?
ClosedPublic

Authored by kfunk on Mar 10 2016, 8:55 AM.

Details

Summary

WIP: Problem context on magic modifier

WIP: Kill AssistantPopup

WIP: Extend ProblemNavigationContext with assistants

Add contexts to navigate through assistant actions.

Screencast: http://krf.kollide.net/files/work/kdevelop/kdevelop-new-assistant.gif

Issues this patch attempts to fix:

  • No longer automatically popup a widget whenever there's a problem (distracting!)
  • Only popup when invoked (via Alt)
  • Show problems on keyboard activation (via Alt, wasn't possible before)
  • We can use more text in the solution assistant descriptions (since we requested them, we can cover more space implicitly)
  • No longer creates a OpenGL context each time there's an error (this has been slow at times, using the old assistant popup. There was a noticable lag while typing on heavy load)
  • No longer issues wrt "when to hide" the popup again. Because it isn't automatically invoked in the first place
  • Uses well-known navigation context behavior (Alt + arrow keys + enter), which the user already knows how to use. Mouse navigation works as well.

CCBUG: 354461
BUG: 358499

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kfunk updated this revision to Diff 2653.Mar 10 2016, 8:55 AM
kfunk retitled this revision from to WIP: AssistantPopups NG?.
kfunk updated this object.
kfunk edited the test plan for this revision. (Show Details)
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptMar 10 2016, 8:55 AM
kfunk updated this object.Mar 10 2016, 9:06 AM
kfunk added a project: KDevelop.
brauch added a subscriber: brauch.Mar 10 2016, 9:56 AM

Good plan, +1. Thanks.

apol added a subscriber: apol.Mar 10 2016, 10:44 AM

+1 That could work. I'd get this in master though.

kfunk updated this object.Mar 10 2016, 10:59 AM
In D1100#20877, @apol wrote:

+1 That could work. I'd get this in master though.

As said on IRC: /me would definitely like to get this into 5.0. The assistant popup behavior as-is is unacceptable, IMO. It really distracts *me* during typing in KDevelop.

mwolff added a subscriber: mwolff.Mar 10 2016, 9:22 PM

I like the idea, can you push your branch already somewhere so we can try it out? And I agree with Kevin btw. that we should aim for a proper solution to this problem for 5.0. I'd rather we introduce this new workflow there instead of waiting for 5.1.

I've marked a few nitpicks, probably in code that you did not write. So ignore at will.

language/duchain/navigation/problemnavigationcontext.cpp
83

const&

193

const&

language/duchain/navigation/problemnavigationcontext.h
41

const&

41

and also make the method const, if possible

49

const&

kfunk added a comment.EditedMar 10 2016, 9:39 PM

Pushed a new branch to kdevplatform: assistantpopup-ng

Feel free to hack on it, we can squash & rebase on top of 5.0 later.

language/duchain/navigation/problemnavigationcontext.h
41

It's a method override -> can't change signature. Same below.

Played around with it now, and still like it. Here are some suggestions for improvements we should implement before merging this:

a) inline the solution page with the problem page, but keep a "jump to solutions" link as first link
b) keep compatibility with the old behavior, i.e. have "alt + 1" automatically apply the first solution and so forth
c) potentially strip very long problem descriptions and show a "show more" link. only do this if this really is an issue
d) the problem tooltips should be embedded into the context browser toolview when that is shown

Unrelated to this, kfunk also suggested to add a "Hint: you can press ALT to show this dialog" to tooltips invoked by the mouse. Useful to teach newcomers on how to use this feature with the keyboard. We could also add a "help" link that explains the workflow in more detail, i.e. that alt + 1-9 applies solutions, alt + arrow navigates between links,and alt + enter accepts a link

We also have to fix the static assistants in some way, most notably the rename assistant is broken now. @kfunk's suggestion is to introduce "fake" problems at the location where a rename/signature change was done to make it possible to apply the assistant to the other places.

Another issue I see are parse errors such as missing ";", where the error is reported a few lines below the line you are currently writing (and forgot to add the ";").

kfunk planned changes to this revision.Mar 14 2016, 10:43 AM
This revision was automatically updated to reflect the committed changes.