Toolbar state was not being saved / restored
ClosedPublic

Authored by tcanabrava on Jan 13 2017, 3:19 PM.

Details

Summary

Create a new entry in the KCofig to save / restore
the ToolBars of the toolviews

Signed-off-by: Tomaz Canabrava <tcanabrava@kde.org>

Test Plan

manual testing

Diff Detail

Repository
R33 KDevPlatform
Branch
fix_load_toolbars
Lint
No Linters Available
Unit
No Unit Test Coverage
tcanabrava updated this revision to Diff 10141.Jan 13 2017, 3:19 PM
tcanabrava retitled this revision from to Toolbar state was not being saved / restored.
tcanabrava updated this object.
tcanabrava edited the test plan for this revision. (Show Details)
tcanabrava added reviewers: kfunk, apol.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJan 13 2017, 3:19 PM
antonanikin requested changes to this revision.Jan 16 2017, 7:38 AM
antonanikin added a reviewer: antonanikin.
antonanikin added a subscriber: antonanikin.
antonanikin added inline comments.
sublime/idealcontroller.cpp
123

add this as signal receiver.

This revision now requires changes to proceed.Jan 16 2017, 7:38 AM
tcanabrava updated this revision to Diff 10233.Jan 16 2017, 1:10 PM
tcanabrava edited edge metadata.
  • Pass 'this' to connect
kfunk requested changes to this revision.Jan 16 2017, 1:21 PM
kfunk edited edge metadata.
kfunk added inline comments.
sublime/idealcontroller.cpp
121 ↗(On Diff #10233)

There's no need for the indirection through QVariant, is there?

You can just use readEntry(..., true) here.

126 ↗(On Diff #10233)

Dito, just use writeEntry(..., foo->isChecked())?

This revision now requires changes to proceed.Jan 16 2017, 1:21 PM
tcanabrava marked 2 inline comments as done.Jan 20 2017, 3:23 PM
tcanabrava added inline comments.
sublime/idealcontroller.cpp
126 ↗(On Diff #10233)

Too Much QSettings makes tomaz misbehave.

tcanabrava updated this revision to Diff 10390.Jan 20 2017, 3:23 PM
tcanabrava edited edge metadata.
tcanabrava marked an inline comment as done.
  • Pass 'this' to connect
  • Just use booleans, this is not QSettings.
mwolff requested changes to this revision.Jan 22 2017, 8:20 PM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.
mwolff added inline comments.
sublime/idealcontroller.cpp
121

spaces after commas, here and below

121

the windowTitle is a translated string, and it's not guaranteed to be unique, is it? I don't think it's a good idea to use that as an identifier. Can you instead use the dockObjectName (see above)?

This revision now requires changes to proceed.Jan 22 2017, 8:20 PM

@tcanabrava, add link to BUG 360539 to the revision description.

tcanabrava updated this revision to Diff 10844.Feb 2 2017, 11:24 AM
tcanabrava edited edge metadata.
tcanabrava marked 2 inline comments as done.
  • Pass 'this' to connect
  • Just use booleans, this is not QSettings.
  • Use dockObjectName inste of windowTitle
mwolff accepted this revision.Feb 2 2017, 11:28 AM
mwolff edited edge metadata.

one minor code style nitpick, then it's ready to go

thanks!

sublime/idealcontroller.cpp
122 ↗(On Diff #10233)

please introduce a newline before the this and indent the lambda body

connect(toolBar->toggleViewAction(), &QAction::toggled,
        this, [toolBar, dockObjectName]() {
            ...
        });
tcanabrava marked an inline comment as done.Feb 8 2017, 2:56 PM
mwolff added a comment.Feb 9 2017, 7:48 PM

did you commit it yet? if not, please do

Was travelling. :)

tcanabrava marked an inline comment as done.Feb 13 2017, 12:19 PM
This revision was automatically updated to reflect the committed changes.