changeset 27601:7a748f7545c9

improve thread safety of qt_interpreter_events::gui_preference * main-window.h, main-window.cc (main_window::gui_preference, main_window::gui_preference_adjust): Delete. (main_window::construct_octave_qt_link): Connect qt_link settings_changed signal to main_window notice_settings slot. Don't connect qt_link gui_preference_signal to main_window gui_preference slot. * qt-interpreter-events.h, qt-interpreter-events.cc (qt_interpreter_events::gui_preference_slot, qt_interpreter_events::gui_preference_adjust): New functions, imported from main_window class, but without the PREF_VALUE argument. Use m_result to capture value from slot function. (qt_interpreter_events::qt_interpreter_events): Connect gui_preference_signal to gui_preference_slot. (qt_interpreter_events::settings_changed): New signal.
author John W. Eaton <jwe@octave.org>
date Thu, 31 Oct 2019 14:36:11 -0400
parents bb9aecedc167
children ba317c535adb
files libgui/src/main-window.cc libgui/src/main-window.h libgui/src/qt-interpreter-events.cc libgui/src/qt-interpreter-events.h
diffstat 4 files changed, 104 insertions(+), 79 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/src/main-window.cc	Thu Oct 31 11:40:30 2019 -0400
+++ b/libgui/src/main-window.cc	Thu Oct 31 14:36:11 2019 -0400
@@ -2129,6 +2129,9 @@
 
     qt_interpreter_events *qt_link = interp_qobj->qt_link ();
 
+    connect (qt_link, SIGNAL (settings_changed (const QSettings *)),
+             this, SLOT (notice_settings (const QSettings *)));
+
     connect (qt_link,
              SIGNAL (set_workspace_signal (bool, bool, const symbol_info_list&)),
              m_workspace_model,
@@ -2173,12 +2176,6 @@
              this, SLOT (process_settings_dialog_request (void)));
 
     connect (qt_link,
-             SIGNAL (gui_preference_signal (const QString&, const QString&,
-                                            QString*)),
-             this, SLOT (gui_preference (const QString&, const QString&,
-                                         QString*)));
-
-    connect (qt_link,
              SIGNAL (edit_file_signal (const QString&)),
              m_active_editor,
              SLOT (handle_edit_file_request (const QString&)));
@@ -2697,72 +2694,6 @@
              this, SLOT (handle_undo_request (void)));
   }
 
-  QString main_window::gui_preference_adjust (const QString& key,
-                                              const QString& value)
-  {
-    QString adjusted_value = value;
-
-    // Immediately return if no new value is given
-    if (adjusted_value.isEmpty ())
-      return adjusted_value;
-
-    // Not all encodings are available. Encodings are uppercase and do not
-    // use CPxxx but IBMxxx or WINDOWS-xxx.
-    if (key == ed_default_enc.key)
-      {
-        adjusted_value = adjusted_value.toUpper ();
-
-        QStringList codecs;
-        resource_manager::get_codecs (&codecs);
-
-        QRegExp re ("^CP(\\d+)$");
-        if (adjusted_value == "SYSTEM")
-          adjusted_value =
-            QTextCodec::codecForLocale ()->name ().toUpper ().prepend ("SYSTEM (").append (")");
-        else if (re.indexIn (adjusted_value) > -1)
-          {
-            if (codecs.contains ("IBM" + re.cap (1)))
-              adjusted_value = "IBM" + re.cap (1);
-            else if (codecs.contains ("WINDOWS-" + re.cap (1)))
-              adjusted_value = "WINDOWS-" + re.cap (1);
-            else
-              adjusted_value.clear ();
-          }
-        else if (! codecs.contains (adjusted_value))
-          adjusted_value.clear ();
-      }
-
-    return adjusted_value;
-  }
-
-  void main_window::gui_preference (const QString& key, const QString& value,
-                                    QString* read_value)
-  {
-    QSettings *settings = resource_manager::get_settings ();
-    *read_value = settings->value (key).toString ();
-
-    interpreter_qobject *interp_qobj = m_octave_qobj.interpreter_qobj ();
-
-    qt_interpreter_events *qt_link = interp_qobj->qt_link ();
-
-    // Wait for worker to suspend
-    qt_link->lock ();
-
-    // Some preferences need extra handling
-    QString adjusted_value = gui_preference_adjust (key, value);
-
-    if (! adjusted_value.isEmpty () && (*read_value != adjusted_value))
-      {
-        // Change settings only for new, non-empty values
-        settings->setValue (key, QVariant (adjusted_value));
-        emit settings_changed (settings);
-      }
-
-    // We are done: Unlock and wake the worker thread
-    qt_link->unlock ();
-    qt_link->wake_all ();
-  }
-
   void main_window::focus_console_after_command (void)
   {
     QSettings *settings = resource_manager::get_settings ();
--- a/libgui/src/main-window.h	Thu Oct 31 11:40:30 2019 -0400
+++ b/libgui/src/main-window.h	Thu Oct 31 14:36:11 2019 -0400
@@ -198,8 +198,6 @@
     void pasteClipboard (void);
     void selectAll (void);
 
-    void gui_preference (const QString& key, const QString& value,
-                         QString* read_value);
     void focus_console_after_command (void);
     void handle_show_doc (const QString& file);
     void handle_register_doc (const QString& file);
@@ -283,8 +281,6 @@
 
     void construct_tool_bar (void);
 
-    QString gui_preference_adjust (const QString& key, const QString& value);
-
     void configure_shortcuts (void);
 
     QList<octave_dock_widget *> dock_widget_list (void);
--- a/libgui/src/qt-interpreter-events.cc	Thu Oct 31 11:40:30 2019 -0400
+++ b/libgui/src/qt-interpreter-events.cc	Thu Oct 31 14:36:11 2019 -0400
@@ -34,6 +34,7 @@
 #include <QStringList>
 
 #include "dialog.h"
+#include "gui-preferences-ed.h"
 #include "octave-qobject.h"
 #include "qt-interpreter-events.h"
 #include "resource-manager.h"
@@ -120,6 +121,11 @@
 
     connect (this, SIGNAL (get_named_icon_signal (const QString&)),
              this, SLOT (get_named_icon_slot (const QString&)));
+
+    connect (this,
+             SIGNAL (gui_preference_signal (const QString&, const QString&)),
+             this,
+             SLOT (gui_preference_slot (const QString&, const QString&)));
   }
 
   std::list<std::string>
