From 0a202e4c29251d00d9fcca2dbef6820434695bf2 Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Fri, 17 Nov 2017 21:53:54 +0100 Subject: [PATCH] SettingsAccessor: Use optional as return value of findIssues Change-Id: Ib7ef8ec408f812a71735939514d98cf8755d292b Reviewed-by: Marco Bubke --- src/libs/utils/settingsaccessor.cpp | 62 ++++++++++++---------- src/libs/utils/settingsaccessor.h | 33 ++---------- tests/auto/utils/settings/tst_settings.cpp | 36 +++++-------- 3 files changed, 52 insertions(+), 79 deletions(-) diff --git a/src/libs/utils/settingsaccessor.cpp b/src/libs/utils/settingsaccessor.cpp index c2e328be375..210bfca9fbd 100644 --- a/src/libs/utils/settingsaccessor.cpp +++ b/src/libs/utils/settingsaccessor.cpp @@ -397,27 +397,31 @@ SettingsAccessor::ProceedInfo SettingsAccessor::reportIssues(const QVariantMap & if (!path.exists()) return Continue; - IssueInfo issue = findIssues(data, path); - QMessageBox::Icon icon = QMessageBox::Information; - - if (issue.buttons.count() > 1) - icon = QMessageBox::Question; - - QMessageBox::StandardButtons buttons = QMessageBox::NoButton; - foreach (QMessageBox::StandardButton b, issue.buttons.keys()) - buttons |= b; - - if (buttons == QMessageBox::NoButton) + const Utils::optional issue = findIssues(data, path); + if (!issue) return Continue; - QMessageBox msgBox(icon, issue.title, issue.message, buttons, parent); - if (issue.defaultButton != QMessageBox::NoButton) - msgBox.setDefaultButton(issue.defaultButton); - if (issue.escapeButton != QMessageBox::NoButton) - msgBox.setEscapeButton(issue.escapeButton); + const IssueInfo &details = issue.value(); + + const QMessageBox::Icon icon + = details.buttons.count() > 1 ? QMessageBox::Question : QMessageBox::Information; + const QMessageBox::StandardButtons buttons = [&details]() + { + QMessageBox::StandardButtons buttons = QMessageBox::NoButton; + for (const QMessageBox::StandardButton &b : details.buttons.keys()) + buttons |= b; + return buttons; + }(); + QTC_ASSERT(buttons != QMessageBox::NoButton, return Continue); + + QMessageBox msgBox(icon, details.title, details.message, buttons, parent); + if (details.defaultButton != QMessageBox::NoButton) + msgBox.setDefaultButton(details.defaultButton); + if (details.escapeButton != QMessageBox::NoButton) + msgBox.setEscapeButton(details.escapeButton); int boxAction = msgBox.exec(); - return issue.buttons.value(static_cast(boxAction)); + return details.buttons.value(static_cast(boxAction)); } /*! @@ -425,14 +429,14 @@ SettingsAccessor::ProceedInfo SettingsAccessor::reportIssues(const QVariantMap & * * Returns a IssueInfo object which is then used by reportIssues. */ -SettingsAccessor::IssueInfo SettingsAccessor::findIssues(const QVariantMap &data, const FileName &path) const +Utils::optional +SettingsAccessor::findIssues(const QVariantMap &data, const FileName &path) const { - SettingsAccessor::IssueInfo result; - const FileName defaultSettingsPath = userFilePath(d->m_baseFile, d->m_userSuffix); - int version = versionFromMap(data); + const int version = versionFromMap(data); if (data.isEmpty() || version < firstSupportedVersion() || version > currentVersion()) { + IssueInfo result; result.title = QApplication::translate("Utils::SettingsAccessor", "No Valid Settings Found"); result.message = QApplication::translate("Utils::SettingsAccessor", "

No valid settings file could be found.

" @@ -440,7 +444,10 @@ SettingsAccessor::IssueInfo SettingsAccessor::findIssues(const QVariantMap &data "were either too new or too old to be read.

") .arg(path.toUserOutput()); result.buttons.insert(QMessageBox::Ok, DiscardAndContinue); - } else if ((path != defaultSettingsPath) && (version < currentVersion())) { + return result; + } + if ((path != defaultSettingsPath) && (version < currentVersion())) { + IssueInfo result; result.title = QApplication::translate("Utils::SettingsAccessor", "Using Old Settings"); result.message = QApplication::translate("Utils::SettingsAccessor", "

The versioned backup \"%1\" of the settings " @@ -452,13 +459,12 @@ SettingsAccessor::IssueInfo SettingsAccessor::findIssues(const QVariantMap &data "the newer version.

").arg(path.toUserOutput()) .arg(d->m_applicationDisplayName); result.buttons.insert(QMessageBox::Ok, Continue); + return result; } - if (!result.buttons.isEmpty()) - return result; - - QByteArray readId = settingsIdFromMap(data); + const QByteArray readId = settingsIdFromMap(data); if (!readId.isEmpty() && readId != settingsId()) { + IssueInfo result; result.title = differentEnvironmentMsg(d->m_displayName); result.message = QApplication::translate("Utils::EnvironmentIdAccessor", "

No .user settings file created by this instance " @@ -472,8 +478,10 @@ SettingsAccessor::IssueInfo SettingsAccessor::findIssues(const QVariantMap &data result.escapeButton = QMessageBox::No; result.buttons.insert(QMessageBox::Yes, SettingsAccessor::Continue); result.buttons.insert(QMessageBox::No, SettingsAccessor::DiscardAndContinue); + return result; } - return result; + + return Utils::nullopt; } void SettingsAccessor::storeSharedSettings(const QVariantMap &data) const diff --git a/src/libs/utils/settingsaccessor.h b/src/libs/utils/settingsaccessor.h index b6e712f9107..0c62c0fd799 100644 --- a/src/libs/utils/settingsaccessor.h +++ b/src/libs/utils/settingsaccessor.h @@ -28,6 +28,7 @@ #include "utils_global.h" #include "fileutils.h" +#include "optional.h" #include #include @@ -80,35 +81,10 @@ public: typedef QHash ButtonMap; class IssueInfo { public: - IssueInfo() : defaultButton(QMessageBox::NoButton), escapeButton(QMessageBox::NoButton) { } - IssueInfo(const QString &t, const QString &m, - QMessageBox::StandardButton d = QMessageBox::NoButton, - QMessageBox::StandardButton e = QMessageBox::NoButton, - const ButtonMap &b = ButtonMap()) : - title(t), message(m), defaultButton(d), escapeButton(e), buttons(b) - { } - IssueInfo(const IssueInfo &other) : - title(other.title), - message(other.message), - defaultButton(other.defaultButton), - escapeButton(other.escapeButton), - buttons(other.buttons) - { } - - IssueInfo &operator = (const IssueInfo &other) - { - title = other.title; - message = other.message; - defaultButton = other.defaultButton; - escapeButton = other.escapeButton; - buttons = other.buttons; - return *this; - } - QString title; QString message; - QMessageBox::StandardButton defaultButton; - QMessageBox::StandardButton escapeButton; + QMessageBox::StandardButton defaultButton = QMessageBox::NoButton; + QMessageBox::StandardButton escapeButton = QMessageBox::NoButton; QHash buttons; }; @@ -130,7 +106,8 @@ protected: virtual Utils::FileName backupName(const QVariantMap &data) const; - virtual IssueInfo findIssues(const QVariantMap &data, const Utils::FileName &path) const; + virtual Utils::optional findIssues(const QVariantMap &data, + const Utils::FileName &path) const; virtual void storeSharedSettings(const QVariantMap &data) const; virtual QVariant retrieveSharedSettings() const; diff --git a/tests/auto/utils/settings/tst_settings.cpp b/tests/auto/utils/settings/tst_settings.cpp index cdc824d4d88..f926cdf2ef6 100644 --- a/tests/auto/utils/settings/tst_settings.cpp +++ b/tests/auto/utils/settings/tst_settings.cpp @@ -473,11 +473,9 @@ void tst_SettingsAccessor::findIssues_ok() const QVariantMap data = versionedMap(6, TESTACCESSOR_DEFAULT_ID); const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(info.title.isEmpty()); - QVERIFY(info.message.isEmpty()); - QVERIFY(info.buttons.isEmpty()); + QVERIFY(!info); } void tst_SettingsAccessor::findIssues_emptyData() @@ -486,11 +484,9 @@ void tst_SettingsAccessor::findIssues_emptyData() const QVariantMap data; const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(!info.title.isEmpty()); - QVERIFY(!info.message.isEmpty()); - QVERIFY(!info.buttons.isEmpty()); + QVERIFY(info); } void tst_SettingsAccessor::findIssues_tooNew() @@ -499,11 +495,9 @@ void tst_SettingsAccessor::findIssues_tooNew() const QVariantMap data = versionedMap(42, TESTACCESSOR_DEFAULT_ID); const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(!info.title.isEmpty()); - QVERIFY(!info.message.isEmpty()); - QVERIFY(!info.buttons.isEmpty()); + QVERIFY(info); } void tst_SettingsAccessor::findIssues_tooOld() @@ -512,11 +506,9 @@ void tst_SettingsAccessor::findIssues_tooOld() const QVariantMap data = versionedMap(2, TESTACCESSOR_DEFAULT_ID); const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(!info.title.isEmpty()); - QVERIFY(!info.message.isEmpty()); - QVERIFY(!info.buttons.isEmpty()); + QVERIFY(info); } void tst_SettingsAccessor::findIssues_wrongId() @@ -525,11 +517,9 @@ void tst_SettingsAccessor::findIssues_wrongId() const QVariantMap data = versionedMap(6, "foo"); const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(!info.title.isEmpty()); - QVERIFY(!info.message.isEmpty()); - QVERIFY(!info.buttons.isEmpty()); + QVERIFY(info); } void tst_SettingsAccessor::findIssues_nonDefaultPath() @@ -538,11 +528,9 @@ void tst_SettingsAccessor::findIssues_nonDefaultPath() const QVariantMap data = versionedMap(6, TESTACCESSOR_DEFAULT_ID); const Utils::FileName path = Utils::FileName::fromString("/foo/bar.user.foobar"); - const Utils::SettingsAccessor::IssueInfo info = accessor.findIssues(data, path); + const Utils::optional info = accessor.findIssues(data, path); - QVERIFY(!info.title.isEmpty()); - QVERIFY(!info.message.isEmpty()); - QVERIFY(!info.buttons.isEmpty()); + QVERIFY(info); } QTEST_MAIN(tst_SettingsAccessor)