Merge pull request #23273 from mantidproject/workbench_use_config

Workbench use QSettings configuration

qt/applications/workbench/workbench/app/mainwindow.py

@@ -44,7 +44,7 @@ requirements.check_qt()
 # -----------------------------------------------------------------------------
 # Qt
 # -----------------------------------------------------------------------------
-from qtpy.QtCore import (QEventLoop, Qt, QCoreApplication)  # noqa
+from qtpy.QtCore import (QEventLoop, Qt, QCoreApplication, QSettings, QPoint, QSize)  # noqa
 from qtpy.QtGui import (QColor, QPixmap)  # noqa
 from qtpy.QtWidgets import (QApplication, QDesktopWidget, QFileDialog,
                             QMainWindow, QSplashScreen)  # noqa
@@ -53,6 +53,8 @@ from mantidqt.utils.qt import plugins, widget_updates_disabled  # noqa
 # Pre-application setup
 plugins.setup_library_paths()
 
+from workbench.config import APPNAME, CONF, ORG_DOMAIN, ORGANIZATION  # noqa
+
 
 # -----------------------------------------------------------------------------
 # Create the application instance early, set the application name for window
@@ -67,7 +69,14 @@ def qapplication():
     app = QApplication.instance()
     if app is None:
         QCoreApplication.setAttribute(Qt.AA_ShareOpenGLContexts)
-        app = QApplication(['Mantid Workbench'])
+        argv = sys.argv[:]
+        argv[0] = APPNAME # replace application name
+        app = QApplication(argv)
+        app.setOrganizationName(ORGANIZATION)
+        app.setOrganizationDomain(ORG_DOMAIN)
+        app.setApplicationName(APPNAME)
+        # not calling app.setApplicationVersion(mantid.kernel.version_str())
+        # because it needs to happen after logging is monkey-patched in
     return app
 
 
@@ -107,17 +116,12 @@ class MainWindow(QMainWindow):
     def __init__(self):
         QMainWindow.__init__(self)
 
-        qapp = QApplication.instance()
-        qapp.setAttribute(Qt.AA_UseHighDpiPixmaps)
-        if hasattr(Qt, 'AA_EnableHighDpiScaling'):
-            qapp.setAttribute(Qt.AA_EnableHighDpiScaling, True)
-
+        # -- instance attributes --
         self.setWindowTitle("Mantid Workbench")
 
-        # -- instance attributes --
-        self.window_size = None
-        self.window_position = None
-        self.maximized_flag = None
+        # uses default configuration as necessary
+        self.readSettings(CONF)
+
         # widgets
         self.messagedisplay = None
         self.ipythonconsole = None
@@ -258,14 +262,8 @@ class MainWindow(QMainWindow):
     # ----------------------- Layout ---------------------------------
 
     def setup_layout(self):
-        self.setup_for_first_run()
-
-    def setup_for_first_run(self):
         """Assume this is a first run of the application and set layouts
         accordingly"""
-        self.setWindowState(Qt.WindowMaximized)
-        desktop = QDesktopWidget()
-        self.window_size = desktop.screenGeometry().size()
         self.setup_default_layouts()
 
     def prep_window_for_reset(self):
@@ -335,6 +333,8 @@ class MainWindow(QMainWindow):
     def closeEvent(self, event):
         # Close editors
         if self.editor.app_closing():
+            self.writeSettings(CONF) # write current window information to global settings object
+
             # Close all open plots
             # We don't want this at module scope here
             import matplotlib.pyplot as plt  #noqa
@@ -361,6 +361,46 @@ class MainWindow(QMainWindow):
     def open_manage_directories(self):
         ManageUserDirectories(self).exec_()
 
