D28580: [Kleopatra] Fix size saving/restore

[Andrey Legayev]

andreylegayev marked 3 inline comments as done.
andreylegayev added a comment.


  @dfaure now I know that I need to click "Submit" :)

INLINE COMMENTS

> dfaure wrote in mainwindow.cpp:706
> Shorter form:
> 
>   setVisible(cfgGroup.readEntry("hidden", false));

Replaced, but `setHidden()`

> dfaure wrote in mainwindow.cpp:696
> The code to load autosaved settings is the end of KMainWindow::setAutoSaveSettings.
> This works for all other KMainWindow-based applications, you know ;)
> 
> To enable the autosave functionality, you call setAutoSaveSettings at creation time, and it will load the previously saved settings.
> 
> readProperties and saveProperties are called from here, so obviously not just for session management purposes.
> 
> I would really like kleopatra to do like all other KMainWindow-based applications and rely on the KMainWindow code as much as possible. This makes long term maintenance easier.
> 
> I see that we're missing something similar for dialogs, which means we should have something in a KDE Frameworks about that, but for now I'm ok with your solution of duplicating a bit of code in every one of kleopatra's dialogs. For the mainwindow, though, I'd like it to do the right thing since that is available.

Thanks for the hint with setAutoSaveSettings(). I've added it to constructor.
I've reverted back the rest to original version, but removed geometry save/load.

`readProperties` and `saveProperties` are really called only from session management. 
I've re-checked it by setting breakpoints and by investigating source code:
https://github.com/vasi/kdelibs/blob/master/kdeui/widgets/kmainwindow.cpp

Call chain from constructor - `readProperties()` isn't called:

  1. setAutoSaveSettings()
  2. applyMainWindowSettings()

Call chain from session management:

  1. restore()
  2. readPropertiesInternal()
  3. applyMainWindowSettings() + readProperties()

REPOSITORY
  R168 Kleopatra

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

To: andreylegayev, dfaure
Cc: dfaure, mlaurent, kde-pim, andreylegayev, fbampaloukas, dcaliste, dvasin, rodsevich, ach, winterz, vkrause, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200411/d15d3806/attachment.html>