Add API for saving settings with default value

We should never actually write default values into the settings, because

- if the default value changes in a later Qt Creator version, the new
  default should automatically take effect if the user didn't change the
  value
- it senselessly grows the settings file

Add a QtcSettings class that extends QSettings by a
"setValueWithDefault" method, which does not write default values to the
settings, and actually removes the settingskey if the user switches back
to the default.

Use it at the places where we already do this manually.

Task-number: QTCREATORBUG-24762
Change-Id: Ia76414cb21e8521f3aeed1e37b43ae4fb3393ea3
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>

src/app/main.cpp

@@ -37,6 +37,7 @@
 #include <utils/fileutils.h>
 #include <utils/hostosinfo.h>
 #include <utils/optional.h>
+#include <utils/qtcsettings.h>
 #include <utils/temporarydirectory.h>
 
 #include <QDebug>
@@ -44,7 +45,6 @@
 #include <QFontDatabase>
 #include <QFileInfo>
 #include <QLibraryInfo>
-#include <QSettings>
 #include <QStyle>
 #include <QTextStream>
 #include <QThreadPool>
@@ -274,16 +274,17 @@ static void setupInstallSettings(QString &installSettingspath)
     }
 }
 
-static QSettings *createUserSettings()
+static Utils::QtcSettings *createUserSettings()
 {
-    return new QSettings(QSettings::IniFormat, QSettings::UserScope,
+    return new Utils::QtcSettings(QSettings::IniFormat,
-                         QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR),
+                                  QSettings::UserScope,
-                         QLatin1String(Core::Constants::IDE_CASED_ID));
+                                  QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR),
+                                  QLatin1String(Core::Constants::IDE_CASED_ID));
 }
 