+    def readSettings(self, settings):
+        qapp = QApplication.instance()
+        qapp.setAttribute(Qt.AA_UseHighDpiPixmaps)
+        if hasattr(Qt, 'AA_EnableHighDpiScaling'):
+            qapp.setAttribute(Qt.AA_EnableHighDpiScaling, settings.get('main/high_dpi_scaling'))
+
+        # get the saved window geometry
+        window_size = settings.get('main/window/size')
+        if not isinstance(window_size, QSize):
+            window_size = QSize(*window_size)
+        window_pos = settings.get('main/window/position')
+        if not isinstance(window_pos, QPoint):
+            window_pos = QPoint(*window_pos)
+
+        # make sure main window is smaller than the desktop
+        desktop_size = QDesktopWidget().screenGeometry().size()
+        w = min(desktop_size.width(), window_size.width())
+        h = min(desktop_size.height(), window_size.height())
+        window_size = QSize(w, h)
+
+        # and that it will be painted on screen
+        x = min(window_pos.x(), desktop_size.width() - window_size.width())
+        y = min(window_pos.y(), desktop_size.height() - window_size.height())
+        window_pos = QPoint(x, y)
+
+        # set the geometry
+        self.resize(window_size)
+        self.move(window_pos)
+
+        # restore window state
+        if settings.has('main/window/state'):
+            self.restoreState(settings.get('main/window/state'))
+        else:
+            self.setWindowState(Qt.WindowMaximized)
+
+    def writeSettings(self, settings):
+        settings.set('main/window/size', self.size()) # QSize
+        settings.set('main/window/position', self.pos()) # QPoint
+        settings.set('main/window/state', self.saveState()) # QByteArray
+
 
 def initialize():
     """Perform an initialization of the application instance. Most notably
@@ -402,12 +442,11 @@ def start_workbench(app):
     importlib.import_module('mantid')
 
     main_window.show()
+
     if main_window.splash:
         main_window.splash.hide()
     # lift-off!
-    app.exec_()
-
-    return main_window
+    return app.exec_()
 
 
 def main():
@@ -429,9 +468,9 @@ def main():
     # the default sys check interval leads to long lags
     # when request scripts to be aborted
     sys.setcheckinterval(SYSCHECK_INTERVAL)
-    main_window = None
+    exit_value = 0
     try:
-        main_window = start_workbench(app)
+        exit_value = start_workbench(app)
     except BaseException:
         # We count this as a crash
         import traceback
@@ -439,12 +478,9 @@ def main():
         # about. Prints to stderr as we can't really count on anything
         # else
         traceback.print_exc(file=ORIGINAL_STDERR)
-
-    if main_window is None:
-        # An exception occurred don't exit here
-        return
-
-    ORIGINAL_SYS_EXIT()
+        exit_value = -1
+    finally:
+        ORIGINAL_SYS_EXIT(exit_value)
 
 
 if __name__ == '__main__':

qt/applications/workbench/workbench/config/__init__.py

@@ -14,3 +14,37 @@
 #
 #  You should have received a copy of the GNU General Public License
 #  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+""" Main configuration module.
