Review Request 110960: FramestackWidget: Make keyboard navigation useful

[Nicolai Hähnle]

Guys and gals,

what's up with this? I know you probably have a shit-ton of work on your
hands. Still, I would appreciate *some* sign of life, even if it's just a
single line about when you'll find the time for this.

Cheers,
Nicolai


On Wed, Jun 12, 2013 at 4:08 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110960/
>   Review request for KDevelop and Niko Sams.
> By Nicolai Hähnle.
>
> *Updated June 12, 2013, 2:08 p.m.*
> Changes
>
> I have been a bit overzealous in eliminating the clicked() signals:
>
> When the user navigates away from the location of the currently selected
> frame, a direct moues click on that frame in the view should still return
> us to the location of that frame.
>
> The new diff should be identical to the old one, except that
> m_frame->clicked() is again connected to frameSelectionChanged().
>
> There is now an unfortunate redundant call to openDocument() when the user
> selects a new frame by clicking on it: it is called once when the clicked()
> signal fires, and a second time when the selectionModel()->currentChanged()
> signal fires. I don't know how to easily and _cleanly_ detect this
> situation, and it really shouldn't hurt - but if you know a way to improve
> this, please do tell me.
>
>   Description
>
> Allow users to navigate the Frame Stack Toolview using the arrow keys
> and tab/shift-tab without distraction to locate the thread(s) and
> frame(s) of interest.
>
> This required the following changes:
>
> 1) Add IDocumentController::DoNotFocus; assorted changes in
> DocumentController and MainWindow.
>
> The intention is clear: Allow opening and showing a document without
> yanking the focus away from where it currently is. This could also be
> interesting for other toolviews (e.g., keyboard navigation in the Code
> Browser).
>
> Focusing the new view is hidden inside MainWindow::activateView, so
> I added an overloaded variant of this method which allows the caller
> to choose whether the view should get the focus.
>
> 2) Use DoNotFocus in the relevant parts of the debugger.
>
> 3) Change FramestackWidget signal wiring to react to all types of input
>
> Previously, FramestackWidget connected its slots to the item views'
> clicked() signal, which only reacts on mouse input. Use the selection
> models' currentChanged() signal instead, which reacts to other types of
> input, in particular the keyboard.
>
> As a consequence, the connections have to be set up in
> currentSessionChanged(), because that's where the model is fully set up.
>
> Relatedly, switch only m_frames->rootIndex in currentThreadChanged()
> rather than the entire model, because the entire model doesn't actually
> change.
>
> 4) There is one unrelated change, which is that I moved a connection
> attempt to checkFetchMoreFrames down into currentSessionChanged;
> otherwise, the connection does not succeed (and QObject complains
> about it on the console).
>
> I noticed that there is a related bug report, bug #316873, about this
> fetch-more-frames feature; the bug report ends rather vaguely without
> resolution, and I only noticed it after starting this Review Request.
> However, fetching more frames automatically, triggered by scrolling,
> works for me.
>
> That's pretty much it. I hope you consider this a useful new feature,
> and I hope the patch looks good to you. If you see any problems, or if
> I have misstepped outside the coding guidelines at some point, please
> let me know!
>
>   Testing
>
> Only manual testing - I don't know if/how this could be tested
> automatically:
>
> Within {my own project, the threads test case from kdevelop,
> kdevelop itself} I have interrupted the debug run and done a lot
> of mouse- and keyboard-based back and forth between threads and
> frames. I also explicitly checked that loading additional frames
> still works. That's pretty much it.
>
>   *Bugs: * 316873 <http://bugs.kde.org/show_bug.cgi?id=316873>
> Diffs (updated)
>
>    - debugger/framestack/framestackwidget.h (9794353)
>    - debugger/framestack/framestackwidget.cpp (1be6aff)
>    - interfaces/idocumentcontroller.h (434cb4d)
>    - shell/debugcontroller.cpp (d1924fb)
>    - shell/documentcontroller.cpp (660637f)
>    - sublime/mainwindow.h (421ee57)
>    - sublime/mainwindow.cpp (dce4bf7)
>
> View Diff <http://git.reviewboard.kde.org/r/110960/diff/>
>



-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130621/44fc712a/attachment-0001.html>