@@ -394,12 +400,14 @@
 
     // Emit the signal for changing or getting a preference
     emit gui_preference_signal (QString::fromStdString (key),
-                                QString::fromStdString (value), &pref_value);
+                                QString::fromStdString (value));
 
     // Wait for response (pref_value).
     wait ();
 
-    return pref_value.toStdString ();
+    QString pref = m_result.toString ();
+
+    return pref.toStdString ();
   }
 
   bool qt_interpreter_events::copy_image_to_clipboard (const std::string& file)
@@ -552,4 +560,87 @@
 
     wake_all ();
   }
+
+  // If VALUE is empty, return current value of preference named by KEY.
+  //
+  // If VALUE is not empty, set preference named by KEY to VALUE return
+  // previous value.
+  //
+  // FIXME: should we have separate get and set functions?  With only
+  // one, we don't allow a preference value to be set to the empty
+  // string.
+
+  void
+  qt_interpreter_events::gui_preference_slot (const QString& key,
+                                              const QString& value)
+  {
+    // Wait for worker to suspend
+    lock ();
+
+    QSettings *settings = resource_manager::get_settings ();
+
+    QString read_value = settings->value (key).toString ();
+
+    // Some preferences need extra handling
+    QString adjusted_value = gui_preference_adjust (key, value);
+
+    if (! adjusted_value.isEmpty () && (read_value != adjusted_value))
+      {
+        // Change settings only for new, non-empty values
+        settings->setValue (key, QVariant (adjusted_value));
+
+        emit settings_changed (settings);
+      }
+
+    m_result = read_value;
+
+    // We are done: Unlock and wake the worker thread
+    unlock ();
+
+    wake_all ();
+  }
+
+  QString
+  qt_interpreter_events::gui_preference_adjust (const QString& key,
+                                                const QString& value)
+  {
+    // Immediately return if no new value is given.
+
+    if (value.isEmpty ())
+      return value;
+
+    QString adjusted_value = value;
+
+    // Not all encodings are available. Encodings are uppercase and do
+    // not use CPxxx but IBMxxx or WINDOWS-xxx.
+
+    if (key == ed_default_enc.key)
+      {
+        adjusted_value = adjusted_value.toUpper ();
+
+        QStringList codecs;
+        resource_manager::get_codecs (&codecs);
+
+        QRegExp re ("^CP(\\d+)$");
+
+        if (adjusted_value == "SYSTEM")
+          adjusted_value =
+            QTextCodec::codecForLocale ()->name ().toUpper ().prepend ("SYSTEM (").append (")");
+        else if (re.indexIn (adjusted_value) > -1)
+          {
+            if (codecs.contains ("IBM" + re.cap (1)))
+              adjusted_value = "IBM" + re.cap (1);
+            else if (codecs.contains ("WINDOWS-" + re.cap (1)))
+              adjusted_value = "WINDOWS-" + re.cap (1);
+            else
+              adjusted_value.clear ();
+          }
+        else if (! codecs.contains (adjusted_value))
+          adjusted_value.clear ();
+      }
+
+    return adjusted_value;
+  }
+
+  
 }
--- a/libgui/src/qt-interpreter-events.h	Thu Oct 31 11:40:30 2019 -0400
+++ b/libgui/src/qt-interpreter-events.h	Thu Oct 31 14:36:11 2019 -0400
@@ -31,6 +31,7 @@
 #include <QList>
 #include <QMutex>
 #include <QObject>
+#include <QSettings>
 #include <QString>
 #include <QWaitCondition>
 
@@ -182,6 +183,8 @@
 
     void get_named_icon_slot (const QString& name);
 
+    void gui_preference_slot (const QString& key, const QString& value);
+
   signals:
 
     void copy_image_to_clipboard_signal (const QString& file, bool remove_file);
@@ -222,7 +225,7 @@
 
     void show_preferences_signal (void);
 
-    void gui_preference_signal (const QString&, const QString&, QString*);
+    void gui_preference_signal (const QString& key, const QString& value);
 
     void show_doc_signal (const QString& file);
 
@@ -238,8 +241,12 @@
 
     void get_named_icon_signal (const QString& name);
 
+    void settings_changed (const QSettings *);
+
   private:
 
+    QString gui_preference_adjust (const QString& key, const QString& value);
+
     void insert_debugger_pointer (const std::string& file, int line);
 
     void delete_debugger_pointer (const std::string& file, int line);