+
+A singleton instance called CONF is defined. Modules wishing to access the settings
+should import the CONF object as
+
+    from workbench.config import CONF
+
+and use it to access the settings
+"""
+from __future__ import (absolute_import, unicode_literals)
+
+from workbench.config.user import UserConfig
+
+# -----------------------------------------------------------------------------
+# Constants
+# -----------------------------------------------------------------------------
+ORGANIZATION = 'mantidproject'
+ORG_DOMAIN = 'mantidproject.org'
+APPNAME = 'mantidworkbench'
+
+# Iterable containing defaults for each configurable section of the code
+# General application settings are in the main section
+DEFAULTS = {
+    'main': {
+      'high_dpi_scaling': True,
+      'window/size': (1260, 740),
+      'window/position': (10, 10),
+    }
+}
+
+# -----------------------------------------------------------------------------
+# 'Singleton' instance
+# -----------------------------------------------------------------------------
+CONF = UserConfig(ORGANIZATION, APPNAME, defaults=DEFAULTS)

qt/applications/workbench/workbench/config/main.py

-#  This file is part of the mantid workbench.
-#
-#  Copyright (C) 2017 mantidproject
-#
-#  This program is free software: you can redistribute it and/or modify
-#  it under the terms of the GNU General Public License as published by
-#  the Free Software Foundation, either version 3 of the License, or
-#  (at your option) any later version.
-#
-#  This program is distributed in the hope that it will be useful,
-#  but WITHOUT ANY WARRANTY; without even the implied warranty of
-#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#  GNU General Public License for more details.
-#
-#  You should have received a copy of the GNU General Public License
-#  along with this program.  If not, see <http://www.gnu.org/licenses/>.
-""" Main configuration module.
-
-A singleton instance called CONF is defined. Modules wishing to access the settings
-should import the CONF object as
-
-    from workbench.config.main import CONF
-
-and use it to access the settings
-"""
-from __future__ import (absolute_import, unicode_literals)
-
-from workbench.config.user import UserConfig
-
-# -----------------------------------------------------------------------------
-# Constants
-# -----------------------------------------------------------------------------
-ORGANIZATION = 'mantidproject'
-APPNAME = 'workbench'
-
-# Iterable containing defaults for each configurable section of the code
-# General application settings are in the main section
-DEFAULTS = {
-    'main': {
-      'high_dpi_scaling': True,
-      'window/size': (1260, 740),
-      'window/position': (10, 10),
-      'window/is_maximized': True,
-      'window/is_fullscreen': False,
-    }
-}
-
-# -----------------------------------------------------------------------------
-# 'Singleton' instance
-# -----------------------------------------------------------------------------
-CONF = UserConfig(ORGANIZATION, APPNAME, defaults=DEFAULTS)

qt/applications/workbench/workbench/config/test/test_user.py

@@ -30,7 +30,8 @@ class ConfigUserTest(TestCase):
             defaults = {
                 'main': {
                     'a_default_key': 100,
-                    'bool_option': False
+                    'bool_option': False,
+                    'bool_option2': True
                 },
             }
             cls.cfg = UserConfig(cls.__name__, cls.__name__, defaults)
@@ -53,28 +54,45 @@ class ConfigUserTest(TestCase):
 
     def test_value_not_in_settings_retrieves_default_if_it_exists(self):
         self.assertEqual(100, self.cfg.get('main', 'a_default_key'))
+        self.assertEqual(100, self.cfg.get('main/a_default_key'))
 
     def test_boolean_with_default_false_can_be_retrieved(self):
         self.assertEqual(False, self.cfg.get('main', 'bool_option'))
+        self.assertEqual(False, self.cfg.get('main/bool_option'))
+
+        self.assertEqual(True, self.cfg.get('main/bool_option2'))
+
+    def test_has_keys(self):
+        self.assertTrue(self.cfg.has('main', 'a_default_key'))
+        self.assertTrue(self.cfg.has('main/a_default_key'))
+        self.assertFalse(self.cfg.has('main', 'missing-key'))
+        self.assertFalse(self.cfg.has('main/missing-key'))
+
+    def test_remove_key(self):
+        self.cfg.set('main', 'key1', 1)
+        self.assertTrue(self.cfg.has('main/key1'))
+        self.cfg.remove('main/key1')
+        self.assertFalse(self.cfg.has('main/key1'))
 
     # ----------------------------------------------
     # Failure tests
     # ----------------------------------------------
 
     def test_get_raises_error_with_invalid_section_type(self):
-        self.assertRaises(RuntimeError, self.cfg.get, 1, 'key1')
+        self.assertRaises(TypeError, self.cfg.get, 1, 'key1')
 
     def test_get_raises_error_with_invalid_option_type(self):
-        self.assertRaises(RuntimeError, self.cfg.get, 'section', 1)
+        self.assertRaises(TypeError, self.cfg.get, 'section', 1)
 
     def test_get_raises_keyerror_with_no_saved_setting_or_default(self):
         self.assertRaises(KeyError, self.cfg.get, 'main', 'missing-key')
+        self.assertRaises(KeyError, self.cfg.get, 'main/missing-key')
 
     def test_set_raises_error_with_invalid_section_type(self):
-        self.assertRaises(RuntimeError, self.cfg.set, 1, 'key1', 1)
+        self.assertRaises(TypeError, self.cfg.set, 1, 'key1', 1)
 
     def test_set_raises_error_with_invalid_option_type(self):
-        self.assertRaises(RuntimeError, self.cfg.set, 'section', 1, 1)
+        self.assertRaises(TypeError, self.cfg.set, 'section', 1, 1)
 
 
 if __name__ == '__main__':

qt/applications/workbench/workbench/config/user.py

@@ -18,7 +18,7 @@ from __future__ import (absolute_import, division, print_function,
                         unicode_literals)
 
 from mantidqt.py3compat import is_text_string
-
+from posixpath import join as joinsettings
 from qtpy.QtCore import QSettings
 
 
@@ -32,11 +32,9 @@ class UserConfig(object):
 
     # The raw QSettings instance
     qsettings = None
-    defaults = None
 
     def __init__(self, organization, application, defaults=None):
         """
-
         :param organization: A string name for the organization
         :param application: A string name for the application name
         :param defaults: Default configuration values for this instance in the
@@ -45,77 +43,113 @@ class UserConfig(object):
         # Loads the saved settings if found
         self.qsettings = QSettings(QSettings.IniFormat, QSettings.UserScope,
                                    organization, application)
-        self.defaults = defaults
 
-    def all_keys(self):
-        return self.qsettings.allKeys()
+        # convert the defaults into something that qsettings can handle
+        default_settings = self._flatten_defaults(defaults)
+
+        # put defaults into qsettings if they weren't there already
+        configFileKeys = self.qsettings.allKeys()
+        for key in default_settings.keys():
+            if key not in configFileKeys:
+                self.qsettings.setValue(key, default_settings[key])
+
+        # fixup the values of booleans - they do not evaluate correctly when read from the config file
+        # TODO come up with a unit test for this
+        for key in self.all_keys():
+            value = self.get(key)
+            if value == 'true':
+                self.set(key, True)
+            elif value == 'false':
+                self.set(key, False)
+
+    def all_keys(self, group=None):
+        if group is not None:
+            self.qsettings.beginGroup(group)
+            result = self.qsettings.allKeys()
+            self.qsettings.endGroup()
+        else:
+            result = self.qsettings.allKeys()
+
+        return result
 
     @property
     def filename(self):
         return self.qsettings.fileName()
 
