[Differential] [Requested Changes To] D4122: Toolbar state was not being saved / restored

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a reviewer: mwolff.
mwolff added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> idealcontroller.cpp:121
> +      KConfigGroup cg(KSharedConfig::openConfig(), "UiSettings/Docks/ToolbarEnabled");
> +      toolBar->setVisible(cg.readEntry(qPrintable(toolBar->windowTitle()),true));
> +      connect(toolBar->toggleViewAction(), &QAction::toggled, this, [toolBar](){

spaces after commas, here and below

> idealcontroller.cpp:121
> +      KConfigGroup cg(KSharedConfig::openConfig(), "UiSettings/Docks/ToolbarEnabled");
> +      toolBar->setVisible(cg.readEntry(qPrintable(toolBar->windowTitle()),true));
> +      connect(toolBar->toggleViewAction(), &QAction::toggled, this, [toolBar](){

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)?

REPOSITORY
  R33 KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D4122

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: tcanabrava, apol, antonanikin, kfunk, #kdevelop, mwolff
Cc: mwolff, antonanikin, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170122/22e6c165/attachment.html>