Debugger: Make cancel work in DebuggerOptionsPage

Change-Id: I09735507703e9a2c55aff68a25a4405cf12c9ab4
Reviewed-by: hjk <hjk121@nokiamail.com>

6 files changed, 202 insertions, 113 deletions

diff --git a/src/plugins/debugger/debuggeritem.cpp b/src/plugins/debugger/debuggeritem.cpp
index 33396b84be..0768d2ecdd 100644
--- a/src/plugins/debugger/debuggeritem.cpp
+++ b/src/plugins/debugger/debuggeritem.cpp
@@ -61,6 +61,13 @@ DebuggerItem::DebuggerItem()
     m_isAutoDetected = false;
 }
 
+DebuggerItem::DebuggerItem(const QVariant &id)
+{
+    m_id = id;
+    m_engineType = NoEngineType;
+    m_isAutoDetected = false;
+}
+
 DebuggerItem::DebuggerItem(const QVariantMap &data)
 {
     m_command = FileName::fromUserInput(data.value(QLatin1String(DEBUGGER_INFORMATION_COMMAND)).toString());
@@ -143,9 +150,9 @@ QStringList DebuggerItem::abiNames() const
 bool DebuggerItem::operator==(const DebuggerItem &other) const
 {
     return m_id == other.m_id
+            && m_displayName == other.m_displayName
             && m_isAutoDetected == other.m_isAutoDetected
-            && m_command == other.m_command
-            && m_abis == other.m_abis;
+            && m_command == other.m_command;
 }
 
 QVariantMap DebuggerItem::toMap() const
@@ -233,7 +240,7 @@ DebuggerItem::MatchLevel DebuggerItem::matchTarget(const Abi &targetAbi) const
 
 bool Debugger::DebuggerItem::isValid() const
 {
-    return m_engineType != NoEngineType;
+    return !m_id.isNull();
 }
 
 } // namespace Debugger;
diff --git a/src/plugins/debugger/debuggeritem.h b/src/plugins/debugger/debuggeritem.h
index c2ef2a6fec..14219471e9 100644
--- a/src/plugins/debugger/debuggeritem.h
+++ b/src/plugins/debugger/debuggeritem.h
@@ -42,6 +42,8 @@
 
 namespace Debugger {
 
+namespace Internal { class DebuggerItemModel; }
+
 // -----------------------------------------------------------------------
 // DebuggerItem
 // -----------------------------------------------------------------------
@@ -85,12 +87,16 @@ public:
     bool operator==(const DebuggerItem &other) const;
 
 private:
+    DebuggerItem(const QVariant &id);
+
     QVariant m_id;
     QString m_displayName;
     DebuggerEngineType m_engineType;
     Utils::FileName m_command;
     bool m_isAutoDetected;
     QList<ProjectExplorer::Abi> m_abis;
+
+    friend class Internal::DebuggerItemModel;
 };
 
 } // namespace Debugger