-    def get(self, section, option):
-        """
-        Return a value for an option in a given section. If not
-        specified in the saved settings then the initial
-        defaults are consulted. If no option is found then
+    def get(self, option, second=None):
+        """Return a value for an option. If two arguments are given the first
+        is the group/section and the second is the option within it.
+        ``config.get('main', 'window/size')`` is equivalent to
+        ``config.get('main/window/size')`` If no option is found then
         a KeyError is raised
-        :param section: A string section name
-        :param option: A string option name
-        :return: The value of the option
         """
-        value = self.qsettings.value(self._settings_path(section, option))
-        if not value:
-            value = self._get_default_or_raise(section, option)
-
-        return value
-
-    def set(self, section, option, value):
+        option = self._check_section_option_is_valid(option, second)
+        value = self.qsettings.value(option)
+
+        # qsettings appears to return None if the option isn't found
+        if value is None:
+            raise KeyError('Unknown config item requested: "{}"'.format(option))
+        else:
+            return value
+
+    def has(self, option, second=None):
+        """Return a True if the key exists in the
+        settings. ``config.get('main', 'window/size')`` and
+        ``config.get('main/window/size')`` are equivalent.
+        """
+        option = self._check_section_option_is_valid(option, second)
+        return option in self.all_keys()
+
+    def set(self, option, value, extra=None):
+        """Set a value for an option in a given section. Can either supply
+        the fully qualified option or add the section as an additional
+        first argument. ``config.set('main', 'high_dpi_scaling',
+        True)`` is equivalent to ``config.set('main/high_dpi_scaling',
+        True)``
         """
-        Set a value for an option in a given section.
-        :param section: A string section name
-        :param option: A string option name
-        :param value: The value of the setting
+        if extra is None:
+            option = self._check_section_option_is_valid(option, extra)
+            # value is in the right place
+        else:
+            option = self._check_section_option_is_valid(option, value)
+            value = extra
+        self.qsettings.setValue(option, value)
+
+    def remove(self, option, second=None):
+        """Removes a key from the settings. Key not existing returns without effect.
         """
-        self.qsettings.setValue(self._settings_path(section, option), value)
+        option = self._check_section_option_is_valid(option, second)
+        if self.has(option):
+            self.qsettings.remove(option)
 
     # -------------------------------------------------------------------------
     # "Private" methods
     # -------------------------------------------------------------------------
 
-    def _check_section_option_is_valid(self, section, option):
-        """
-        Sanity check the section and option are strings
-        """
-        if not is_text_string(section):
-            raise RuntimeError("section is not a text string")
-        if not is_text_string(option):
-            raise RuntimeError("option is not a text string")
-
-    def _get_default_or_raise(self, section, option):
-        """
-        Returns the value listed in the defaults if it exists
-        :param section: A string denoting the section (not checked)
-        :param option: A string denoting the option name
-        :return: The value of the default
-        :raises KeyError: if the item does not exist
-        """
-        value = None
-        if self.defaults and section in self.defaults:
-            try:
-                value = self.defaults[section][option]
-            except KeyError:
-                raise KeyError("Unknown config item requested: " +
-                               self._settings_path(section, option))
-        return value
-
-    def _settings_path(self, section, option):
+    @staticmethod
+    def _flatten_defaults(input_dict):
+        result = {}
+        for key in input_dict:
+            value = input_dict[key]
+            if isinstance(value, dict):
+                value = UserConfig._flatten_defaults(value)
+                for key_inner in value.keys():
+                    result[joinsettings(key, key_inner)] = value[key_inner]
+            else:
+                result[key] = value
+        return result
+
+    def _check_section_option_is_valid(self, option, second):
         """
-        Private method to construct a path to the given option with the
-        section
-        :param section: The name of the section
-        :param option: The name of the option
-        :return: A path to the location within the QSettings instance
+        Sanity check the section and option are strings and return the flattened option key
         """
-        self._check_section_option_is_valid(section, option)
-        return section + "/" + option
+        if second is None:
+            if not is_text_string(option):
+                raise TypeError('Found invalid type ({}) for option ({}) must be a string'.format(type(option), option))
+            return option
+        else: # fist argument is actually the section/group
+            if not is_text_string(option):
+                raise TypeError('Found invalid type ({}) for section ({}) must be a string'.format(type(option), option))
+            if not is_text_string(second):
+                raise TypeError('Found invalid type ({}) for option ({}) must be a string'.format(type(second), second))
+            return joinsettings(option, second)