From edad1ba516d7ccf94db17b80389eb34e06926c24 Mon Sep 17 00:00:00 2001
From: Daniel Teske <daniel.teske@theqtcompany.com>
Date: Mon, 18 May 2015 16:57:29 +0200
Subject: [PATCH] Project: Improve fromMap() error reporting interface

Instead of just a bool, return a tristate of: Ok, Error and UserAbort.
Also add a out parameter errorMessgge.

Change-Id: Icb076de49998e9372839d0631c2776e905e4a0f6
Task-number: QTCREATORBUG-13223
Reviewed-by: Tobias Hunger <tobias.hunger@theqtcompany.com>
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@theqtcompany.com>
---
 .../autotoolsproject.cpp                      |  9 +++++----
 .../autotoolsproject.h                        |  2 +-
 .../cmakeprojectmanager/cmakeproject.cpp      | 19 +++++++++++--------
 .../cmakeprojectmanager/cmakeproject.h        |  2 +-
 .../genericprojectmanager/genericproject.cpp  |  9 +++++----
 .../genericprojectmanager/genericproject.h    |  2 +-
 src/plugins/projectexplorer/project.cpp       | 17 +++++++++--------
 src/plugins/projectexplorer/project.h         |  5 +++--
 .../projectexplorer/projectexplorer.cpp       |  8 +++++---
 src/plugins/qbsprojectmanager/qbsproject.cpp  |  9 +++++----
 src/plugins/qbsprojectmanager/qbsproject.h    |  2 +-
 .../qmakeprojectmanager/qmakeproject.cpp      |  9 +++++----
 .../qmakeprojectmanager/qmakeproject.h        |  2 +-
 src/plugins/qmlprojectmanager/qmlproject.cpp  |  9 +++++----
 src/plugins/qmlprojectmanager/qmlproject.h    |  2 +-
 15 files changed, 59 insertions(+), 47 deletions(-)

diff --git a/src/plugins/autotoolsprojectmanager/autotoolsproject.cpp b/src/plugins/autotoolsprojectmanager/autotoolsproject.cpp
index 5af16f3e4f7..03969c94275 100644
--- a/src/plugins/autotoolsprojectmanager/autotoolsproject.cpp
+++ b/src/plugins/autotoolsprojectmanager/autotoolsproject.cpp
@@ -140,10 +140,11 @@ QStringList AutotoolsProject::files(FilesMode fileMode) const
 
 // This function, is called at the very beginning, to
 // restore the settings if there are some stored.
-bool AutotoolsProject::fromMap(const QVariantMap &map)
+Project::RestoreResult AutotoolsProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result != RestoreResult::Ok)
+        return result;
 
     connect(m_fileWatcher, &Utils::FileSystemWatcher::fileChanged,
             this, &AutotoolsProject::onFileChanged);
@@ -155,7 +156,7 @@ bool AutotoolsProject::fromMap(const QVariantMap &map)
     if (!activeTarget() && defaultKit)
         addTarget(createTarget(defaultKit));
 
-    return true;
+    return RestoreResult::Ok;
 }
 
 void AutotoolsProject::loadProjectTree()