-static inline QSettings *userSettings()
+static inline Utils::QtcSettings *userSettings()
 {
-    QSettings *settings = createUserSettings();
+    Utils::QtcSettings *settings = createUserSettings();
     const QString fromVariant = QLatin1String(Core::Constants::IDE_COPY_SETTINGS_FROM_VARIANT_STR);
     if (fromVariant.isEmpty())
         return settings;
@@ -534,10 +535,12 @@ int main(int argc, char **argv)
 
     /*Initialize global settings and resetup install settings with QApplication::applicationDirPath */
     setupInstallSettings(options.installSettingsPath);
-    QSettings *settings = userSettings();
+    Utils::QtcSettings *settings = userSettings();
-    QSettings *globalSettings = new QSettings(QSettings::IniFormat, QSettings::SystemScope,
+    Utils::QtcSettings *globalSettings
-                                              QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR),
+        = new Utils::QtcSettings(QSettings::IniFormat,
-                                              QLatin1String(Core::Constants::IDE_CASED_ID));
+                                 QSettings::SystemScope,
+                                 QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR),
+                                 QLatin1String(Core::Constants::IDE_CASED_ID));
     loadFonts();
 
     if (Utils::HostOsInfo::isWindowsHost()

src/libs/extensionsystem/pluginmanager.cpp

@@ -43,7 +43,6 @@
 #include <QMessageBox>
 #include <QMetaProperty>
 #include <QPushButton>
-#include <QSettings>
 #include <QSysInfo>
 #include <QTextStream>
 #include <QTimer>
@@ -56,6 +55,7 @@
 #include <utils/hostosinfo.h>
 #include <utils/mimetypes/mimedatabase.h>
 #include <utils/qtcassert.h>
+#include <utils/qtcsettings.h>
 #include <utils/synchronousprocess.h>
 
 #ifdef WITH_TESTS
@@ -487,7 +487,7 @@ void PluginManager::setPluginIID(const QString &iid)
     disabled plugins.
     Needs to be set before the plugin search path is set with setPluginPaths().
 */
-void PluginManager::setSettings(QSettings *settings)
+void PluginManager::setSettings(QtcSettings *settings)
 {
     d->setSettings(settings);
 }
@@ -497,7 +497,7 @@ void PluginManager::setSettings(QSettings *settings)
     default disabled plugins.
     Needs to be set before the plugin search path is set with setPluginPaths().
 */
-void PluginManager::setGlobalSettings(QSettings *settings)
+void PluginManager::setGlobalSettings(QtcSettings *settings)
 {
     d->setGlobalSettings(settings);
 }
@@ -506,7 +506,7 @@ void PluginManager::setGlobalSettings(QSettings *settings)
     Returns the user specific settings used for information about enabled and
     disabled plugins.
 */
-QSettings *PluginManager::settings()
+QtcSettings *PluginManager::settings()
 {
     return d->settings;
 }
@@ -514,7 +514,7 @@ QSettings *PluginManager::settings()
 /*!
     Returns the global (user-independent) settings used for information about default disabled plugins.
 */
-QSettings *PluginManager::globalSettings()
+QtcSettings *PluginManager::globalSettings()
 {
     return d->globalSettings;
 }
@@ -817,7 +817,7 @@ PluginSpec *PluginManagerPrivate::createSpec()
 /*!
     \internal
 */
-void PluginManagerPrivate::setSettings(QSettings *s)
+void PluginManagerPrivate::setSettings(QtcSettings *s)
 {
     if (settings)
         delete settings;
@@ -829,7 +829,7 @@ void PluginManagerPrivate::setSettings(QSettings *s)
 /*!
     \internal
 */
-void PluginManagerPrivate::setGlobalSettings(QSettings *s)
+void PluginManagerPrivate::setGlobalSettings(QtcSettings *s)
 {
     if (globalSettings)
         delete globalSettings;

src/libs/extensionsystem/pluginmanager.h

@@ -26,14 +26,15 @@
 #pragma once
 
 #include "extensionsystem_global.h"
+
 #include <aggregation/aggregate.h>
+#include <utils/qtcsettings.h>
 
 #include <QObject>
 #include <QStringList>
 
 QT_BEGIN_NAMESPACE
 class QTextStream;
-class QSettings;
 QT_END_NAMESPACE
 
 namespace ExtensionSystem {
@@ -99,10 +100,10 @@ public:
     static void checkForProblematicPlugins();
 
     // Settings
-    static void setSettings(QSettings *settings);
+    static void setSettings(Utils::QtcSettings *settings);
-    static QSettings *settings();
+    static Utils::QtcSettings *settings();
-    static void setGlobalSettings(QSettings *settings);
+    static void setGlobalSettings(Utils::QtcSettings *settings);
-    static QSettings *globalSettings();
+    static Utils::QtcSettings *globalSettings();
     static void writeSettings();
 
     // command line arguments

src/libs/extensionsystem/pluginmanager_p.h

@@ -41,10 +41,13 @@
 QT_BEGIN_NAMESPACE
 class QTime;
 class QTimer;
-class QSettings;
 class QEventLoop;
 QT_END_NAMESPACE
 
+namespace Utils {
+class QtcSettings;
+}
+
 namespace ExtensionSystem {
 
 class PluginManager;
@@ -76,8 +79,8 @@ public:
     void initProfiling();
     void profilingSummary() const;
     void profilingReport(const char *what, const PluginSpec *spec = nullptr);
-    void setSettings(QSettings *settings);
+    void setSettings(Utils::QtcSettings *settings);
-    void setGlobalSettings(QSettings *settings);
+    void setGlobalSettings(Utils::QtcSettings *settings);
     void readSettings();
     void writeSettings();
 
@@ -124,8 +127,8 @@ public:
     QHash<const PluginSpec *, int> m_profileTotal;
     int m_profileElapsedMS = 0;
     unsigned m_profilingVerbosity = 0;
-    QSettings *settings = nullptr;
+    Utils::QtcSettings *settings = nullptr;
-    QSettings *globalSettings = nullptr;
+    Utils::QtcSettings *globalSettings = nullptr;
 
     // Look in argument descriptions of the specs for the option.
     PluginSpec *pluginForOption(const QString &option, bool *requiresArgument) const;

src/libs/utils/CMakeLists.txt

@@ -124,6 +124,7 @@ add_qtc_library(Utils
     qtcassert.cpp qtcassert.h
     qtcolorbutton.cpp qtcolorbutton.h
     qtcprocess.cpp qtcprocess.h
+    qtcsettings.cpp qtcsettings.h
     reloadpromptutils.cpp reloadpromptutils.h
     removefiledialog.cpp removefiledialog.h removefiledialog.ui
     runextensions.cpp runextensions.h

src/libs/utils/qtcsettings.cpp

@@ -0,0 +1,54 @@
+/****************************************************************************
+**
+** Copyright (C) 2020 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#include "qtcsettings.h"
+
+namespace Utils {
+
+/*!
+    \class Utils::QtcSettings
+    \inheaderfile utils/qtcsettings.h
+    \inmodule QtCreator
+
+    \brief The QtcSettings class is an extension of the QSettings class.
+
+    Use Utils::QtcSettings::setValueWithDefault() to write values with a
+    default.
+*/
+
+/*!
+    \fn template<typename T> void setValueWithDefault(const QString &key, const T &val, const T &defaultValue)
+
+    Sets the value of setting \a key to \a val. If \a val is the same as the \a
+    defaultValue, the settings key is removed instead. This makes sure that
+    settings are only written if actually necessary, namely when the user
+    changed them from the default. It also makes a new default value for a
+    setting in a new version of the application take effect, if the user did
+    not change the setting before.
+
+    \sa QSettings::setValue()
+*/
+
+} // namespace Utils

src/libs/utils/qtcsettings.h

@@ -0,0 +1,52 @@
+/****************************************************************************
+**
+** Copyright (C) 2020 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+#include "utils_global.h"
+
+#include <QSettings>
+
+namespace Utils {
+
+class QTCREATOR_UTILS_EXPORT QtcSettings : public QSettings
+{
+public:
+    using QSettings::QSettings;
+
+    template<typename T>
+    void setValueWithDefault(const QString &key, const T &val, const T &defaultValue);
+};
+
+template<typename T>
+void QtcSettings::setValueWithDefault(const QString &key, const T &val, const T &defaultValue)
+{
+    if (val == defaultValue)
+        remove(key);
+    else
+        setValue(key, QVariant::fromValue(val));
+}
+
+} // namespace Utils

src/plugins/coreplugin/generalsettings.cpp

@@ -197,25 +197,21 @@ QString GeneralSettingsWidget::language() const
 
 void GeneralSettingsWidget::setLanguage(const QString &locale)
 {
-    QSettings *settings = ICore::settings();
+    QtcSettings *settings = ICore::settings();
     if (settings->value(QLatin1String("General/OverrideLanguage")).toString() != locale) {
         RestartDialog dialog(ICore::dialogParent(),
                              tr("The language change will take effect after restart."));
         dialog.exec();
     }
 
-    if (locale.isEmpty())
+    settings->setValueWithDefault(QLatin1String("General/OverrideLanguage"), locale, {});
-        settings->remove(QLatin1String("General/OverrideLanguage"));
-    else
-        settings->setValue(QLatin1String("General/OverrideLanguage"), locale);
 }
 
 void GeneralSettings::setShowShortcutsInContextMenu(bool show)
 {
-    if (show == m_defaultShowShortcutsInContextMenu)
+    ICore::settings()->setValueWithDefault(settingsKeyShortcutsInContextMenu,
-        ICore::settings()->remove(settingsKeyShortcutsInContextMenu);
+                                           show,
-    else
+                                           m_defaultShowShortcutsInContextMenu);
-        ICore::settings()->setValue(settingsKeyShortcutsInContextMenu, show);
 #if (QT_VERSION >= QT_VERSION_CHECK(5, 13, 0))
     QGuiApplication::styleHints()->setShowShortcutsInContextMenus(show);
 #endif

src/plugins/coreplugin/icore.cpp

@@ -343,7 +343,7 @@ bool ICore::showWarningWithOptions(const QString &title, const QString &text,
 
     \sa settingsDatabase()
 */
-QSettings *ICore::settings(QSettings::Scope scope)
+QtcSettings *ICore::settings(QSettings::Scope scope)
 {
     if (scope == QSettings::UserScope)
         return PluginManager::settings();

src/plugins/coreplugin/icore.h

@@ -28,6 +28,8 @@
 #include "core_global.h"
 #include "icontext.h"
 
+#include <utils/qtcsettings.h>
+
 #include <QList>
 #include <QMainWindow>
 #include <QObject>
@@ -87,7 +89,7 @@ public:
                                        Utils::Id settingsId = {},
                                        QWidget *parent = nullptr);
 
-    static QSettings *settings(QSettings::Scope scope = QSettings::UserScope);
+    static Utils::QtcSettings *settings(QSettings::Scope scope = QSettings::UserScope);
     static SettingsDatabase *settingsDatabase();
     static QPrinter *printer();
     static QString userInterfaceLanguage();

src/plugins/help/localhelpmanager.cpp

@@ -110,16 +110,6 @@ static QString defaultFallbackFontStyleName(const QString &fontFamily)
     return styles.first();
 }
 
-template <typename T>
-static void setOrRemoveSetting(const char *key, const T &value, const T &defaultValue)
-{
-    QSettings *settings = Core::ICore::settings();
-    if (value == defaultValue)
-        settings->remove(QLatin1String(key));
-    else
-        settings->setValue(QLatin1String(key), value);
-}
-
 LocalHelpManager::LocalHelpManager(QObject *parent)
     : QObject(parent)
 {
@@ -179,9 +169,15 @@ QFont LocalHelpManager::fallbackFont()
 
 void LocalHelpManager::setFallbackFont(const QFont &font)
 {
-    setOrRemoveSetting(kFontFamilyKey, font.family(), defaultFallbackFontFamily());
+    Core::ICore::settings()->setValueWithDefault(kFontFamilyKey,
-    setOrRemoveSetting(kFontStyleNameKey, font.styleName(), defaultFallbackFontStyleName(font.family()));
+                                                 font.family(),
-    setOrRemoveSetting(kFontSizeKey, font.pointSize(), kDefaultFallbackFontSize);
+                                                 defaultFallbackFontFamily());
+    Core::ICore::settings()->setValueWithDefault(kFontStyleNameKey,
+                                                 font.styleName(),
+                                                 defaultFallbackFontStyleName(font.family()));
+    Core::ICore::settings()->setValueWithDefault(kFontSizeKey,
+                                                 font.pointSize(),
+                                                 kDefaultFallbackFontSize);
     emit m_instance->fallbackFontChanged(font);
 }
 
@@ -369,10 +365,7 @@ HelpViewerFactory LocalHelpManager::viewerBackend()
 
 void LocalHelpManager::setViewerBackendId(const QByteArray &id)
 {
-    if (id.isEmpty())
+    Core::ICore::settings()->setValueWithDefault(kViewerBackend, id, {});
-        Core::ICore::settings()->remove(kViewerBackend);
-    else
-        Core::ICore::settings()->setValue(kViewerBackend, id);
 }
 
 QByteArray LocalHelpManager::viewerBackendId()

src/plugins/mcusupport/mcusupportoptions.cpp

@@ -210,11 +210,7 @@ void McuPackage::writeToSettings() const
 {
     const QString key = QLatin1String(Constants::SETTINGS_GROUP) + '/' +
             QLatin1String(Constants::SETTINGS_KEY_PACKAGE_PREFIX) + m_settingsKey;
-    QSettings *settings = Core::ICore::settings();
+    Core::ICore::settings()->setValueWithDefault(key, m_path, m_defaultPath);
-    if (m_path == m_defaultPath)
-        settings->remove(key);
-    else
-        settings->setValue(key, m_path);
 }
 
 void McuPackage::setRelativePathModifier(const QString &path)

src/plugins/vcsbase/commonvcssettings.cpp

@@ -62,7 +62,7 @@ CommonVcsSettings::CommonVcsSettings() :
 {
 }
 
-void CommonVcsSettings::toSettings(QSettings *s) const
+void CommonVcsSettings::toSettings(Utils::QtcSettings *s) const
 {
     s->beginGroup(QLatin1String(settingsGroupC));
     s->setValue(QLatin1String(nickNameMailMapKeyC), nickNameMailMap);
@@ -70,11 +70,9 @@ void CommonVcsSettings::toSettings(QSettings *s) const
     s->setValue(QLatin1String(submitMessageCheckScriptKeyC), submitMessageCheckScript);
     s->setValue(QLatin1String(lineWrapKeyC), lineWrap);
     s->setValue(QLatin1String(lineWrapWidthKeyC), lineWrapWidth);
-    // Do not store the default setting to avoid clobbering the environment.
+    s->setValueWithDefault(QLatin1String(sshPasswordPromptKeyC),
-    if (sshPasswordPrompt != sshPasswordPromptDefault())
+                           sshPasswordPrompt,
-        s->setValue(QLatin1String(sshPasswordPromptKeyC), sshPasswordPrompt);
+                           sshPasswordPromptDefault());
-    else
-        s->remove(QLatin1String(sshPasswordPromptKeyC));
     s->endGroup();
 }
 

src/plugins/vcsbase/commonvcssettings.h

@@ -25,10 +25,11 @@
 
 #pragma once
 
+#include <utils/qtcsettings.h>
+
 #include <QString>
 
 QT_BEGIN_NAMESPACE
-class QSettings;
 class QDebug;
 QT_END_NAMESPACE
 
@@ -52,7 +53,7 @@ public:
     bool lineWrap;
     int lineWrapWidth;
 
-    void toSettings(QSettings *) const;
+    void toSettings(Utils::QtcSettings *) const;
     void fromSettings(QSettings *);
 
     bool equals(const CommonVcsSettings &rhs) const;

tests/auto/extensionsystem/pluginmanager/tst_pluginmanager.cpp

@@ -77,7 +77,7 @@ static QString pluginFolder(const QLatin1String &folder)
 void tst_PluginManager::init()
 {
     m_pm = new PluginManager;
-    PluginManager::setSettings(new QSettings);
+    PluginManager::setSettings(new Utils::QtcSettings);
     PluginManager::setPluginIID(QLatin1String("plugin"));
     m_objectAdded = new QSignalSpy(m_pm, SIGNAL(objectAdded(QObject*)));
     m_aboutToRemoveObject = new QSignalSpy(m_pm, SIGNAL(aboutToRemoveObject(QObject*)));