Mercurial > octave
diff libgui/src/resource-manager.cc @ 31619:ad014fc78bd6
use individual local gui_settings objects
Previously, we created a single gui_settings object (derived from
QSettings) and accessed it from the resource_manager object. That
design is not necessary and is not the way QSettings was designed to
be used. Instead of managing a single object, we should be using
individual QSettings objects where needed. Each individual QSettings
object manages thread-safe access to a single global collection of
settings. The Qt docs say that operations on QSettings are not thread
safe, but that means that you can't create a QSettings object in one
thread and use it in another without some locking. I'm not sure
whether we were doing that correctly, but with this change it no
longer matters. Each QSettings object does perform locking when
reading or writing the underlying global data.
* resource-manager.h, resource-manager.cc
(resource_manager::m_settings): Delete data member.
(resource_manager::get_settings): Delete.
* annotation-dialog.cc, QTerminal.cc, QTerminal.h, command-widget.cc,
command-widget.h, community-news.cc, dialog.cc,
documentation-bookmarks.cc, documentation-bookmarks.h,
documentation-dock-widget.cc, documentation-dock-widget.h,
documentation.cc, documentation.h, dw-main-window.cc,
dw-main-window.h, external-editor-interface.cc, files-dock-widget.cc,
files-dock-widget.h, find-files-dialog.cc, history-dock-widget.cc,
history-dock-widget.h, file-editor-interface.h, file-editor-tab.cc,
file-editor-tab.h, file-editor.cc, file-editor.h, find-dialog.cc,
octave-qscintilla.cc, main-window.cc, main-window.h, news-reader.cc,
octave-dock-widget.cc, octave-dock-widget.h, qt-interpreter-events.cc,
qt-interpreter-events.h, release-notes.cc, resource-manager.cc,
resource-manager.h, set-path-dialog.cc, settings-dialog.cc,
settings-dialog.h, shortcut-manager.cc, shortcut-manager.h,
terminal-dock-widget.cc, terminal-dock-widget.h, variable-editor.cc,
variable-editor.h, welcome-wizard.cc, workspace-model.cc,
workspace-model.h, workspace-view.cc: Use local gui_settings objects
instead of accessing a pointer to a single gui_settings object owned
by the resource_manager object.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Fri, 02 Dec 2022 14:23:53 -0500 |
parents | 1f57ea5dfd4a |
children | 6dfaaf8ecf3b |
line wrap: on
line diff
--- a/libgui/src/resource-manager.cc Fri Dec 02 15:51:44 2022 -0500 +++ b/libgui/src/resource-manager.cc Fri Dec 02 14:23:53 2022 -0500 @@ -64,17 +64,15 @@ namespace octave { resource_manager::resource_manager (void) - : m_settings_directory (), m_settings_file (), m_settings (nullptr), + : m_settings_directory (), m_settings_file (), m_temporary_files (), m_icon_fallbacks () { // Location, name, and format of settings file determined by // settings in qt_application class construtor. - m_settings = new gui_settings (); - check_settings (); - m_settings_file = m_settings->fileName (); + m_settings_file = settings.fileName (); QFileInfo sfile (m_settings_file); m_settings_directory = sfile.absolutePath (); @@ -82,8 +80,6 @@ resource_manager::~resource_manager (void) { - delete m_settings; - for (int i = m_temporary_files.count () - 1; i >=0; i--) remove_tmp_file (m_temporary_files.at (i)); } @@ -111,12 +107,11 @@ // FIXME: can we somehow ensure that the settings object will always // be initialize and valid? - if (m_settings) - { - // get the locale from the settings if already available - language = m_settings->value (global_language.key, - global_language.def).toString (); - } + gui_settings settings; + + // get the locale from the settings if already available + language = settings.value (global_language.key, + global_language.def).toString (); // load the translations depending on the settings if (language == "SYSTEM") @@ -151,24 +146,18 @@ int theme = global_icon_theme_index.def.toInt (); - if (m_settings) + gui_settings settings; + + // check for new and old setting and use old if required + if (! settings.contains (global_icon_theme_index.key)) { - // check for new and old setting and use old if required - if (! m_settings->contains (global_icon_theme_index.key)) - { - // new pref does not exist - if (m_settings->value (global_icon_theme).toBool ()) - theme = ICON_THEME_SYSTEM; - else - theme = ICON_THEME_OCTAVE; - m_settings->setValue (global_icon_theme_index.key, theme); // add new - m_settings->remove (global_icon_theme.key); // remove deprecated key - } - else - { - // get new settings - theme = m_settings->value (global_icon_theme_index).toInt (); - } + // new pref does not exist + if (settings.value (global_icon_theme).toBool ()) + theme = ICON_THEME_SYSTEM; + else + theme = ICON_THEME_OCTAVE; + settings.setValue (global_icon_theme_index.key, theme); // add new + settings.remove (global_icon_theme.key); // remove deprecated key } QIcon::setThemeName (global_all_icon_themes.at (theme)); @@ -191,24 +180,6 @@ m_icon_fallbacks << global_icon_paths.at (ICON_THEME_CURSORS); } - gui_settings * resource_manager::get_settings (void) const - { - if (! m_settings) - { - QString msg (QT_TR_NOOP ("Octave has lost its settings.\n" - "This should not happen.\n\n" - "Please report this bug.\n\n" - "Octave GUI must be closed now.")); - - QMessageBox::critical (nullptr, - QString (QT_TR_NOOP ("Octave Critical Error")), - msg); - exit (1); - } - - return m_settings; - } - QString resource_manager::get_settings_directory (void) { return m_settings_directory; @@ -317,24 +288,24 @@ check_settings (); + gui_settings settings; + // Write some settings that were dynamically determined at first startup - if (m_settings) - { - // Custom editor - if (! custom_editor.isEmpty ()) - m_settings->setValue (global_custom_editor.key, custom_editor); + + // Custom editor + if (! custom_editor.isEmpty ()) + settings.setValue (global_custom_editor.key, custom_editor); - // Default monospace font for the terminal - if (def_font.count () > 1) - { - m_settings->setValue (cs_font.key, def_font[0]); - m_settings->setValue (cs_font_size.key, def_font[1].toInt ()); - } + // Default monospace font for the terminal + if (def_font.count () > 1) + { + settings.setValue (cs_font.key, def_font[0]); + settings.setValue (cs_font_size.key, def_font[1].toInt ()); + } - // Write the default monospace font into the settings for later use by - // console and editor as fallbacks of their font preferences. - m_settings->setValue (global_mono_font.key, get_default_font_family ()); - } + // Write the default monospace font into the settings for later use by + // console and editor as fallbacks of their font preferences. + settings.setValue (global_mono_font.key, get_default_font_family ()); } #if defined (HAVE_QSCINTILLA) @@ -366,9 +337,10 @@ #if defined (HAVE_QSCINTILLA) void resource_manager::read_lexer_settings (QsciLexer *lexer, - gui_settings *settings, int mode, int def) { + gui_settings settings; + // Test whether the settings for lexer is already contained in the // given gui settings file. If yes, load them, if not copy them from the // default settings file. @@ -380,13 +352,13 @@ QString group ("Scintilla" + settings_color_modes_ext[m]); - settings->beginGroup (group); - settings->beginGroup (lexer->language ()); + settings.beginGroup (group); + settings.beginGroup (lexer->language ()); - QStringList lexer_keys = settings->allKeys (); + QStringList lexer_keys = settings.allKeys (); - settings->endGroup (); - settings->endGroup (); + settings.endGroup (); + settings.endGroup (); if (def == settings_reload_default_colors_flag || lexer_keys.count () == 0) { @@ -408,9 +380,9 @@ for (int i = 0; i < max_style; i++) { - c = settings->get_color_value (QVariant (lexer->color (styles[i])), m); + c = settings.get_color_value (QVariant (lexer->color (styles[i])), m); lexer->setColor (c, styles[i]); - p = settings->get_color_value (QVariant (lexer->paper (styles[i])), m); + p = settings.get_color_value (QVariant (lexer->paper (styles[i])), m); lexer->setPaper (p, styles[i]); dfa = copy_font_attributes (lexer->font (styles[i]), df); lexer->setFont (dfa, styles[i]); @@ -423,31 +395,33 @@ if (def != settings_reload_default_colors_flag) { const std::string group_str = group.toStdString (); - lexer->writeSettings (*settings, group_str.c_str ()); - settings->sync (); + lexer->writeSettings (settings, group_str.c_str ()); + settings.sync (); } } else { // Found lexer keys, read the settings const std::string group_str = group.toStdString (); - lexer->readSettings (*settings, group_str.c_str ()); + lexer->readSettings (settings, group_str.c_str ()); } } #endif void resource_manager::check_settings (void) { - if (m_settings->status () == QSettings::NoError) + gui_settings settings; + + if (settings.status () == QSettings::NoError) { // Test usability (force file to be really created) - m_settings->setValue ("dummy", 0); - m_settings->sync (); + settings.setValue ("dummy", 0); + settings.sync (); } - if (! (QFile::exists (m_settings->fileName ()) - && m_settings->isWritable () - && m_settings->status () == QSettings::NoError)) + if (! (QFile::exists (settings.fileName ()) + && settings.isWritable () + && settings.status () == QSettings::NoError)) { QString msg = QString (QT_TR_NOOP ("The settings file\n%1\n" @@ -462,17 +436,19 @@ exit (1); } else - m_settings->remove ("dummy"); // Remove test entry + settings.remove ("dummy"); // Remove test entry } bool resource_manager::update_settings_key (const QString& old_key, const QString& new_key) { - if (m_settings->contains (old_key)) + gui_settings settings; + + if (settings.contains (old_key)) { - QVariant preference = m_settings->value (old_key); - m_settings->setValue (new_key, preference); - m_settings->remove (old_key); + QVariant preference = settings.value (old_key); + settings.setValue (new_key, preference); + settings.remove (old_key); return true; } @@ -486,9 +462,6 @@ void resource_manager::update_network_settings (void) { - if (! m_settings) - return; - QNetworkProxy proxy; // Assume no proxy and empty proxy data @@ -500,11 +473,13 @@ QString pass; QUrl proxy_url = QUrl (); - if (m_settings->value (global_use_proxy.key, global_use_proxy.def).toBool ()) + gui_settings settings; + + if (settings.value (global_use_proxy.key, global_use_proxy.def).toBool ()) { // Use a proxy, collect all required information QString proxy_type_string - = m_settings->value (global_proxy_type.key, global_proxy_type.def).toString (); + = settings.value (global_proxy_type.key, global_proxy_type.def).toString (); // The proxy type for the Qt proxy settings if (proxy_type_string == "Socks5Proxy") @@ -516,14 +491,14 @@ if (proxy_type_string == "HttpProxy" || proxy_type_string == "Socks5Proxy") { - host = m_settings->value (global_proxy_host.key, - global_proxy_host.def).toString (); - port = m_settings->value (global_proxy_port.key, - global_proxy_port.def).toInt (); - user = m_settings->value (global_proxy_user.key, - global_proxy_user.def).toString (); - pass = m_settings->value (global_proxy_pass.key, - global_proxy_pass.def).toString (); + host = settings.value (global_proxy_host.key, + global_proxy_host.def).toString (); + port = settings.value (global_proxy_port.key, + global_proxy_port.def).toInt (); + user = settings.value (global_proxy_user.key, + global_proxy_user.def).toString (); + pass = settings.value (global_proxy_pass.key, + global_proxy_pass.def).toString (); if (proxy_type_string == "HttpProxy") scheme = "http"; else if (proxy_type_string == "Socks5Proxy") @@ -662,7 +637,9 @@ if (enc.isEmpty ()) { - enc = m_settings->value (ed_default_enc).toString (); + gui_settings settings; + + enc = settings.value (ed_default_enc).toString (); if (enc.isEmpty ()) // still empty? {