diff --git a/src/plugins/autotoolsprojectmanager/autotoolsproject.h b/src/plugins/autotoolsprojectmanager/autotoolsproject.h
index aee708bdadd..4ec357e5fe3 100644
--- a/src/plugins/autotoolsprojectmanager/autotoolsproject.h
+++ b/src/plugins/autotoolsprojectmanager/autotoolsproject.h
@@ -82,7 +82,7 @@ public:
     QStringList buildTargets() const;
 
 protected:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
 
 private slots:
     /**
diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.cpp b/src/plugins/cmakeprojectmanager/cmakeproject.cpp
index ef10812ffdf..22894371d54 100644
--- a/src/plugins/cmakeprojectmanager/cmakeproject.cpp
+++ b/src/plugins/cmakeprojectmanager/cmakeproject.cpp
@@ -506,16 +506,17 @@ QStringList CMakeProject::files(FilesMode fileMode) const
     return m_files;
 }
 
-bool CMakeProject::fromMap(const QVariantMap &map)
+Project::RestoreResult CMakeProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result != RestoreResult::Ok)
+        return result;
 
     bool hasUserFile = activeTarget();
     if (!hasUserFile) {
         CMakeOpenProjectWizard copw(Core::ICore::mainWindow(), m_manager, projectDirectory().toString(), Environment::systemEnvironment());
         if (copw.exec() != QDialog::Accepted)
-            return false;
+            return RestoreResult::UserAbort;
         Kit *k = copw.kit();
         Target *t = new Target(this, k);
         CMakeBuildConfiguration *bc(new CMakeBuildConfiguration(t));
@@ -542,8 +543,10 @@ bool CMakeProject::fromMap(const QVariantMap &map)
         // We have a user file, but we could still be missing the cbp file
         // or simply run createXml with the saved settings
         CMakeBuildConfiguration *activeBC = qobject_cast<CMakeBuildConfiguration *>(activeTarget()->activeBuildConfiguration());
-        if (!activeBC)
-            return false;
+        if (!activeBC) {
+            *errorMessage = tr("Internal Error: No build configuration found in settings file.");
+            return RestoreResult::Error;
+        }
         QString cbpFile = CMakeManager::findCbpFile(QDir(activeBC->buildDirectory().toString()));
         QFileInfo cbpFileFi(cbpFile);
 
@@ -557,7 +560,7 @@ bool CMakeProject::fromMap(const QVariantMap &map)
             CMakeBuildInfo info(activeBC);
             CMakeOpenProjectWizard copw(Core::ICore::mainWindow(), m_manager, mode, &info);
             if (copw.exec() != QDialog::Accepted)
-                return false;
+                return RestoreResult::UserAbort;
             else
                 activeBC->setUseNinja(copw.useNinja());
         }
@@ -573,7 +576,7 @@ bool CMakeProject::fromMap(const QVariantMap &map)
     connect(this, SIGNAL(activeTargetChanged(ProjectExplorer::Target*)),
             this, SLOT(activeTargetWasChanged(ProjectExplorer::Target*)));
 
-    return true;
+    return RestoreResult::Ok;
 }
 
 bool CMakeProject::setupTarget(Target *t)
diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.h b/src/plugins/cmakeprojectmanager/cmakeproject.h
index db58567df9b..27d88b1613c 100644
--- a/src/plugins/cmakeprojectmanager/cmakeproject.h
+++ b/src/plugins/cmakeprojectmanager/cmakeproject.h
@@ -118,7 +118,7 @@ signals:
     void buildTargetsChanged();
 
 protected:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
     bool setupTarget(ProjectExplorer::Target *t);
 
     // called by CMakeBuildSettingsWidget
diff --git a/src/plugins/genericprojectmanager/genericproject.cpp b/src/plugins/genericprojectmanager/genericproject.cpp
index f006984922d..1465c868b89 100644
--- a/src/plugins/genericprojectmanager/genericproject.cpp
+++ b/src/plugins/genericprojectmanager/genericproject.cpp
@@ -418,10 +418,11 @@ QStringList GenericProject::buildTargets() const
     return targets;
 }
 
-bool GenericProject::fromMap(const QVariantMap &map)
+Project::RestoreResult GenericProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result != RestoreResult::Ok)
+        return result;
 
     Kit *defaultKit = KitManager::defaultKit();
     if (!activeTarget() && defaultKit)
@@ -439,7 +440,7 @@ bool GenericProject::fromMap(const QVariantMap &map)
     }
 
     refresh(Everything);
-    return true;
+    return RestoreResult::Ok;
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/plugins/genericprojectmanager/genericproject.h b/src/plugins/genericprojectmanager/genericproject.h
index 91b86e4316c..37883ea9d4a 100644
--- a/src/plugins/genericprojectmanager/genericproject.h
+++ b/src/plugins/genericprojectmanager/genericproject.h
@@ -86,7 +86,7 @@ public:
     QStringList files() const;
 
 protected:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
 
 private:
     bool saveRawFileList(const QStringList &rawFileList);
diff --git a/src/plugins/projectexplorer/project.cpp b/src/plugins/projectexplorer/project.cpp
index 6d57ccd9bf1..c9b0b8f1daa 100644
--- a/src/plugins/projectexplorer/project.cpp
+++ b/src/plugins/projectexplorer/project.cpp
@@ -312,15 +312,15 @@ void Project::saveSettings()
         d->m_accessor->saveSettings(toMap(), Core::ICore::mainWindow());
 }
 
-bool Project::restoreSettings()
+Project::RestoreResult Project::restoreSettings(QString *errorMessage)
 {
     if (!d->m_accessor)
         d->m_accessor = new Internal::UserFileAccessor(this);
     QVariantMap map(d->m_accessor->restoreSettings(Core::ICore::mainWindow()));
-    bool ok = fromMap(map);
-    if (ok)
+    RestoreResult result = fromMap(map, errorMessage);
+    if (result == RestoreResult::Ok)
         emit settingsLoaded();
-    return ok;
+    return result;
 }
 
 
@@ -364,7 +364,7 @@ Utils::FileName Project::projectDirectory(const Utils::FileName &top)
 }
 
 
-bool Project::fromMap(const QVariantMap &map)
+Project::RestoreResult Project::fromMap(const QVariantMap &map, QString *errorMessage)
 {
     if (map.contains(QLatin1String(EDITOR_SETTINGS_KEY))) {
         QVariantMap values(map.value(QLatin1String(EDITOR_SETTINGS_KEY)).toMap());
@@ -385,8 +385,9 @@ bool Project::fromMap(const QVariantMap &map)
     for (int i = 0; i < maxI; ++i) {
         const QString key(QString::fromLatin1(TARGET_KEY_PREFIX) + QString::number(i));
         if (!map.contains(key)) {
-            qWarning() << key << "was not found in data.";
-            return false;
+            if (errorMessage)
+                *errorMessage = tr("Target key %1 was not found in project settings.").arg(key);
+            return RestoreResult::Error;
         }
         QVariantMap targetMap = map.value(key).toMap();
 
@@ -399,7 +400,7 @@ bool Project::fromMap(const QVariantMap &map)
             setActiveTarget(t);
     }
 
-    return true;
+    return RestoreResult::Ok;
 }
 
 EditorConfiguration *Project::editorConfiguration() const
diff --git a/src/plugins/projectexplorer/project.h b/src/plugins/projectexplorer/project.h
index 6ebc8008538..800e0e352c5 100644
--- a/src/plugins/projectexplorer/project.h
+++ b/src/plugins/projectexplorer/project.h
@@ -105,7 +105,8 @@ public:
     Target *restoreTarget(const QVariantMap &data);
 
     void saveSettings();
-    bool restoreSettings();
+    enum class RestoreResult { Ok, Error, UserAbort };
+    RestoreResult restoreSettings(QString *errorMessage);
 
     virtual ProjectNode *rootProjectNode() const = 0;
 
@@ -168,7 +169,7 @@ signals:
     void projectLanguagesUpdated();
 
 protected:
-    virtual bool fromMap(const QVariantMap &map);
+    virtual RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
     virtual bool setupTarget(Target *t);
 
     void setId(Core::Id id);
diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index 910991dcd91..4ee86039e4e 100644
--- a/src/plugins/projectexplorer/projectexplorer.cpp
+++ b/src/plugins/projectexplorer/projectexplorer.cpp
@@ -1725,14 +1725,16 @@ QList<Project *> ProjectExplorerPlugin::openProjects(const QStringList &fileName
                     foundProjectManager = true;
                     QString tmp;
                     if (Project *pro = manager->openProject(filePath, &tmp)) {
-                        if (pro->restoreSettings()) {
+                        QString restoreError;
+                        Project::RestoreResult restoreResult = pro->restoreSettings(&restoreError);
+                        if (restoreResult == Project::RestoreResult::Ok) {
                             connect(pro, &Project::fileListChanged,
                                     m_instance, &ProjectExplorerPlugin::fileListChanged);
                             SessionManager::addProject(pro);
                             openedPro += pro;
                         } else {
-                            appendError(errorString, tr("Failed opening project \"%1\": Settings could not be restored.")
-                                        .arg(QDir::toNativeSeparators(fileName)));
+                            if (restoreResult == Project::RestoreResult::Error)
+                                appendError(errorString, restoreError);
                             delete pro;
                         }
                     }
diff --git a/src/plugins/qbsprojectmanager/qbsproject.cpp b/src/plugins/qbsprojectmanager/qbsproject.cpp
index 5d88012f3dc..d1f8b3a2117 100644
--- a/src/plugins/qbsprojectmanager/qbsproject.cpp
+++ b/src/plugins/qbsprojectmanager/qbsproject.cpp
@@ -603,10 +603,11 @@ void QbsProject::registerQbsProjectParser(QbsProjectParser *p)
         connect(m_qbsProjectParser, SIGNAL(done(bool)), this, SLOT(handleQbsParsingDone(bool)));
 }
 
-bool QbsProject::fromMap(const QVariantMap &map)
+Project::RestoreResult QbsProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result != RestoreResult::Ok)
+        return result;
 
     Kit *defaultKit = KitManager::defaultKit();
     if (!activeTarget() && defaultKit) {
@@ -617,7 +618,7 @@ bool QbsProject::fromMap(const QVariantMap &map)
         addTarget(t);
     }
 
-    return true;
+    return RestoreResult::Ok;
 }
 
 void QbsProject::generateErrors(const qbs::ErrorInfo &e)
diff --git a/src/plugins/qbsprojectmanager/qbsproject.h b/src/plugins/qbsprojectmanager/qbsproject.h
index 67ccbfc6f4f..3ab050f8275 100644
--- a/src/plugins/qbsprojectmanager/qbsproject.h
+++ b/src/plugins/qbsprojectmanager/qbsproject.h
@@ -129,7 +129,7 @@ private slots:
     void startParsing();
 
 private:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
 
     void parse(const QVariantMap &config, const Utils::Environment &env, const QString &dir);
 
diff --git a/src/plugins/qmakeprojectmanager/qmakeproject.cpp b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
index 9d54d2eafdd..4387c8806d4 100644
--- a/src/plugins/qmakeprojectmanager/qmakeproject.cpp
+++ b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
@@ -360,10 +360,11 @@ void QmakeProject::updateFileList()
     }
 }
 
-bool QmakeProject::fromMap(const QVariantMap &map)
+Project::RestoreResult QmakeProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result != RestoreResult::Ok)
+        return result;
 
     // Prune targets without buildconfigurations:
     // This can happen esp. when updating from a old version of Qt Creator
@@ -390,7 +391,7 @@ bool QmakeProject::fromMap(const QVariantMap &map)
             this, &QmakeProject::activeTargetWasChanged);
 
     scheduleAsyncUpdate(QmakeProFileNode::ParseNow);
-    return true;
+    return RestoreResult::Ok;
 }
 
 /// equalFileList compares two file lists ignoring
diff --git a/src/plugins/qmakeprojectmanager/qmakeproject.h b/src/plugins/qmakeprojectmanager/qmakeproject.h
index 6d6b85d03d4..15121c2e05b 100644
--- a/src/plugins/qmakeprojectmanager/qmakeproject.h
+++ b/src/plugins/qmakeprojectmanager/qmakeproject.h
@@ -153,7 +153,7 @@ public slots:
     void scheduleAsyncUpdateLater() { scheduleAsyncUpdate(); }
 
 protected:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
 
 private slots:
     void asyncUpdate();
diff --git a/src/plugins/qmlprojectmanager/qmlproject.cpp b/src/plugins/qmlprojectmanager/qmlproject.cpp
index 435918aa9a4..439b62c8681 100644
--- a/src/plugins/qmlprojectmanager/qmlproject.cpp
+++ b/src/plugins/qmlprojectmanager/qmlproject.cpp
@@ -343,10 +343,11 @@ QStringList QmlProject::files(FilesMode) const
     return files();
 }
 
-bool QmlProject::fromMap(const QVariantMap &map)
+Project::RestoreResult QmlProject::fromMap(const QVariantMap &map, QString *errorMessage)
 {
-    if (!Project::fromMap(map))
-        return false;
+    RestoreResult result = Project::fromMap(map, errorMessage);
+    if (result == RestoreResult::Ok)
+        return result;
 
     // refresh first - project information is used e.g. to decide the default RC's
     refresh(Everything);
@@ -407,7 +408,7 @@ bool QmlProject::fromMap(const QVariantMap &map)
 
     onActiveTargetChanged(activeTarget());
 
-    return true;
+    return RestoreResult::Ok;
 }
 
 } // namespace QmlProjectManager
diff --git a/src/plugins/qmlprojectmanager/qmlproject.h b/src/plugins/qmlprojectmanager/qmlproject.h
index 00dd5323a4d..bb529a5defc 100644
--- a/src/plugins/qmlprojectmanager/qmlproject.h
+++ b/src/plugins/qmlprojectmanager/qmlproject.h
@@ -102,7 +102,7 @@ private slots:
     void addedRunConfiguration(ProjectExplorer::RunConfiguration *);
 
 protected:
-    bool fromMap(const QVariantMap &map);
+    RestoreResult fromMap(const QVariantMap &map, QString *errorMessage);
 
 private:
     // plain format
-- 
GitLab