ProjectExplorer: Improve target setup page

- Do not hide any kits, as that it not transparent to the user.
  Instead, show all of them and disable the unsuitable ones, with
  an explanatory tool tip.
- Keep the list of kits sorted after new ones have been added.
- Do not do tons of unnecessary layout operations when setting the
  kit list up from scratch.
- Code clean-up.

Fixes: QTCREATORBUG-20018
Change-Id: I7823ec9c3e5be00c6791e61926999cea0d7e43df
Reviewed-by: hjk <hjk@qt.io>

src/plugins/projectexplorer/kit.cpp

@@ -211,6 +211,7 @@ void Kit::copyKitCommon(Kit *target, const Kit *source)
     target->d->m_sticky = source->d->m_sticky;
     target->d->m_mutable = source->d->m_mutable;
     target->d->m_irrelevantAspects = source->d->m_irrelevantAspects;
+    target->d->m_hasValidityInfo = false;
 }
 
 Kit *Kit::clone(bool keepName) const

src/plugins/projectexplorer/targetsetuppage.cpp

@@ -90,7 +90,7 @@ public:
     QLabel *noValidKitLabel;
     QLabel *optionHintLabel;
     QCheckBox *allKitsCheckBox;
-    Utils::FancyLineEdit *kitFilterLineEdit;
+    FancyLineEdit *kitFilterLineEdit;
 
     void setupUi(TargetSetupPage *q)
     {
@@ -120,7 +120,7 @@ public:
         allKitsCheckBox->setTristate(true);
         allKitsCheckBox->setText(TargetSetupPage::tr("Select all kits"));
 
-        kitFilterLineEdit = new Utils::FancyLineEdit(setupTargetPage);
+        kitFilterLineEdit = new FancyLineEdit(setupTargetPage);
         kitFilterLineEdit->setFiltering(true);
         kitFilterLineEdit->setPlaceholderText(TargetSetupPage::tr("Type to filter kits by name..."));
 
@@ -164,7 +164,7 @@ public:
         QObject::connect(allKitsCheckBox, &QAbstractButton::clicked,
                          q, &TargetSetupPage::changeAllKitsSelections);
 
-        QObject::connect(kitFilterLineEdit, &Utils::FancyLineEdit::filterChanged,
+        QObject::connect(kitFilterLineEdit, &FancyLineEdit::filterChanged,
                          q, &TargetSetupPage::kitFilterChanged);
     }
 };
@@ -174,7 +174,7 @@ public:
 using namespace Internal;
 
 TargetSetupPage::TargetSetupPage(QWidget *parent) :
-    Utils::WizardPage(parent),
+    WizardPage(parent),
     m_ui(new TargetSetupPageUi),
     m_importWidget(new ImportWidget(this)),
     m_spacer(new QSpacerItem(0,0, QSizePolicy::Minimum, QSizePolicy::MinimumExpanding))
@@ -212,9 +212,9 @@ TargetSetupPage::TargetSetupPage(QWidget *parent) :
     connect(km, &KitManager::kitRemoved, this, &TargetSetupPage::handleKitRemoval);
     connect(km, &KitManager::kitUpdated, this, &TargetSetupPage::handleKitUpdate);
     connect(m_importWidget, &ImportWidget::importFrom,
-            this, [this](const Utils::FilePath &dir) { import(dir); });
+            this, [this](const FilePath &dir) { import(dir); });
 
-    setProperty(Utils::SHORT_TITLE_PROPERTY, tr("Kits"));
+    setProperty(SHORT_TITLE_PROPERTY, tr("Kits"));
 }
 
 void TargetSetupPage::initializePage()