diff --git a/src/plugins/debugger/debuggeritemmanager.cpp b/src/plugins/debugger/debuggeritemmanager.cpp
index f5f0fead2e..fcca5d8eac 100644
--- a/src/plugins/debugger/debuggeritemmanager.cpp
+++ b/src/plugins/debugger/debuggeritemmanager.cpp
@@ -90,7 +90,7 @@ static void readDebuggers(const FileName &fileName, bool isSystem)
             // SDK debuggers are always considered to be up-to-date, so no need to recheck them.
         } else {
             // User settings.
-            if (item.isAutoDetected() && !item.isValid()) {
+            if (item.isAutoDetected() && (!item.isValid() || item.engineType() == NoEngineType)) {
                 qWarning() << QString::fromLatin1("DebuggerItem \"%1\" (%2) dropped since it is not valid")
                               .arg(item.command().toString()).arg(item.id().toString());
                 continue;
@@ -321,7 +321,7 @@ void DebuggerItemManager::saveDebuggers()
 
     int count = 0;
     foreach (const DebuggerItem &item, m_debuggers) {
-        if (item.isValid()) {
+        if (item.isValid() && item.engineType() != NoEngineType) {
             QVariantMap tmp = item.toMap();
             if (tmp.isEmpty())
                 continue;
diff --git a/src/plugins/debugger/debuggeritemmodel.cpp b/src/plugins/debugger/debuggeritemmodel.cpp
index 8779b9ab69..2adb8d75d4 100644
--- a/src/plugins/debugger/debuggeritemmodel.cpp
+++ b/src/plugins/debugger/debuggeritemmodel.cpp
@@ -44,9 +44,12 @@ static QList<QStandardItem *> describeItem(const DebuggerItem &item)
     row.append(new QStandardItem(item.command().toUserOutput()));
     row.append(new QStandardItem(item.engineTypeName()));
     row.at(0)->setData(item.id());
+    row.at(0)->setData(item.abiNames(), Qt::UserRole + 2);
     row.at(0)->setEditable(false);
     row.at(1)->setEditable(false);
+    row.at(1)->setData(item.toMap());
     row.at(2)->setEditable(false);
+    row.at(2)->setData(static_cast<int>(item.engineType()));
     row.at(0)->setSelectable(true);
     row.at(1)->setSelectable(true);
     row.at(2)->setSelectable(true);
@@ -86,7 +89,7 @@ DebuggerItemModel::DebuggerItemModel(QObject *parent)
     appendRow(row);
 
     foreach (const DebuggerItem &item, DebuggerItemManager::debuggers())
-        addDebugger(item);
+        addDebuggerStandardItem(item, false);
 
     QObject *manager = DebuggerItemManager::instance();
     connect(manager, SIGNAL(debuggerAdded(QVariant)),
@@ -112,6 +115,87 @@ QVariant DebuggerItemModel::headerData(int section, Qt::Orientation orientation,
     return QVariant();
 }
 
+bool DebuggerItemModel::addDebuggerStandardItem(const DebuggerItem &item, bool changed)
+{
+    if (findStandardItemById(item.id()))
+        return false;
+
+    QList<QStandardItem *> row = describeItem(item);
+    foreach (QStandardItem *cell, row) {
+        QFont font = cell->font();
+        font.setBold(changed);
+        cell->setFont(font);
+    }
+    (item.isAutoDetected() ? m_autoRoot : m_manualRoot)->appendRow(row);
+    return true;
+}
+
+bool DebuggerItemModel::removeDebuggerStandardItem(const QVariant &id)
+{
+    QStandardItem *sitem = findStandardItemById(id);
+    QTC_ASSERT(sitem, return false);
+    QStandardItem *parent = sitem->parent();
+    QTC_ASSERT(parent, return false);
+    // This will trigger a change of m_currentDebugger via changing the
+    // view selection.
+    parent->removeRow(sitem->row());
+    return true;
+}
+
+bool DebuggerItemModel::updateDebuggerStandardItem(const DebuggerItem &item, bool changed)
+{
+    QStandardItem *sitem = findStandardItemById(item.id());
+    QTC_ASSERT(sitem, return false);
+    QStandardItem *parent = sitem->parent();
+    QTC_ASSERT(parent, return false);
+    int row = sitem->row();
+    QFont font = sitem->font();
+    font.setBold(changed);
+    parent->child(row, 0)->setData(item.displayName(), Qt::DisplayRole);
+    parent->child(row, 0)->setFont(font);
+    parent->child(row, 1)->setData(item.command().toUserOutput(), Qt::DisplayRole);
+    parent->child(row, 1)->setFont(font);
+    parent->child(row, 2)->setData(item.engineTypeName(), Qt::DisplayRole);
+    parent->child(row, 2)->setData(static_cast<int>(item.engineType()));
+    parent->child(row, 2)->setFont(font);
+    return true;
+}
+
+DebuggerItem DebuggerItemModel::debuggerItem(QStandardItem *sitem) const
+{
+    DebuggerItem item = DebuggerItem(QVariant());
+    if (sitem && sitem->parent()) {
+        item.setAutoDetected(sitem->parent() == m_autoRoot);
+
+        QStandardItem *i = sitem->parent()->child(sitem->row(), 0);
+        item.m_id = i->data();
+        item.setDisplayName(i->data(Qt::DisplayRole).toString());
+
+        QStringList abis = i->data(Qt::UserRole + 2).toStringList();
+        QList<ProjectExplorer::Abi> abiList;
+        foreach (const QString &abi, abis)
+            abiList << ProjectExplorer::Abi(abi);
+        item.setAbis(abiList);
+
+        i = sitem->parent()->child(sitem->row(), 1);
+        item.setCommand(Utils::FileName::fromUserInput(i->data(Qt::DisplayRole).toString()));
+
+        i = sitem->parent()->child(sitem->row(), 2);
+        item.setEngineType(static_cast<DebuggerEngineType>(i->data().toInt()));
+    }
+    return item;
+}
+
+QList<DebuggerItem> DebuggerItemModel::debuggerItems() const
+{
+    QList<DebuggerItem> result;
+    for (int i = 0, n = m_autoRoot->rowCount(); i != n; ++i)
+        result << debuggerItem(m_autoRoot->child(i));
+    for (int i = 0, n = m_manualRoot->rowCount(); i != n; ++i)
+        result << debuggerItem(m_manualRoot->child(i));
+    return result;
+}
+
 QStandardItem *DebuggerItemModel::currentStandardItem() const
 {
     return findStandardItemById(m_currentDebugger);
@@ -145,70 +229,62 @@ QModelIndex DebuggerItemModel::lastIndex() const
     return current ? current->index() : QModelIndex();
 }
 
-void DebuggerItemModel::markCurrentDirty()
-{
-    QStandardItem *sitem = currentStandardItem();
-    QTC_ASSERT(sitem, return);
-    QFont font = sitem->font();
-    font.setBold(true);
-    sitem->setFont(font);
-}
-
 void DebuggerItemModel::onDebuggerAdded(const QVariant &id)
 {
     const DebuggerItem *item = DebuggerItemManager::findById(id);
     QTC_ASSERT(item, return);
-    addDebugger(*item);
+    if (!addDebuggerStandardItem(*item, false))
+        updateDebuggerStandardItem(*item, false); // already had it added, so just update it.
 }
 
 void DebuggerItemModel::onDebuggerUpdate(const QVariant &id)
 {
-    updateDebugger(id);
+    const DebuggerItem *item = DebuggerItemManager::findById(id);
+    QTC_ASSERT(item, return);
+    updateDebuggerStandardItem(*item, false);
 }
 
 void DebuggerItemModel::onDebuggerRemoval(const QVariant &id)
 {
-    removeDebugger(id);
+    removeDebuggerStandardItem(id);
 }
 
 void DebuggerItemModel::addDebugger(const DebuggerItem &item)
 {
-    QTC_ASSERT(item.id().isValid(), return);
-    QList<QStandardItem *> row = describeItem(item);
-    (item.isAutoDetected() ? m_autoRoot : m_manualRoot)->appendRow(row);
+    addDebuggerStandardItem(item, true);
 }
 
 void DebuggerItemModel::removeDebugger(const QVariant &id)
 {
-    QStandardItem *sitem = findStandardItemById(id);
-    QTC_ASSERT(sitem, return);
-    QStandardItem *parent = sitem->parent();
-    QTC_ASSERT(parent, return);
-    // This will trigger a change of m_currentDebugger via changing the
-    // view selection.
-    parent->removeRow(sitem->row());
+    if (!removeDebuggerStandardItem(id)) // Nothing there!
+        return;
+
+    if (DebuggerItemManager::findById(id))
+        m_removedItems.append(id);
+}
+
+void DebuggerItemModel::updateDebugger(const DebuggerItem &item)
+{
+    updateDebuggerStandardItem(item, true);
 }
 
-void DebuggerItemModel::updateDebugger(const QVariant &id)
-{
-    QList<DebuggerItem> debuggers = DebuggerItemManager::debuggers();
-    for (int i = 0, n = debuggers.size(); i != n; ++i) {
-        DebuggerItem &item = debuggers[i];
-        if (item.id() == id) {
-            QStandardItem *sitem = findStandardItemById(id);
-            QTC_ASSERT(sitem, return);
-            QStandardItem *parent = sitem->parent();
-            QTC_ASSERT(parent, return);
-            int row = sitem->row();
-            QFont font = sitem->font();
-            font.setBold(false);
-            parent->child(row, 0)->setData(item.displayName(), Qt::DisplayRole);
-            parent->child(row, 0)->setFont(font);
-            parent->child(row, 1)->setData(item.command().toUserOutput(), Qt::DisplayRole);
-            parent->child(row, 1)->setFont(font);
-            parent->child(row, 2)->setData(item.engineTypeName(), Qt::DisplayRole);
-            parent->child(row, 2)->setFont(font);
-            return;
+void DebuggerItemModel::apply()
+{
+    foreach (const QVariant &id, m_removedItems) {
+        const DebuggerItem *item = DebuggerItemManager::findById(id);
+        QTC_CHECK(item);
+        DebuggerItemManager::deregisterDebugger(*item);
+    }
+
+    foreach (const DebuggerItem &item, debuggerItems()) {
+        const DebuggerItem *managed = DebuggerItemManager::findById(item.id());
+        if (managed) {
+            if (*managed == item)
+                continue;
+            else
+                DebuggerItemManager::setItemData(item.id(), item.displayName(), item.command());
+        } else {
+            DebuggerItemManager::registerDebugger(item);
         }
     }
 }
@@ -219,5 +295,10 @@ void DebuggerItemModel::setCurrentIndex(const QModelIndex &index)
     m_currentDebugger = sit ? sit->data() : QVariant();
 }
 
+DebuggerItem DebuggerItemModel::currentDebugger() const
+{
+    return debuggerItem(currentStandardItem());
+}
+
 } // namespace Internal
 } // namespace Debugger
diff --git a/src/plugins/debugger/debuggeritemmodel.h b/src/plugins/debugger/debuggeritemmodel.h
index f85097748e..1f26169625 100644
--- a/src/plugins/debugger/debuggeritemmodel.h
+++ b/src/plugins/debugger/debuggeritemmodel.h
@@ -52,13 +52,13 @@ public:
     QModelIndex currentIndex() const;
     QModelIndex lastIndex() const;
     void setCurrentIndex(const QModelIndex &index);
-    QVariant currentDebugger() const { return m_currentDebugger; }
+    QVariant currentDebuggerId() const { return m_currentDebugger; }
+    DebuggerItem currentDebugger() const;
     void addDebugger(const DebuggerItem &item);
     void removeDebugger(const QVariant &id);
-    void updateDebugger(const QVariant &id);
+    void updateDebugger(const DebuggerItem &item);
 
-public slots:
-    void markCurrentDirty();
+    void apply();
 
 private slots:
     void onDebuggerAdded(const QVariant &id);
@@ -70,11 +70,20 @@ private:
     QStandardItem *findStandardItemById(const QVariant &id) const;
     QVariant headerData(int section, Qt::Orientation orientation, int role) const;
 
+    bool addDebuggerStandardItem(const DebuggerItem &item, bool changed);
+    bool removeDebuggerStandardItem(const QVariant &id);
+    bool updateDebuggerStandardItem(const DebuggerItem &item, bool changed);
+
+    DebuggerItem debuggerItem(QStandardItem *sitem) const;
+    QList<DebuggerItem> debuggerItems() const;
+
     QVariant m_currentDebugger;
 
     QStandardItem *m_autoRoot;
     QStandardItem *m_manualRoot;
     QStringList removed;
+
+    QList<QVariant> m_removedItems;
 };
 
 } // namespace Internal
diff --git a/src/plugins/debugger/debuggeroptionspage.cpp b/src/plugins/debugger/debuggeroptionspage.cpp
index 06b1c084cf..3c5aa436c6 100644
--- a/src/plugins/debugger/debuggeroptionspage.cpp
+++ b/src/plugins/debugger/debuggeroptionspage.cpp
@@ -64,10 +64,8 @@ class DebuggerItemConfigWidget : public QWidget
 
 public:
     explicit DebuggerItemConfigWidget(DebuggerItemModel *model);
-    void loadItem();
-    void saveItem();
-    void connectDirty();
-    void disconnectDirty();
+    void setItem(const DebuggerItem &item);
+    void apply();
 
 private:
     QLineEdit *m_displayNameLineEdit;
@@ -102,43 +100,19 @@ DebuggerItemConfigWidget::DebuggerItemConfigWidget(DebuggerItemModel *model) :
     formLayout->addRow(m_cdbLabel);
     formLayout->addRow(new QLabel(tr("Path:")), m_binaryChooser);
     formLayout->addRow(new QLabel(tr("ABIs:")), m_abis);
-
-    connectDirty();
-}
-
-void DebuggerItemConfigWidget::connectDirty()
-{
-    connect(m_displayNameLineEdit, SIGNAL(textChanged(QString)),
-            m_model, SLOT(markCurrentDirty()));
-    connect(m_binaryChooser, SIGNAL(changed(QString)),
-            m_model, SLOT(markCurrentDirty()));
-}
-
-void DebuggerItemConfigWidget::disconnectDirty()
-{
-    disconnect(m_displayNameLineEdit, SIGNAL(textChanged(QString)),
-               m_model, SLOT(markCurrentDirty()));
-    disconnect(m_binaryChooser, SIGNAL(changed(QString)),
-               m_model, SLOT(markCurrentDirty()));
 }
 
-void DebuggerItemConfigWidget::loadItem()
+void DebuggerItemConfigWidget::setItem(const DebuggerItem &item)
 {
-    const DebuggerItem *item = DebuggerItemManager::findById(m_model->currentDebugger());
-    if (!item)
-        return;
-
-    disconnectDirty();
-    m_displayNameLineEdit->setEnabled(!item->isAutoDetected());
-    m_displayNameLineEdit->setText(item->displayName());
+    m_displayNameLineEdit->setEnabled(!item.isAutoDetected());
+    m_displayNameLineEdit->setText(item.displayName());
 
-    m_binaryChooser->setEnabled(!item->isAutoDetected());
-    m_binaryChooser->setFileName(item->command());
-    connectDirty();
+    m_binaryChooser->setEnabled(!item.isAutoDetected());
+    m_binaryChooser->setFileName(item.command());
 
     QString text;
     QString versionCommand;
-    if (item->engineType() == CdbEngineType) {
+    if (item.engineType() == CdbEngineType) {
 #ifdef Q_OS_WIN
         const bool is64bit = winIs64BitSystem();
 #else
@@ -159,15 +133,18 @@ void DebuggerItemConfigWidget::loadItem()
     m_cdbLabel->setVisible(!text.isEmpty());
     m_binaryChooser->setCommandVersionArguments(QStringList(versionCommand));
 
-    m_abis->setText(item->abiNames().join(QLatin1String(", ")));
+    m_abis->setText(item.abiNames().join(QLatin1String(", ")));
 }
 
-void DebuggerItemConfigWidget::saveItem()
+void DebuggerItemConfigWidget::apply()
 {
-    const DebuggerItem *item = DebuggerItemManager::findById(m_model->currentDebugger());
-    QTC_ASSERT(item, return);
-    DebuggerItemManager::setItemData(item->id(), m_displayNameLineEdit->text(),
-                                                             m_binaryChooser->fileName());
+    DebuggerItem item = m_model->currentDebugger();
+    QTC_ASSERT(item.isValid(), return);
+
+    item.setDisplayName(m_displayNameLineEdit->text());
+    item.setCommand(m_binaryChooser->fileName());
+    item.reinitializeFromFile();
+    m_model->updateDebugger(item);
 }
 
 // --------------------------------------------------------------------------
@@ -253,21 +230,23 @@ QWidget *DebuggerOptionsPage::createPage(QWidget *parent)
 
 void DebuggerOptionsPage::apply()
 {
-    m_itemConfigWidget->saveItem();
-    debuggerModelChanged();
+    m_itemConfigWidget->apply();
+    m_model->apply();
 }
 
 void DebuggerOptionsPage::cloneDebugger()
 {
-    const DebuggerItem *item = DebuggerItemManager::findById(m_model->currentDebugger());
-    QTC_ASSERT(item, return);
+    DebuggerItem item = m_model->currentDebugger();
+    if (!item.isValid())
+        return;
+
     DebuggerItem newItem;
-    newItem.setCommand(item->command());
-    newItem.setEngineType(item->engineType());
-    newItem.setAbis(item->abis());
-    newItem.setDisplayName(DebuggerItemManager::uniqueDisplayName(tr("Clone of %1").arg(item->displayName())));
+    newItem.setCommand(item.command());
+    newItem.setEngineType(item.engineType());
+    newItem.setAbis(item.abis());
+    newItem.setDisplayName(DebuggerItemManager::uniqueDisplayName(tr("Clone of %1").arg(item.displayName())));
     newItem.setAutoDetected(false);
-    DebuggerItemManager::addDebugger(newItem);
+    m_model->addDebugger(newItem);
     m_debuggerView->setCurrentIndex(m_model->lastIndex());
 }
 
@@ -277,14 +256,14 @@ void DebuggerOptionsPage::addDebugger()
     item.setEngineType(NoEngineType);
     item.setDisplayName(DebuggerItemManager::uniqueDisplayName(tr("New Debugger")));
     item.setAutoDetected(false);
-    DebuggerItemManager::addDebugger(item);
+    m_model->addDebugger(item);
     m_debuggerView->setCurrentIndex(m_model->lastIndex());
 }
 
 void DebuggerOptionsPage::removeDebugger()
 {
-    QVariant id = m_model->currentDebugger();
-    DebuggerItemManager::removeDebugger(id);
+    QVariant id = m_model->currentDebuggerId();
+    m_model->removeDebugger(id);
     m_debuggerView->setCurrentIndex(m_model->lastIndex());
 }
 
@@ -314,8 +293,10 @@ void DebuggerOptionsPage::debuggerSelectionChanged()
     mi = mi.sibling(mi.row(), 0);
     m_model->setCurrentIndex(mi);
 
-    m_itemConfigWidget->loadItem();
-    m_container->setVisible(m_model->currentDebugger().isValid());
+    DebuggerItem item = m_model->currentDebugger();
+
+    m_itemConfigWidget->setItem(item);
+    m_container->setVisible(item.isValid());
     updateState();
 }
 
@@ -323,8 +304,13 @@ void DebuggerOptionsPage::debuggerModelChanged()
 {
     QTC_ASSERT(m_container, return);
 
-    m_itemConfigWidget->loadItem();
-    m_container->setVisible(m_model->currentDebugger().isValid());
+    QVariant id = m_model->currentDebuggerId();
+    const DebuggerItem *item = DebuggerItemManager::findById(id);
+    if (!item)
+        return;
+
+    m_itemConfigWidget->setItem(*item);
+    m_container->setVisible(m_model->currentDebuggerId().isValid());
     m_debuggerView->setCurrentIndex(m_model->currentIndex());
     updateState();
 }
@@ -337,11 +323,11 @@ void DebuggerOptionsPage::updateState()
     bool canCopy = false;
     bool canDelete = false;
 
-    if (const DebuggerItem *item = DebuggerItemManager::findById(m_model->currentDebugger())) {
-        canCopy = item->isValid() && item->canClone();
-        canDelete = !item->isAutoDetected();
-        canDelete = true; // Do we want to remove auto-detected items?
-    }
+    DebuggerItem item = m_model->currentDebugger();
+
+    canCopy = item.isValid() && item.canClone();
+    canDelete = !item.isAutoDetected();
+
     m_cloneButton->setEnabled(canCopy);
     m_delButton->setEnabled(canDelete);
 }