From 5ef822da9f77e76ae0059a2cf96e82ad05654fd7 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Tue, 23 Jan 2018 16:12:47 +0100 Subject: [PATCH] AutoTest: Redo check state handling Simplify and re-arrange to avoid wrong indirections and unnecessary delegation. Beside this ensure correct check states of group nodes when adding them. Change-Id: I24a32249d785e48c9d27111d062c2a06a17327ef Reviewed-by: David Schulz --- src/plugins/autotest/qtest/qttesttreeitem.cpp | 2 +- src/plugins/autotest/testtreeitem.cpp | 69 +---------------- src/plugins/autotest/testtreeitem.h | 4 +- src/plugins/autotest/testtreemodel.cpp | 77 ++++++++++++++----- src/plugins/autotest/testtreemodel.h | 4 +- src/plugins/autotest/testtreeview.cpp | 6 +- 6 files changed, 66 insertions(+), 96 deletions(-) diff --git a/src/plugins/autotest/qtest/qttesttreeitem.cpp b/src/plugins/autotest/qtest/qttesttreeitem.cpp index a06598db74e..642bcfa21df 100644 --- a/src/plugins/autotest/qtest/qttesttreeitem.cpp +++ b/src/plugins/autotest/qtest/qttesttreeitem.cpp @@ -38,7 +38,7 @@ QtTestTreeItem::QtTestTreeItem(const QString &name, const QString &filePath, Tes : TestTreeItem(name, filePath, type) { if (type == TestDataTag) - setChecked(Qt::Checked); + setData(0, Qt::Checked, Qt::CheckStateRole); } QVariant QtTestTreeItem::data(int column, int role) const diff --git a/src/plugins/autotest/testtreeitem.cpp b/src/plugins/autotest/testtreeitem.cpp index 5c3b03724db..a597a2de7ad 100644 --- a/src/plugins/autotest/testtreeitem.cpp +++ b/src/plugins/autotest/testtreeitem.cpp @@ -106,9 +106,9 @@ QVariant TestTreeItem::data(int /*column*/, int role) const bool TestTreeItem::setData(int /*column*/, const QVariant &data, int role) { if (role == Qt::CheckStateRole) { - Qt::CheckState old = checked(); - setChecked((Qt::CheckState)data.toInt()); - return checked() != old; + Qt::CheckState old = m_checked; + m_checked = (Qt::CheckState)data.toInt(); + return m_checked != old; } return false; } @@ -168,34 +168,6 @@ bool TestTreeItem::modifyLineAndColumn(const TestParseResult *result) return hasBeenModified; } -void TestTreeItem::setChecked(const Qt::CheckState checkState) -{ - switch (m_type) { - case TestDataTag: { - m_checked = (checkState == Qt::Unchecked ? Qt::Unchecked : Qt::Checked); - if (auto parent = parentItem()) - parent->revalidateCheckState(); - break; - } - case Root: - case GroupNode: - case TestFunctionOrSet: - case TestCase: { - Qt::CheckState usedState = (checkState == Qt::Unchecked ? Qt::Unchecked : Qt::Checked); - for (int row = 0, count = childCount(); row < count; ++row) - childItem(row)->setChecked(usedState); - m_checked = usedState; - if (m_type != Root) { - if (auto parent = parentItem()) - parent->revalidateCheckState(); - } - break; - } - default: - return; - } -} - Qt::CheckState TestTreeItem::checked() const { switch (m_type) { @@ -327,41 +299,6 @@ QSet TestTreeItem::internalTargets() const return targets; } -void TestTreeItem::revalidateCheckState() -{ - const Type ttiType = type(); - if (ttiType != TestCase && ttiType != TestFunctionOrSet && ttiType != Root && ttiType != GroupNode) - return; - if (childCount() == 0) // can this happen? (we're calling revalidateCS() on parentItem() - return; - bool foundChecked = false; - bool foundUnchecked = false; - bool foundPartiallyChecked = false; - for (int row = 0, count = childCount(); row < count; ++row) { - TestTreeItem *child = childItem(row); - switch (child->type()) { - case TestDataFunction: - case TestSpecialFunction: - continue; - default: - break; - } - - foundChecked |= (child->checked() == Qt::Checked); - foundUnchecked |= (child->checked() == Qt::Unchecked); - foundPartiallyChecked |= (child->checked() == Qt::PartiallyChecked); - if (foundPartiallyChecked || (foundChecked && foundUnchecked)) { - m_checked = Qt::PartiallyChecked; - if (ttiType == TestFunctionOrSet || ttiType == TestCase || ttiType == GroupNode) - parentItem()->revalidateCheckState(); - return; - } - } - m_checked = (foundUnchecked ? Qt::Unchecked : Qt::Checked); - if (ttiType == TestFunctionOrSet || ttiType == TestCase || ttiType == GroupNode) - parentItem()->revalidateCheckState(); -} - inline bool TestTreeItem::modifyFilePath(const QString &filePath) { if (m_filePath != filePath) { diff --git a/src/plugins/autotest/testtreeitem.h b/src/plugins/autotest/testtreeitem.h index 33d7117e4fe..8234321bc03 100644 --- a/src/plugins/autotest/testtreeitem.h +++ b/src/plugins/autotest/testtreeitem.h @@ -89,7 +89,6 @@ public: unsigned column() const { return m_column; } QString proFile() const { return m_proFile; } void setProFile(const QString &proFile) { m_proFile = proFile; } - virtual void setChecked(const Qt::CheckState checked); virtual Qt::CheckState checked() const; Type type() const { return m_type; } void markForRemoval(bool mark); @@ -123,7 +122,6 @@ protected: const QString &file); private: - void revalidateCheckState(); bool modifyFilePath(const QString &filePath); bool modifyName(const QString &name); @@ -143,7 +141,7 @@ private: QString m_proFile; Status m_status = NewlyAdded; - friend class TestTreeModel; // grant access to (private) revalidateCheckState() + friend class TestTreeModel; // grant access to (protected) findChildBy() }; class TestCodeLocationAndType diff --git a/src/plugins/autotest/testtreemodel.cpp b/src/plugins/autotest/testtreemodel.cpp index 63d83e54e6a..a4dea3c8e00 100644 --- a/src/plugins/autotest/testtreemodel.cpp +++ b/src/plugins/autotest/testtreemodel.cpp @@ -108,21 +108,18 @@ bool TestTreeModel::setData(const QModelIndex &index, const QVariant &value, int if (item && item->setData(index.column(), value, role)) { emit dataChanged(index, index); if (role == Qt::CheckStateRole) { - switch (item->type()) { - case TestTreeItem::Root: - case TestTreeItem::GroupNode: - case TestTreeItem::TestCase: - if (item->childCount() > 0) - emit dataChanged(index.child(0, 0), index.child(item->childCount() - 1, 0)); - break; - case TestTreeItem::TestFunctionOrSet: - emit dataChanged(index.parent(), index.parent()); - break; - default: // avoid warning regarding unhandled enum member - break; + Qt::CheckState checked = item->checked(); + if (item->hasChildren() && checked != Qt::PartiallyChecked) { + // handle the new checkstate for children as well... + for (Utils::TreeItem *child : *item) { + const QModelIndex &idx = indexForItem(child); + setData(idx, checked ? Qt::Checked : Qt::Unchecked, Qt::CheckStateRole); + } } + if (item->parent() != rootItem() && item->parentItem()->checked() != checked) + revalidateCheckState(item->parentItem()); // handle parent too + return true; } - return true; } return false; } @@ -254,7 +251,7 @@ bool TestTreeModel::sweepChildren(TestTreeItem *item) if (child->type() != TestTreeItem::Root && child->markedForRemoval()) { destroyItem(child); - item->revalidateCheckState(); + revalidateCheckState(item); hasChanged = true; } else if (child->hasChildren()) { hasChanged |= sweepChildren(child); @@ -281,6 +278,50 @@ void TestTreeModel::insertItemInParent(TestTreeItem *item, TestTreeItem *root, b } } parentNode->appendChild(item); + if (item->checked() != parentNode->checked()) + revalidateCheckState(parentNode); +} + +void TestTreeModel::revalidateCheckState(TestTreeItem *item) +{ + QTC_ASSERT(item, return); + QTC_ASSERT(item->hasChildren(), return); + const TestTreeItem::Type type = item->type(); + if (type == TestTreeItem::TestSpecialFunction || type == TestTreeItem::TestDataFunction + || type == TestTreeItem::TestDataTag) { + return; + } + const Qt::CheckState oldState = (Qt::CheckState)item->data(0, Qt::CheckStateRole).toInt(); + Qt::CheckState newState = Qt::Checked; + bool foundChecked = false; + bool foundUnchecked = false; + bool foundPartiallyChecked = false; + for (int row = 0, count = item->childCount(); row < count; ++row) { + TestTreeItem *child = item->childItem(row); + switch (child->type()) { + case TestTreeItem::TestDataFunction: + case TestTreeItem::TestSpecialFunction: + continue; + default: + break; + } + + foundChecked |= (child->checked() == Qt::Checked); + foundUnchecked |= (child->checked() == Qt::Unchecked); + foundPartiallyChecked |= (child->checked() == Qt::PartiallyChecked); + if (foundPartiallyChecked || (foundChecked && foundUnchecked)) { + newState = Qt::PartiallyChecked; + break; + } + } + if (newState != Qt::PartiallyChecked) + newState = foundUnchecked ? Qt::Unchecked : Qt::Checked; + if (oldState != newState) { + item->setData(0, newState, Qt::CheckStateRole); + emit dataChanged(item->index(), item->index()); + if (item->parent() != rootItem() && item->parentItem()->checked() != newState) + revalidateCheckState(item->parentItem()); + } } void TestTreeModel::onParseResultReady(const TestParseResultPtr result) @@ -321,12 +362,6 @@ void TestTreeModel::handleParseResult(const TestParseResult *result, TestTreeIte QTC_ASSERT(newItem, return); insertItemInParent(newItem, parentNode, groupingEnabled); - // new items are checked by default - revalidation of parents might be necessary - if (parentNode->checked() != Qt::Checked) { - parentNode->revalidateCheckState(); - const QModelIndex &idx = indexForItem(parentNode); - emit dataChanged(idx, idx); - } } void TestTreeModel::removeAllTestItems() @@ -335,7 +370,7 @@ void TestTreeModel::removeAllTestItems() item->removeChildren(); TestTreeItem *testTreeItem = static_cast(item); if (testTreeItem->checked() == Qt::PartiallyChecked) - testTreeItem->setChecked(Qt::Checked); + testTreeItem->setData(0, Qt::Checked, Qt::CheckStateRole); } emit testTreeModelChanged(); } diff --git a/src/plugins/autotest/testtreemodel.h b/src/plugins/autotest/testtreemodel.h index a47fc9bea77..58c9b279aaa 100644 --- a/src/plugins/autotest/testtreemodel.h +++ b/src/plugins/autotest/testtreemodel.h @@ -87,8 +87,8 @@ private: void removeTestRootNodes(); void removeFiles(const QStringList &files); bool sweepChildren(TestTreeItem *item); - static void insertItemInParent(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled); - + void insertItemInParent(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled); + void revalidateCheckState(TestTreeItem *item); explicit TestTreeModel(QObject *parent = 0); void setupParsingConnections(); diff --git a/src/plugins/autotest/testtreeview.cpp b/src/plugins/autotest/testtreeview.cpp index a6bbe91b909..5c285ee229a 100644 --- a/src/plugins/autotest/testtreeview.cpp +++ b/src/plugins/autotest/testtreeview.cpp @@ -71,7 +71,7 @@ void TestTreeView::changeCheckStateAll(const Qt::CheckState checkState) int funcCount = model->rowCount(classesIndex); TestTreeItem *item = static_cast(classesIndex.internalPointer()); if (item) { - item->setChecked(checkState); + item->setData(0, checkState, Qt::CheckStateRole); if (!item->childCount()) last = classesIndex; } @@ -79,12 +79,12 @@ void TestTreeView::changeCheckStateAll(const Qt::CheckState checkState) last = model->index(functionRow, 0, classesIndex); TestTreeItem *item = static_cast(last.internalPointer()); if (item) - item->setChecked(checkState); + item->setData(0, checkState, Qt::CheckStateRole); } } if (count == 0) { if (auto item = static_cast(currentRootIndex.internalPointer())) - item->setChecked(checkState); + item->setData(0, checkState, Qt::CheckStateRole); } emit dataChanged(currentRootIndex, last); }