TargetSetupPage: Improve handling of checked kits

This patch makes the TargetSetupPage only check one kit by default,
independent of whether or not a preferredKitPredicate is set or not.

The logic is the following:
  * Use the default kit first if it is preferred
  * Use any preferred kit
  * Use the default kit
  * Use any enabled kit

* Do not check kits when the kit filter changes for now, that is
  for a later patch.

* Do not change kit selection as Kits get added/removed/modified if
  that is not necessary

Change-Id: I1e2bf594b902cf3e64d62588cde1dfb8e797bd3f
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/projectexplorer/targetsetuppage.cpp

@@ -259,7 +259,6 @@ void TargetSetupPage::setupWidgets(const QString &filterText)
         if (!filterText.isEmpty() && !k->displayName().contains(filterText, Qt::CaseInsensitive))
             continue;
         const auto widget = new TargetSetupWidget(k, m_projectPath);
-        setInitialCheckState(widget);
         connect(widget, &TargetSetupWidget::selectedToggled,
                 this, &TargetSetupPage::kitSelectionChanged);
         connect(widget, &TargetSetupWidget::selectedToggled, this, &QWizardPage::completeChanged);
@@ -272,6 +271,7 @@ void TargetSetupPage::setupWidgets(const QString &filterText)
     // Setup import widget:
     m_importWidget->setCurrentDirectory(Internal::importDirectory(m_projectPath));
 
+    kitSelectionChanged();
     updateVisibility();
 }
 
@@ -291,12 +291,6 @@ void TargetSetupPage::reset()
     m_ui->allKitsCheckBox->setChecked(false);
 }
 
-void TargetSetupPage::setInitialCheckState(TargetSetupWidget *widget)
-{
-    widget->setKitSelected(widget->isEnabled() && m_preferredPredicate
-                           && m_preferredPredicate(widget->kit()));
-}
-
 TargetSetupWidget *TargetSetupPage::widget(const Core::Id kitId,
                                            TargetSetupWidget *fallback) const
 {
@@ -357,6 +351,7 @@ void TargetSetupPage::handleKitAddition(Kit *k)
 
     Q_ASSERT(!widget(k));
     addWidget(k);
+    kitSelectionChanged();
     updateVisibility();
 }
 
@@ -391,31 +386,46 @@ void TargetSetupPage::handleKitUpdate(Kit *k)
     updateVisibility();
 }
 
-void TargetSetupPage::selectAtLeastOneKit()
+void TargetSetupPage::selectAtLeastOneEnabledKit()
 {
-    bool atLeastOneKitSelected = anyOf(m_widgets, [](TargetSetupWidget *w) {
+    if (anyOf(m_widgets, [](const TargetSetupWidget *w) { return w->isKitSelected(); })) {
-            return w->isKitSelected();
+        // Something is already selected, we are done.
+        return;
+    }
+
+    TargetSetupWidget *toCheckWidget = nullptr;
+
+    const Kit *defaultKit = KitManager::defaultKit();
+    auto isPreferred = [this](const TargetSetupWidget *w) {
+        return w->isEnabled() && (!m_preferredPredicate || m_preferredPredicate(w->kit()));
+    };
+
+    // Use default kit if that is preferred:
+    toCheckWidget = findOrDefault(m_widgets, [defaultKit, isPreferred](const TargetSetupWidget *w) {
+        return isPreferred(w) && w->kit() == defaultKit;
     });
 
-    if (!atLeastOneKitSelected) {
+    if (!toCheckWidget) {
-        Kit * const defaultKit = KitManager::defaultKit();
+        // Use the first preferred widget:
-        if (defaultKit && isUsable(defaultKit)) {
+        toCheckWidget = findOrDefault(m_widgets, isPreferred);
-            if (TargetSetupWidget * const w = widget(defaultKit)) {
-                w->setKitSelected(true);
-                atLeastOneKitSelected = true;
-            }
-        }
     }
-    if (!atLeastOneKitSelected) {
+
-        for (TargetSetupWidget * const w : qAsConst(m_widgets)) {
+    if (!toCheckWidget) {
-            if (isUsable(w->kit())) {
+        // Use default kit if it is enabled:
-                w->setKitSelected(true);
+        toCheckWidget = findOrDefault(m_widgets, [defaultKit](const TargetSetupWidget *w) {
-                atLeastOneKitSelected = true;
+            return w->isEnabled() && w->kit() == defaultKit;
-            }
+        });
-        }
     }
-    if (atLeastOneKitSelected) {
+
-        kitSelectionChanged();
+    if (!toCheckWidget) {
+        // Use the first enabled widget:
+        toCheckWidget = findOrDefault(m_widgets,
+                                      [](const TargetSetupWidget *w) { return w->isEnabled(); });
+    }
+
+    if (toCheckWidget) {
+        toCheckWidget->setKitSelected(true);
+
         emit completeChanged(); // Is this necessary?
     }
 }
@@ -491,7 +501,6 @@ void TargetSetupPage::kitFilterChanged(const QString &filterText)
     // Reset currently shown kits
     reset();
     setupWidgets(filterText);
-    selectAtLeastOneKit();
 }
 
 void TargetSetupPage::doInitializePage()
@@ -499,7 +508,9 @@ void TargetSetupPage::doInitializePage()
     reset();
     setupWidgets();
     setupImports();
-    selectAtLeastOneKit();
+
+    selectAtLeastOneEnabledKit();
+
     updateVisibility();
 }
 
@@ -554,7 +565,6 @@ void TargetSetupPage::removeWidget(TargetSetupWidget *w)
 TargetSetupWidget *TargetSetupPage::addWidget(Kit *k)
 {
     const auto widget = new TargetSetupWidget(k, m_projectPath);
-    setInitialCheckState(widget);
     updateWidget(widget);
     connect(widget, &TargetSetupWidget::selectedToggled,
             this, &TargetSetupPage::kitSelectionChanged);
@@ -575,6 +585,7 @@ TargetSetupWidget *TargetSetupPage::addWidget(Kit *k)
     } else {
         reLayout();
     }
+
     return widget;
 }
 

src/plugins/projectexplorer/targetsetuppage.h

@@ -101,7 +101,7 @@ private:
     void kitSelectionChanged();
 
     bool isUpdating() const;
-    void selectAtLeastOneKit();
+    void selectAtLeastOneEnabledKit();
     void removeWidget(Kit *k) { removeWidget(widget(k)); }
     void removeWidget(Internal::TargetSetupWidget *w);
     Internal::TargetSetupWidget *addWidget(Kit *k);
@@ -116,10 +116,12 @@ private:
 
     void setupWidgets(const QString &filterText = QString());
     void reset();
-    void setInitialCheckState(Internal::TargetSetupWidget *widget);
 
-    Internal::TargetSetupWidget *widget(Kit *k, Internal::TargetSetupWidget *fallback = nullptr) const
+    Internal::TargetSetupWidget *widget(const Kit *k,
-    { return widget(k->id(), fallback); }
+                                        Internal::TargetSetupWidget *fallback = nullptr) const
+    {
+        return k ? widget(k->id(), fallback) : fallback;
+    }
     Internal::TargetSetupWidget *widget(const Core::Id kitId,
                                         Internal::TargetSetupWidget *fallback = nullptr) const;