From 41b73594ad749ff167ce78d1dfc91b5bc4f3c525 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Thu, 19 Nov 2020 15:34:59 +0100 Subject: [PATCH] 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 --- src/app/main.cpp | 25 +++++---- src/libs/extensionsystem/pluginmanager.cpp | 14 ++--- src/libs/extensionsystem/pluginmanager.h | 11 ++-- src/libs/extensionsystem/pluginmanager_p.h | 13 +++-- src/libs/utils/CMakeLists.txt | 1 + src/libs/utils/qtcsettings.cpp | 54 +++++++++++++++++++ src/libs/utils/qtcsettings.h | 52 ++++++++++++++++++ src/plugins/coreplugin/generalsettings.cpp | 14 ++--- src/plugins/coreplugin/icore.cpp | 2 +- src/plugins/coreplugin/icore.h | 4 +- src/plugins/help/localhelpmanager.cpp | 27 ++++------ src/plugins/mcusupport/mcusupportoptions.cpp | 6 +-- src/plugins/vcsbase/commonvcssettings.cpp | 10 ++-- src/plugins/vcsbase/commonvcssettings.h | 5 +- .../pluginmanager/tst_pluginmanager.cpp | 2 +- 15 files changed, 170 insertions(+), 70 deletions(-) create mode 100644 src/libs/utils/qtcsettings.cpp create mode 100644 src/libs/utils/qtcsettings.h diff --git a/src/app/main.cpp b/src/app/main.cpp index 9f540e7279d..52c176c0572 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -44,7 +45,6 @@ #include #include #include -#include #include #include #include @@ -274,16 +274,17 @@ static void setupInstallSettings(QString &installSettingspath) } } -static QSettings *createUserSettings() +static Utils::QtcSettings *createUserSettings() { - return new QSettings(QSettings::IniFormat, QSettings::UserScope, - QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR), - QLatin1String(Core::Constants::IDE_CASED_ID)); + return new Utils::QtcSettings(QSettings::IniFormat, + QSettings::UserScope, + 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(); - QSettings *globalSettings = new QSettings(QSettings::IniFormat, QSettings::SystemScope, - QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR), - QLatin1String(Core::Constants::IDE_CASED_ID)); + Utils::QtcSettings *settings = userSettings(); + Utils::QtcSettings *globalSettings + = new Utils::QtcSettings(QSettings::IniFormat, + QSettings::SystemScope, + QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR), + QLatin1String(Core::Constants::IDE_CASED_ID)); loadFonts(); if (Utils::HostOsInfo::isWindowsHost() diff --git a/src/libs/extensionsystem/pluginmanager.cpp b/src/libs/extensionsystem/pluginmanager.cpp index 386cf788fd8..eb097344b5f 100644 --- a/src/libs/extensionsystem/pluginmanager.cpp +++ b/src/libs/extensionsystem/pluginmanager.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -56,6 +55,7 @@ #include #include #include +#include #include #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; diff --git a/src/libs/extensionsystem/pluginmanager.h b/src/libs/extensionsystem/pluginmanager.h index 40ccec7469f..cb9bc81d317 100644 --- a/src/libs/extensionsystem/pluginmanager.h +++ b/src/libs/extensionsystem/pluginmanager.h @@ -26,14 +26,15 @@ #pragma once #include "extensionsystem_global.h" + #include +#include #include #include 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 QSettings *settings(); - static void setGlobalSettings(QSettings *settings); - static QSettings *globalSettings(); + static void setSettings(Utils::QtcSettings *settings); + static Utils::QtcSettings *settings(); + static void setGlobalSettings(Utils::QtcSettings *settings); + static Utils::QtcSettings *globalSettings(); static void writeSettings(); // command line arguments diff --git a/src/libs/extensionsystem/pluginmanager_p.h b/src/libs/extensionsystem/pluginmanager_p.h index 6f5dbe50e30..47968fe943a 100644 --- a/src/libs/extensionsystem/pluginmanager_p.h +++ b/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 setGlobalSettings(QSettings *settings); + void setSettings(Utils::QtcSettings *settings); + void setGlobalSettings(Utils::QtcSettings *settings); void readSettings(); void writeSettings(); @@ -124,8 +127,8 @@ public: QHash m_profileTotal; int m_profileElapsedMS = 0; unsigned m_profilingVerbosity = 0; - QSettings *settings = nullptr; - QSettings *globalSettings = nullptr; + Utils::QtcSettings *settings = nullptr; + Utils::QtcSettings *globalSettings = nullptr; // Look in argument descriptions of the specs for the option. PluginSpec *pluginForOption(const QString &option, bool *requiresArgument) const; diff --git a/src/libs/utils/CMakeLists.txt b/src/libs/utils/CMakeLists.txt index 3ccc7a2c2d9..c76e367abc7 100644 --- a/src/libs/utils/CMakeLists.txt +++ b/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 diff --git a/src/libs/utils/qtcsettings.cpp b/src/libs/utils/qtcsettings.cpp new file mode 100644 index 00000000000..350eb39a92c --- /dev/null +++ b/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 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 diff --git a/src/libs/utils/qtcsettings.h b/src/libs/utils/qtcsettings.h new file mode 100644 index 00000000000..fd4841a961b --- /dev/null +++ b/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 + +namespace Utils { + +class QTCREATOR_UTILS_EXPORT QtcSettings : public QSettings +{ +public: + using QSettings::QSettings; + + template + void setValueWithDefault(const QString &key, const T &val, const T &defaultValue); +}; + +template +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 diff --git a/src/plugins/coreplugin/generalsettings.cpp b/src/plugins/coreplugin/generalsettings.cpp index ef807f23006..9ceb1752dd1 100644 --- a/src/plugins/coreplugin/generalsettings.cpp +++ b/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->remove(QLatin1String("General/OverrideLanguage")); - else - settings->setValue(QLatin1String("General/OverrideLanguage"), locale); + settings->setValueWithDefault(QLatin1String("General/OverrideLanguage"), locale, {}); } void GeneralSettings::setShowShortcutsInContextMenu(bool show) { - if (show == m_defaultShowShortcutsInContextMenu) - ICore::settings()->remove(settingsKeyShortcutsInContextMenu); - else - ICore::settings()->setValue(settingsKeyShortcutsInContextMenu, show); + ICore::settings()->setValueWithDefault(settingsKeyShortcutsInContextMenu, + show, + m_defaultShowShortcutsInContextMenu); #if (QT_VERSION >= QT_VERSION_CHECK(5, 13, 0)) QGuiApplication::styleHints()->setShowShortcutsInContextMenus(show); #endif diff --git a/src/plugins/coreplugin/icore.cpp b/src/plugins/coreplugin/icore.cpp index f4b4c70fa9a..796afe54880 100644 --- a/src/plugins/coreplugin/icore.cpp +++ b/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(); diff --git a/src/plugins/coreplugin/icore.h b/src/plugins/coreplugin/icore.h index 0dda6a90a7b..1bed598d369 100644 --- a/src/plugins/coreplugin/icore.h +++ b/src/plugins/coreplugin/icore.h @@ -28,6 +28,8 @@ #include "core_global.h" #include "icontext.h" +#include + #include #include #include @@ -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(); diff --git a/src/plugins/help/localhelpmanager.cpp b/src/plugins/help/localhelpmanager.cpp index 3368ec82b80..67f73480b98 100644 --- a/src/plugins/help/localhelpmanager.cpp +++ b/src/plugins/help/localhelpmanager.cpp @@ -110,16 +110,6 @@ static QString defaultFallbackFontStyleName(const QString &fontFamily) return styles.first(); } -template -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()); - setOrRemoveSetting(kFontStyleNameKey, font.styleName(), defaultFallbackFontStyleName(font.family())); - setOrRemoveSetting(kFontSizeKey, font.pointSize(), kDefaultFallbackFontSize); + Core::ICore::settings()->setValueWithDefault(kFontFamilyKey, + font.family(), + 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()->remove(kViewerBackend); - else - Core::ICore::settings()->setValue(kViewerBackend, id); + Core::ICore::settings()->setValueWithDefault(kViewerBackend, id, {}); } QByteArray LocalHelpManager::viewerBackendId() diff --git a/src/plugins/mcusupport/mcusupportoptions.cpp b/src/plugins/mcusupport/mcusupportoptions.cpp index 295e0dfea2d..2e773de3782 100644 --- a/src/plugins/mcusupport/mcusupportoptions.cpp +++ b/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(); - if (m_path == m_defaultPath) - settings->remove(key); - else - settings->setValue(key, m_path); + Core::ICore::settings()->setValueWithDefault(key, m_path, m_defaultPath); } void McuPackage::setRelativePathModifier(const QString &path) diff --git a/src/plugins/vcsbase/commonvcssettings.cpp b/src/plugins/vcsbase/commonvcssettings.cpp index 2d56ac6d8bd..182e5b35aca 100644 --- a/src/plugins/vcsbase/commonvcssettings.cpp +++ b/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. - if (sshPasswordPrompt != sshPasswordPromptDefault()) - s->setValue(QLatin1String(sshPasswordPromptKeyC), sshPasswordPrompt); - else - s->remove(QLatin1String(sshPasswordPromptKeyC)); + s->setValueWithDefault(QLatin1String(sshPasswordPromptKeyC), + sshPasswordPrompt, + sshPasswordPromptDefault()); s->endGroup(); } diff --git a/src/plugins/vcsbase/commonvcssettings.h b/src/plugins/vcsbase/commonvcssettings.h index 70e1969fa4c..9ecc10a2862 100644 --- a/src/plugins/vcsbase/commonvcssettings.h +++ b/src/plugins/vcsbase/commonvcssettings.h @@ -25,10 +25,11 @@ #pragma once +#include + #include 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; diff --git a/tests/auto/extensionsystem/pluginmanager/tst_pluginmanager.cpp b/tests/auto/extensionsystem/pluginmanager/tst_pluginmanager.cpp index 354aa36f4a8..4b55043f968 100644 --- a/tests/auto/extensionsystem/pluginmanager/tst_pluginmanager.cpp +++ b/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*)));