Show all KTextEditor view bars in a central location
ClosedPublic

Authored by mwolff on Feb 18 2017, 1:12 AM.

Details

Summary

Instead of showing the view bars beneath every editor view, we
now always use the central location beneath the central widget.
This greatly improves the usability of the find/replace view bars
when using split views.

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.
mwolff created this revision.Feb 18 2017, 1:12 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptFeb 18 2017, 1:12 AM
cullmann edited edge metadata.Feb 18 2017, 4:46 PM

Looks reasonable, that interface made Kate cope better with split-views + search bar, too.

Christoph, Dominik,: When comparing this implementation to the one in Kate:

  • why exactly is the BarState wrapper needed?
  • i.e. is slotUpdateBottomViewBar really needed, isn't that handled in KTextEditor? If it's needed, shouldn't this be in KTextEditor to call {show,hide}ViewBar?
  • Kate guards against nullptr view bars, is this really needed? Put differently - when would this ever occur? Shouldn't this be handled by KTextEditor to not call addWidgetToViewBar?
In D4657#87433, @mwolff wrote:

Christoph, Dominik,: When comparing this implementation to the one in Kate:

  • why exactly is the BarState wrapper needed?
  • i.e. is slotUpdateBottomViewBar really needed, isn't that handled in KTextEditor? If it's needed, shouldn't this be in KTextEditor to call {show,hide}ViewBar?
  • Kate guards against nullptr view bars, is this really needed? Put differently - when would this ever occur? Shouldn't this be handled by KTextEditor to not call addWidgetToViewBar?

Hmm, for the null check, I guess that is not needed, more caution ;)
For the other two things: Isn't the problem that KTextEditor can't keep track of view switches and has no concept of active view and the slotUpdateBottomViewBar() is used to restore the right view bar if the view is changed?
But that code is old, I could be mistaken ;=) And the state var in the BarState is awful, it means "visible" or?

kfunk added a subscriber: kfunk.Feb 21 2017, 11:11 AM
kfunk added inline comments.
shell/ktexteditorpluginintegration.cpp
175

Minor: Use override

mwolff updated this revision to Diff 11607.Feb 21 2017, 10:47 PM

use override, track view changes

kfunk accepted this revision.Feb 22 2017, 9:01 AM

LGTM

This revision is now accepted and ready to land.Feb 22 2017, 9:01 AM
This revision was automatically updated to reflect the committed changes.