Debugger: Make debugger aspect more explicit

There are basically 3 states for the decision whether to start
a debugger or not. But these had been under-represented by just
displaying 2 states.
Enabled (checked) and disabled (unchecked) are obvious, but the
default of Auto (represented as checked) is there for convenience
of the user, but it has some drawbacks of failing its guess
whether to enable the debugger or not.
Turn the former check boxes into a tri-state combo boxes and
explicitly display the consequences inside the summary.

Task-number: QTCREATORBUG-28627
Change-Id: Ieffed5b227ae978555258097385d6e80dfad3ac6
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@qt.io>
Reviewed-by: hjk <hjk@qt.io>

src/plugins/debugger/debuggerrunconfigurationaspect.cpp

@@ -31,121 +31,10 @@
 #include <QLabel>
 #include <QTextEdit>
 
-using namespace Debugger::Internal;
 using namespace ProjectExplorer;
 using namespace Utils;
 
 namespace Debugger {
-namespace Internal {
-
-enum DebuggerLanguageStatus {
-    DisabledLanguage = 0,
-    EnabledLanguage,
-    AutoEnabledLanguage
-};
-
-class DebuggerLanguageAspect : public BaseAspect
-{
-public:
-    DebuggerLanguageAspect() = default;
-
-    void addToLayout(Layouting::LayoutBuilder &builder) override;
-
-    bool value() const;
-    void setValue(bool val);
-
-    void setAutoSettingsKey(const QString &settingsKey);
-    void setLabel(const QString &label);
-    void setInfoLabelText(const QString &text) { m_infoLabelText = text; }
-
-    void setClickCallBack(const std::function<void (bool)> &clickCallBack)
-    {
-        m_clickCallBack = clickCallBack;
-    }
-
-    void fromMap(const QVariantMap &map) override;
-    void toMap(QVariantMap &map) const override;
-
-public:
-    DebuggerLanguageStatus m_value = AutoEnabledLanguage;
-    bool m_defaultValue = false;
-    QString m_label;
-    QString m_infoLabelText;
-    QPointer<QCheckBox> m_checkBox; // Owned by configuration widget
-    QPointer<QLabel> m_infoLabel; // Owned by configuration widget
-    QString m_autoSettingsKey;
-
-    std::function<void(bool)> m_clickCallBack;
-};
-
-void DebuggerLanguageAspect::addToLayout(Layouting::LayoutBuilder &builder)
-{
-    QTC_CHECK(!m_checkBox);
-    m_checkBox = new QCheckBox(m_label);
-    m_checkBox->setChecked(m_value);
-    m_checkBox->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Preferred);
-
-    QTC_CHECK(m_clickCallBack);
-    connect(m_checkBox, &QAbstractButton::clicked, this, m_clickCallBack, Qt::QueuedConnection);
-
-    connect(m_checkBox.data(), &QAbstractButton::clicked, this, [this] {
-        m_value = m_checkBox->isChecked() ? EnabledLanguage : DisabledLanguage;
-        emit changed();
-    });
-    builder.addItem(QString());
-    builder.addItem(m_checkBox.data());
-
-    if (!m_infoLabelText.isEmpty()) {
-        QTC_CHECK(!m_infoLabel);
-        m_infoLabel = new QLabel(m_infoLabelText);
-        connect(m_infoLabel, &QLabel::linkActivated, [](const QString &link) {
-            Core::HelpManager::showHelpUrl(link);
-        });
-        builder.addItem(m_infoLabel.data());
-    }
-}
-
-void DebuggerLanguageAspect::setAutoSettingsKey(const QString &settingsKey)
-{
-    m_autoSettingsKey = settingsKey;
-}
-
-void DebuggerLanguageAspect::fromMap(const QVariantMap &map)
-{
-    const bool val = map.value(settingsKey(), false).toBool();
-    const bool autoVal = map.value(m_autoSettingsKey, false).toBool();
-    m_value = autoVal ? AutoEnabledLanguage : val ? EnabledLanguage : DisabledLanguage;
-}
-
-void DebuggerLanguageAspect::toMap(QVariantMap &data) const
-{
-    data.insert(settingsKey(), m_value == EnabledLanguage);
-    data.insert(m_autoSettingsKey, m_value == AutoEnabledLanguage);
-}
-
-
-bool DebuggerLanguageAspect::value() const
-{
-    return m_value;
-}
-
-void DebuggerLanguageAspect::setValue(bool value)
-{
-    const DebuggerLanguageStatus status = value ? EnabledLanguage : DisabledLanguage;
-    const bool didChange = status != m_value;
-    m_value = status;
-    if (m_checkBox)
-        m_checkBox->setChecked(m_value);
-    if (didChange)
-        emit changed();
-}
-
-void DebuggerLanguageAspect::setLabel(const QString &label)
-{
-    m_label = label;
-}
-
-} // Internal
 
 /*!
     \class Debugger::DebuggerRunConfigurationAspect
@@ -158,9 +47,16 @@ DebuggerRunConfigurationAspect::DebuggerRunConfigurationAspect(Target *target)
     setDisplayName(Tr::tr("Debugger settings"));
 
     setConfigWidgetCreator([this] {
-        Layouting::Form builder;
+        Layouting::Grid builder;
         builder.addRow(m_cppAspect);
-        builder.addRow(m_qmlAspect);
+        auto info = new QLabel(
+            Tr::tr("<a href=\""
+                   "qthelp://org.qt-project.qtcreator/doc/creator-debugging-qml.html"
+                   "\">What are the prerequisites?</a>"));
+        builder.addRow({m_qmlAspect, info});
+        connect(info, &QLabel::linkActivated, [](const QString &link) {
+            Core::HelpManager::showHelpUrl(link);
+        });
         builder.addRow(m_overrideStartupAspect);
 
         static const QString env = qtcEnvironmentVariable("QTC_DEBUGGER_MULTIPROCESS");
@@ -175,10 +71,16 @@ DebuggerRunConfigurationAspect::DebuggerRunConfigurationAspect(Target *target)
 
         const auto setSummaryText = [this, details] {
             QStringList items;
-            if (m_cppAspect->value())
+            if (m_cppAspect->value() == TriState::Enabled)
-                items.append(m_cppAspect->m_label);
+                items.append(Tr::tr("Enable C++ debugger"));
-            if (m_qmlAspect->value())
+            else if (m_cppAspect->value() == TriState::Default)
-                items.append(m_qmlAspect->m_label);
+                items.append(Tr::tr("Try to determine need for C++ debugger"));
+
+            if (m_qmlAspect->value() == TriState::Enabled)
+                items.append(Tr::tr("Enable QML debugger"));
+            else if (m_qmlAspect->value() == TriState::Default)
+                items.append(Tr::tr("Try to determine need for QML debugger"));
+
             items.append(m_overrideStartupAspect->value().isEmpty()
                              ? Tr::tr("Without additional startup commands")
                              : Tr::tr("With additional startup commands"));
@@ -198,29 +100,25 @@ DebuggerRunConfigurationAspect::DebuggerRunConfigurationAspect(Target *target)
     addDataExtractor(this, &DebuggerRunConfigurationAspect::useMultiProcess, &Data::useMultiProcess);
     addDataExtractor(this, &DebuggerRunConfigurationAspect::overrideStartup, &Data::overrideStartup);
 
-    m_cppAspect = new DebuggerLanguageAspect;
+    m_cppAspect = new TriStateAspect(Tr::tr("Enabled"), Tr::tr("Disabled"), Tr::tr("Automatic"));
-    m_cppAspect->setLabel(Tr::tr("Enable C++"));
+    m_cppAspect->setLabelText(Tr::tr("C++ debugger:"));
     m_cppAspect->setSettingsKey("RunConfiguration.UseCppDebugger");
-    m_cppAspect->setAutoSettingsKey("RunConfiguration.UseCppDebuggerAuto");
 
-    m_qmlAspect = new DebuggerLanguageAspect;
+    m_qmlAspect = new TriStateAspect(Tr::tr("Enabled"), Tr::tr("Disabled"), Tr::tr("Automatic"));
-    m_qmlAspect->setLabel(Tr::tr("Enable QML"));
+    m_qmlAspect->setLabelText(Tr::tr("QML debugger:"));
     m_qmlAspect->setSettingsKey("RunConfiguration.UseQmlDebugger");
-    m_qmlAspect->setAutoSettingsKey("RunConfiguration.UseQmlDebuggerAuto");
-    m_qmlAspect->setInfoLabelText(Tr::tr("<a href=\""
-        "qthelp://org.qt-project.qtcreator/doc/creator-debugging-qml.html"
-        "\">What are the prerequisites?</a>"));
 
     // Make sure at least one of the debuggers is set to be active.
-    m_cppAspect->setClickCallBack([this](bool on) {
+    connect(m_cppAspect, &TriStateAspect::changed, this, [this]{
-        if (!on && !m_qmlAspect->value())
+        if (m_cppAspect->value() == TriState::Disabled && m_qmlAspect->value() == TriState::Disabled)
-            m_qmlAspect->setValue(true);
+            m_qmlAspect->setValue(TriState::Default);
     });
-    m_qmlAspect->setClickCallBack([this](bool on) {
+    connect(m_qmlAspect, &TriStateAspect::changed, this, [this]{
-        if (!on && !m_cppAspect->value())
+        if (m_qmlAspect->value() == TriState::Disabled && m_cppAspect->value() == TriState::Disabled)
-            m_cppAspect->setValue(true);
+            m_cppAspect->setValue(TriState::Default);
     });
 
+
     m_multiProcessAspect = new BoolAspect;
     m_multiProcessAspect->setSettingsKey("RunConfiguration.UseMultiProcess");
     m_multiProcessAspect->setLabel(Tr::tr("Enable Debugging of Subprocesses"),
@@ -242,15 +140,15 @@ DebuggerRunConfigurationAspect::~DebuggerRunConfigurationAspect()
 
 void DebuggerRunConfigurationAspect::setUseQmlDebugger(bool value)
 {
-    m_qmlAspect->setValue(value);
+    m_qmlAspect->setValue(value ? TriState::Enabled : TriState::Disabled);
 }
 
 bool DebuggerRunConfigurationAspect::useCppDebugger() const
 {
-    if (m_cppAspect->m_value == AutoEnabledLanguage)
+    if (m_cppAspect->value() == TriState::Default)
         return m_target->project()->projectLanguages().contains(
                     ProjectExplorer::Constants::CXX_LANGUAGE_ID);
-    return m_cppAspect->m_value == EnabledLanguage;
+    return m_cppAspect->value() == TriState::Enabled;
 }
 
 static bool projectHasQmlDefines(ProjectExplorer::Project *project)
@@ -269,7 +167,7 @@ static bool projectHasQmlDefines(ProjectExplorer::Project *project)
 
 bool DebuggerRunConfigurationAspect::useQmlDebugger() const
 {
-    if (m_qmlAspect->m_value == AutoEnabledLanguage) {
+    if (m_qmlAspect->value() == TriState::Default) {
         const Core::Context languages = m_target->project()->projectLanguages();
         if (!languages.contains(ProjectExplorer::Constants::QMLJS_LANGUAGE_ID))
             return projectHasQmlDefines(m_target->project());
@@ -283,7 +181,7 @@ bool DebuggerRunConfigurationAspect::useQmlDebugger() const
 
         return !languages.contains(ProjectExplorer::Constants::CXX_LANGUAGE_ID);
     }
-    return m_qmlAspect->m_value == EnabledLanguage;
+    return m_qmlAspect->value() == TriState::Enabled;
 }
 
 bool DebuggerRunConfigurationAspect::useMultiProcess() const
@@ -323,6 +221,13 @@ void DebuggerRunConfigurationAspect::fromMap(const QVariantMap &map)
 {
     m_cppAspect->fromMap(map);
     m_qmlAspect->fromMap(map);
+
+    // respect old project settings
+    if (map.value("RunConfiguration.UseCppDebuggerAuto", false).toBool())
+        m_cppAspect->setValue(TriState::Default);
+    if (map.value("RunConfiguration.UseQmlDebuggerAuto", false).toBool())
+        m_qmlAspect->setValue(TriState::Default);
+
     m_multiProcessAspect->fromMap(map);
     m_overrideStartupAspect->fromMap(map);
 }

src/plugins/debugger/debuggerrunconfigurationaspect.h

@@ -10,8 +10,6 @@
 
 namespace Debugger {
 
-namespace Internal { class DebuggerLanguageAspect; }
-
 class DEBUGGER_EXPORT DebuggerRunConfigurationAspect
     : public ProjectExplorer::GlobalOrProjectAspect
 {
@@ -40,8 +38,8 @@ public:
     };
 
 private:
-    Internal::DebuggerLanguageAspect *m_cppAspect;
+    Utils::TriStateAspect *m_cppAspect;
-    Internal::DebuggerLanguageAspect *m_qmlAspect;
+    Utils::TriStateAspect *m_qmlAspect;
     Utils::BoolAspect *m_multiProcessAspect;
     Utils::StringAspect *m_overrideStartupAspect;
     ProjectExplorer::Target *m_target;