This issue should be solved. Closing this report as fixed.

I also meant to mention that I think I understand now why calling the octave_dock_widget::save_settings function from the octave_dock_widget destructor wsan't working properly: save_settings is a virtual function but when called from the destructor it won't behave as a virtual function normally does.  So any overloaded versions in derived classes won't be called.

Another question:  Do we really need the following signal/slot connections now (from main_window::construct in libgui/src/main-window.cc)?

    connect (qApp, &QApplication::aboutToQuit,
             m_command_window, &terminal_dock_widget::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_history_window, &history_dock_widget::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_file_browser_window, &files_dock_widget::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_doc_browser_window, &documentation_dock_widget::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_workspace_window, &workspace_view::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_editor_window, &file_editor_interface::save_settings);

    connect (qApp, &QApplication::aboutToQuit,
             m_variable_editor_window, &variable_editor::save_settings);

It seems to me that now that we are able to close the main window without exiting the application, saving the settings should be linked to something other than the QApplication::aboutToQuit signal.

If I start Octave in command line mode with the new terminal widget enabled and

• execute the "desktop" command
• close the desktop and return to the command line
• exit Octave

then, the only save_settings functions that are executed are for dock widgets that remain open when Octave exits and they appear to be called twice: first because of the connection to the QApplication::aboutToQuit signal and then again from the base_qobject destructor when we now explicitly close them.

Now that the dock widgets and the resource manager object are both owned by the base_qobject, it should be safe to save the settings from the dock widget destructors because we can ensure that the resource manager object is deleted after all the dock widgets and the main window have been deleted.

So it seems like the attached patch should work.  With it, I do see that the settings are saved when the objects are deleted, but if I start the desktop from the command line multiple times, the window layout seems to be lost and I end up with only the command window open, or the command window and any dock widgets that were undocked when the desktop was last closed (they do remain undocked, as expected).

Is the geometry and layout info wiped out before the destructor is called?  How?


(file #51566)

@Torsten: Thanks.  I also pushed the following change:

@Torsten: OK.  I'm also wondering now whether the lines like

    if (m_history_window->isFloating ())
      m_history_window.clear ();

in the main_window destructor are needed now?  These variables like m_history_window should only be deleted if they are children of the main_window object and since they are stored as QPointer objects they will be set to nullptr when they are deleted.  Then if they are needed again at the command line, they will be recreated.

The behavior is better after the following change but I don't fully understand why.

http://hg.savannah.gnu.org/hgweb/octave/rev/f8322ca49578

comment #14:

> @ttl:  So deleting a window/widget doesn't trigger a close signal?  If closing causes other signals to be emitted, do we need to use deleteLater for the widgets that are closed or is it safe to just delete them immediately after calling close?

I think this is only necessary when the 'close' and 'delete' are executed in another thread than the closeEvent. But maybe deleteLater would be the more robust way. 

> I suppose we need to do the same for all the dock widgets that are now owned by base_qobject?

Yes, the documentation widget was just an example I have chosen for the test.

> Is it safe to call close on a widget that is already closed?  Maybe we could use

> if (m_main_window)
>   delete m_main_window;
> else
>   {
>     if (m_documentation_widget)
>       m_documentation_widget->close ();
>
>     if (...)
>       ...->close ();
>
>     ...
>   }
>
> // Or just delete?
> m_documentation_widget->deleteLater ();
> ...

Since we are not setting the property Qt::WA_DeleteOnClose we have to delete the widgets after closing (or better deleteLater). deleteLater () does not trigger close. So we have to use to upper code but with deleting the widgets afterwards.

There is also the destroyed signal triggered just before the widget is deleted but I am reluctant to use this for any required actions like saving the settings.

I have attached a patch that exemplary closes the documentation widget (and therefore triggers saving the settings) if the widget is existing and if the main window is not.

(file #51490)

We do not currently keep track of which ones were opened from the command line.

With the new terminal widget, it will be possible to have the following sequence (the lines marked with ? is the behavior that I would expect but might still be up for debate):

• open a dockable widget from the command line
• open the desktop from the command line

   ? open dockable widgets should remain undocked

• dock one of the widgets that was initially opened from the command line or undock one that was originally opened (and docked) with the main desktop window
• close the desktop window and return to the command line

   ? any undocked widgets should remain open
   ? widgets that were originally opened at the command line but were docked when the desktop were closed should be closed with the main desktop window

When restarting Octave in command line mode, should it open and display widgets that were undocked the last time it was closed?

If starting Octave in GUI mode after a sequence like the one described above for a command line session, should the dockable widget configuration from the previous command-line session be restored, or should CLI/GUI widget positions be handled separately?

I realize now that with the current design of saving the settings in the destructor and having those objects managed by the base_qobject, then, at the point when they are deleted, they will never be attached to the main_window.  So, yes, saving settings there is the wrong thing to do.

But why should we even need to save the settings when the application closes?  Aren't the settings saved any time there is a change in the appearance?  Or are they normally only saved when the window is closed?

I just noticed that after my recent changes that when starting Octave with the --gui option, only the command window is showing.  I can bring all of the others back individually or by resetting the default window layout but then at the next restart they are once again not shown.  Argh!!!

For me, this change fixes the problem with savings settings:

http://hg.savannah.gnu.org/hgweb/octave/rev/b16748c8aa4d

Is it OK to call save_settings from the dock widget destructor?

It looks like the closeEvent of the dock widgets isn't triggered either, which prevents the widgets from saving their settings.

Confirmed, with the whole GUI or with only the doc browser launched from cli. This is a regression: temporary file and directory are correctly deleted in Octave 6.2.