@@ -255,20 +255,26 @@ TargetSetupPage::~TargetSetupPage()
 
 bool TargetSetupPage::isComplete() const
 {
-    return Utils::anyOf(m_widgets, [](const TargetSetupWidget *w) {
+    return anyOf(m_widgets, [](const TargetSetupWidget *w) {
         return w->isKitSelected();
     });
 }
 
 void TargetSetupPage::setupWidgets(const QString &filterText)
 {
-    // Known profiles:
+    const auto kitList = KitManager::sortKits(KitManager::kits());
-    auto kitList = sortedKitList(m_requiredPredicate);
+    for (Kit *k : kitList) {
-
+        if (!filterText.isEmpty() && !k->displayName().contains(filterText, Qt::CaseInsensitive))
-    foreach (Kit *k, kitList) {
+            continue;
-        if (filterText.isEmpty() || k->displayName().contains(filterText, Qt::CaseInsensitive))
+        const auto widget = new TargetSetupWidget(k, m_projectPath);
-            addWidget(k);
+        connect(widget, &TargetSetupWidget::selectedToggled,
+                this, &TargetSetupPage::kitSelectionChanged);
+        connect(widget, &TargetSetupWidget::selectedToggled, this, &QWizardPage::completeChanged);
+        updateWidget(widget);
+        m_widgets.push_back(widget);
+        m_baseLayout->addWidget(widget);
     }
+    addAdditionalWidgets();
 
     // Setup import widget:
     m_importWidget->setCurrentDirectory(Internal::importDirectory(m_projectPath));
@@ -278,6 +284,7 @@ void TargetSetupPage::setupWidgets(const QString &filterText)
 
 void TargetSetupPage::reset()
 {
+    removeAdditionalWidgets();
     while (m_widgets.size() > 0) {
         TargetSetupWidget *w = m_widgets.back();
 
@@ -294,7 +301,7 @@ void TargetSetupPage::reset()
 TargetSetupWidget *TargetSetupPage::widget(const Core::Id kitId,
                                            TargetSetupWidget *fallback) const
 {
-    return Utils::findOr(m_widgets, fallback, [kitId](const TargetSetupWidget *w) {
+    return findOr(m_widgets, fallback, [kitId](const TargetSetupWidget *w) {
         return w->kit() && w->kit()->id() == kitId;
     });
 }
@@ -351,9 +358,9 @@ void TargetSetupPage::setupImports()
     if (!m_importer || m_projectPath.isEmpty())
         return;
 
-    QStringList toImport = m_importer->importCandidates();
+    const QStringList toImport = m_importer->importCandidates();
-    foreach (const QString &path, toImport)
+    for (const QString &path : toImport)
-        import(Utils::FilePath::fromString(path), true);
+        import(FilePath::fromString(path), true);
 }
 
 void TargetSetupPage::handleKitAddition(Kit *k)
@@ -387,41 +394,44 @@ void TargetSetupPage::handleKitUpdate(Kit *k)
     if (m_importer)
         m_importer->makePersistent(k);
 
-    bool acceptable = !m_requiredPredicate || m_requiredPredicate(k);
+    const auto newWidgetList = sortedWidgetList();
-    const bool wasAcceptable = Utils::contains(m_widgets, [k](const TargetSetupWidget *w) {
+    if (newWidgetList != m_widgets) { // Sorting has changed.
-        return w->kit() == k;
+        m_widgets = newWidgetList;
-    });
+        reLayout();
-    if (acceptable == wasAcceptable)
+    }
-        return;
+    updateWidget(widget(k));
-
-    if (!acceptable)
-        removeWidget(k);
-    else if (acceptable)
-        addWidget(k);
-
     kitSelectionChanged();
     updateVisibility();
 }
 
 void TargetSetupPage::selectAtLeastOneKit()
 {
-    const bool atLeastOneKitSelected = Utils::anyOf(m_widgets, [](TargetSetupWidget *w) {
+    bool atLeastOneKitSelected = anyOf(m_widgets, [](TargetSetupWidget *w) {
             return w->isKitSelected();
     });
 
     if (!atLeastOneKitSelected) {
-        TargetSetupWidget *w = m_firstWidget;
+        Kit * const defaultKit = KitManager::defaultKit();
-        Kit *defaultKit = KitManager::defaultKit();
+        if (defaultKit && isUsable(defaultKit)) {
-        if (defaultKit)
+            if (TargetSetupWidget * const w = widget(defaultKit)) {
-            w = widget(defaultKit->id(), m_firstWidget);
-        if (w) {
                 w->setKitSelected(true);
+                atLeastOneKitSelected = true;
+            }
+        }
+    }
+    if (!atLeastOneKitSelected) {
+        for (TargetSetupWidget * const w : qAsConst(m_widgets)) {
+            if (isUsable(w->kit())) {
+                w->setKitSelected(true);
+                atLeastOneKitSelected = true;
+            }
+        }
+    }
+    if (atLeastOneKitSelected) {
         kitSelectionChanged();
-        }
-        m_firstWidget = nullptr;
-    }
         emit completeChanged(); // Is this necessary?
     }
+}
 
 void TargetSetupPage::updateVisibility()
 {
@@ -437,6 +447,36 @@ void TargetSetupPage::updateVisibility()
     emit completeChanged();
 }
 
+void TargetSetupPage::reLayout()
+{
+    removeAdditionalWidgets();
+    for (TargetSetupWidget * const w : qAsConst(m_widgets))
+        m_baseLayout->removeWidget(w);
+    for (TargetSetupWidget * const w : qAsConst(m_widgets))
+        m_baseLayout->addWidget(w);
+    addAdditionalWidgets();
+}
+
+bool TargetSetupPage::compareKits(const Kit *k1, const Kit *k2)
+{
+    const QString name1 = k1->displayName();
+    const QString name2 = k2->displayName();
+    if (name1 < name2)
+        return true;
+    if (name1 > name2)
+        return false;
+    return k1 < k2;
+}
+
+std::vector<TargetSetupWidget *> TargetSetupPage::sortedWidgetList() const
+{
+    std::vector<TargetSetupWidget *> list = m_widgets;
+    sort(list, [](const TargetSetupWidget *w1, const TargetSetupWidget *w2) {
+        return compareKits(w1->kit(), w2->kit());
+    });
+    return list;
+}
+
 void TargetSetupPage::openOptions()
 {
     Core::ICore::showOptionsDialog(Constants::KITS_SETTINGS_PAGE_ID, this);
@@ -460,13 +500,6 @@ void TargetSetupPage::kitSelectionChanged()
         m_ui->allKitsCheckBox->setCheckState(Qt::Unchecked);
 }
 
-QList<Kit *> TargetSetupPage::sortedKitList(const Kit::Predicate &predicate)
-{
-    const auto kitList = KitManager::kits(predicate);
-
-    return KitManager::sortKits(kitList);
-}
-
 void TargetSetupPage::kitFilterChanged(const QString &filterText)
 {
     // Reset currently shown kits
@@ -487,12 +520,10 @@ void TargetSetupPage::changeAllKitsSelections()
 
 bool TargetSetupPage::isUpdating() const
 {
-    if (m_importer)
+    return m_importer && m_importer->isUpdating();
-        return m_importer->isUpdating();
-    return false;
 }
 
-void TargetSetupPage::import(const Utils::FilePath &path, bool silent)
+void TargetSetupPage::import(const FilePath &path, bool silent)
 {
     if (!m_importer)
         return;
@@ -520,8 +551,6 @@ void TargetSetupPage::removeWidget(TargetSetupWidget *w)
 {
     if (!w)
         return;
-    if (w == m_firstWidget)
-        m_firstWidget = nullptr;
     w->deleteLater();
     w->clearKit();
     m_widgets.erase(std::find(m_widgets.begin(), m_widgets.end(), w));
@@ -529,36 +558,58 @@ void TargetSetupPage::removeWidget(TargetSetupWidget *w)
 
 TargetSetupWidget *TargetSetupPage::addWidget(Kit *k)
 {
-    if (!k || (m_requiredPredicate && !m_requiredPredicate(k)))
+    const auto widget = new TargetSetupWidget(k, m_projectPath);
-        return nullptr;
+    updateWidget(widget);
-
-    // Not all projects have BuildConfigurations, that is perfectly fine.
-    auto *widget = new TargetSetupWidget(k, m_projectPath);
-
-    m_baseLayout->removeWidget(m_importWidget);
-    foreach (QWidget *potentialWidget, m_potentialWidgets)
-        m_baseLayout->removeWidget(potentialWidget);
-    m_baseLayout->removeItem(m_spacer);
-
-    widget->setKitSelected(m_preferredPredicate && m_preferredPredicate(k));
-    m_widgets.push_back(widget);
     connect(widget, &TargetSetupWidget::selectedToggled,
             this, &TargetSetupPage::kitSelectionChanged);
-    m_baseLayout->addWidget(widget);
-
-    m_baseLayout->addWidget(m_importWidget);
-    foreach (QWidget *widget, m_potentialWidgets)
-        m_baseLayout->addWidget(widget);
-    m_baseLayout->addItem(m_spacer);
-
     connect(widget, &TargetSetupWidget::selectedToggled, this, &QWizardPage::completeChanged);
 
-    if (!m_firstWidget)
-        m_firstWidget = widget;
 
+    // Insert widget, sorted.
+    const auto insertionPos = std::find_if(m_widgets.begin(), m_widgets.end(),
+                                           [k](const TargetSetupWidget *w) {
+        return compareKits(k, w->kit());
+    });
+    const bool addedToEnd = insertionPos == m_widgets.end();
+    m_widgets.insert(insertionPos, widget);
+    if (addedToEnd) {
+        removeAdditionalWidgets();
+        m_baseLayout->addWidget(widget);
+        addAdditionalWidgets();
+    } else {
+        reLayout();
+    }
     return widget;
 }
 
+void TargetSetupPage::addAdditionalWidgets()
+{
+    m_baseLayout->addWidget(m_importWidget);
+    for (QWidget * const widget : qAsConst(m_potentialWidgets))
+        m_baseLayout->addWidget(widget);
+    m_baseLayout->addItem(m_spacer);
+}
+
+void TargetSetupPage::removeAdditionalWidgets(QLayout *layout)
+{
+    layout->removeWidget(m_importWidget);
+    for (QWidget * const potentialWidget : qAsConst(m_potentialWidgets))
+        layout->removeWidget(potentialWidget);
+    layout->removeItem(m_spacer);
+}
+
+void TargetSetupPage::updateWidget(TargetSetupWidget *widget)
+{
+    widget->setKitSelected(widget->isEnabled() && m_preferredPredicate
+                           && m_preferredPredicate(widget->kit()));
+    widget->updateStatus(m_requiredPredicate);
+}
+
+bool TargetSetupPage::isUsable(const Kit *kit) const
+{
+    return kit->isValid() && (!m_requiredPredicate || m_requiredPredicate(kit));
+}
+
 bool TargetSetupPage::setupProject(Project *project)
 {
     QList<BuildInfo> toSetUp;
@@ -598,17 +649,9 @@ void TargetSetupPage::setUseScrollArea(bool b)
     m_ui->scrollAreaWidget->setVisible(b);
     m_ui->centralWidget->setVisible(!b);
 
-    if (oldBaseLayout) {
+    if (oldBaseLayout)
-        oldBaseLayout->removeWidget(m_importWidget);
+        removeAdditionalWidgets(oldBaseLayout);
-        foreach (QWidget *widget, m_potentialWidgets)
+    addAdditionalWidgets();
-            oldBaseLayout->removeWidget(widget);
-        oldBaseLayout->removeItem(m_spacer);
-    }
-
-    m_baseLayout->addWidget(m_importWidget);
-    foreach (QWidget *widget, m_potentialWidgets)
-        m_baseLayout->addWidget(widget);
-    m_baseLayout->addItem(m_spacer);
 }
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/targetsetuppage.h

@@ -66,8 +66,8 @@ public:
     void initializePage() override;
 
     // Call these before initializePage!
-    void setRequiredKitPredicate(const ProjectExplorer::Kit::Predicate &predicate);
+    void setRequiredKitPredicate(const Kit::Predicate &predicate);
-    void setPreferredKitPredicate(const ProjectExplorer::Kit::Predicate &predicate);
+    void setPreferredKitPredicate(const Kit::Predicate &predicate);
     void setProjectPath(const Utils::FilePath &dir);
     void setProjectImporter(ProjectImporter *importer);
     bool importLineEditHasFocus() const;
@@ -91,19 +91,27 @@ public:
     void kitFilterChanged(const QString &filterText);
 
 private:
-    void handleKitAddition(ProjectExplorer::Kit *k);
+    void handleKitAddition(Kit *k);
-    void handleKitRemoval(ProjectExplorer::Kit *k);
+    void handleKitRemoval(Kit *k);
-    void handleKitUpdate(ProjectExplorer::Kit *k);
+    void handleKitUpdate(Kit *k);
     void updateVisibility();
 
+    void reLayout();
+    static bool compareKits(const Kit *k1, const Kit *k2);
+    std::vector<Internal::TargetSetupWidget *> sortedWidgetList() const;
+
     void kitSelectionChanged();
-    static QList<Kit *> sortedKitList(const Kit::Predicate &predicate);
 
     bool isUpdating() const;
     void selectAtLeastOneKit();
     void removeWidget(Kit *k) { removeWidget(widget(k)); }
     void removeWidget(Internal::TargetSetupWidget *w);
     Internal::TargetSetupWidget *addWidget(Kit *k);
+    void addAdditionalWidgets();
+    void removeAdditionalWidgets(QLayout *layout);
+    void removeAdditionalWidgets() { removeAdditionalWidgets(m_baseLayout); }
+    void updateWidget(Internal::TargetSetupWidget *widget);
+    bool isUsable(const Kit *kit) const;
 
     void setupImports();
     void import(const Utils::FilePath &path, bool silent = false);
@@ -116,14 +124,13 @@ private:
     Internal::TargetSetupWidget *widget(const Core::Id kitId,
                                         Internal::TargetSetupWidget *fallback = nullptr) const;
 
-    ProjectExplorer::Kit::Predicate m_requiredPredicate;
+    Kit::Predicate m_requiredPredicate;
-    ProjectExplorer::Kit::Predicate m_preferredPredicate;
+    Kit::Predicate m_preferredPredicate;
     QPointer<ProjectImporter> m_importer;
     QLayout *m_baseLayout = nullptr;
     Utils::FilePath m_projectPath;
     QString m_defaultShadowBuildLocation;
     std::vector<Internal::TargetSetupWidget *> m_widgets;
-    Internal::TargetSetupWidget *m_firstWidget = nullptr;
 
     Internal::TargetSetupPageUi *m_ui;
 

src/plugins/projectexplorer/targetsetupwidget.cpp

@@ -28,7 +28,6 @@
 #include "buildconfiguration.h"
 #include "buildinfo.h"
 #include "projectexplorerconstants.h"
-#include "kit.h"
 #include "kitmanager.h"
 #include "kitoptionspage.h"
 
@@ -40,6 +39,7 @@
 #include <utils/hostosinfo.h>
 #include <utils/pathchooser.h>
 #include <utils/qtcassert.h>
+#include <utils/utilsicons.h>
 
 #include <QCheckBox>
 #include <QHBoxLayout>
@@ -69,7 +69,6 @@ TargetSetupWidget::TargetSetupWidget(Kit *k, const FilePath &projectPath) :
     m_detailsWidget->setUseCheckBox(true);
     m_detailsWidget->setChecked(false);
     m_detailsWidget->setSummaryFontBold(true);
-    m_detailsWidget->setToolTip(m_kit->toHtml());
     vboxLayout->addWidget(m_detailsWidget);
 
     auto panel = new Utils::FadingWidget(m_detailsWidget);
@@ -227,6 +226,24 @@ void TargetSetupWidget::expandWidget()
     m_detailsWidget->setState(Utils::DetailsWidget::Expanded);
 }
 
+void TargetSetupWidget::updateStatus(const Kit::Predicate &predicate)
+{
+    // Kits that we deem invalid get a warning icon, but users can still select them,
+    // e.g. in case we misdetected an ABI mismatch.
+    // Kits that don't fulfill the project predicate are not selectable, because we cannot
+    // guarantee that we can handle the project sensibly (e.g. qmake project without Qt).
+    if (predicate && !predicate(kit())) {
+        setEnabled(false);
+        m_detailsWidget->setToolTip(tr("You cannot use this kit, because it does not fulfill "
+                                       "the project's prerequisites."));
+        return;
+    }
+    if (!kit()->isValid())
+        m_detailsWidget->setIcon(Icons::CRITICAL.icon());
+    setEnabled(true);
+    m_detailsWidget->setToolTip(m_kit->toHtml());
+}
+
 const QList<BuildInfo> TargetSetupWidget::buildInfoList(const Kit *k, const FilePath &projectPath)
 {
     if (auto factory = BuildConfigurationFactory::find(k, projectPath))

src/plugins/projectexplorer/targetsetupwidget.h

@@ -28,6 +28,7 @@
 #include "projectexplorer_export.h"
 
 #include "buildinfo.h"
+#include "kit.h"
 #include "task.h"
 
 #include <QWidget>
@@ -48,7 +49,6 @@ class PathChooser;
 
 namespace ProjectExplorer {
 class BuildInfo;
-class Kit;
 
 namespace Internal {
 
@@ -70,6 +70,7 @@ public:
     const QList<BuildInfo> selectedBuildInfoList() const;
     void setProjectPath(const Utils::FilePath &projectPath);
     void expandWidget();
+    void updateStatus(const Kit::Predicate &predicate);
 
 signals:
     void selectedToggled() const;