"Group by - Type" now respects "Folders first" setting. Minor code cleanup

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 10d9e50e34ec040153cf580826ce3482c77e753a..7d5b0f51847f0c25f4f7bf20ebf49f516b4d89ed 100644 (file)
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -2340,104 +2340,99 @@ int KFileItemModel::groupRoleCompare(const ItemData *a, const ItemData *b, const
     // Unlike sortRoleCompare, this function can and often will return 0.
     int result = 0;
 
-    int groupA, groupB;
+    ItemGroupInfo groupA, groupB;
     switch (m_groupRole) {
     case NoRole:
         // Non-trivial grouping behavior might be handled there in the future.
         return 0;
     case NameRole:
-        groupA = nameRoleGroup(a, false).comparable;
-        groupB = nameRoleGroup(b, false).comparable;
+        groupA = nameRoleGroup(a, false);
+        groupB = nameRoleGroup(b, false);
         break;
     case SizeRole:
-        groupA = sizeRoleGroup(a, false).comparable;
-        groupB = sizeRoleGroup(b, false).comparable;
+        groupA = sizeRoleGroup(a, false);
+        groupB = sizeRoleGroup(b, false);
         break;
     case ModificationTimeRole:
         groupA = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::ModificationTime);
-                     },
-                     a,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::ModificationTime);
+            },
+            a,
+            false);
         groupB = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::ModificationTime);
-                     },
-                     b,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::ModificationTime);
+            },
+            b,
+            false);
         break;
     case CreationTimeRole:
         groupA = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::CreationTime);
-                     },
-                     a,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::CreationTime);
+            },
+            a,
+            false);
         groupB = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::CreationTime);
-                     },
-                     b,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::CreationTime);
+            },
+            b,
+            false);
         break;
     case AccessTimeRole:
         groupA = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::AccessTime);
-                     },
-                     a,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::AccessTime);
+            },
+            a,
+            false);
         groupB = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->item.time(KFileItem::AccessTime);
-                     },
-                     b,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->item.time(KFileItem::AccessTime);
+            },
+            b,
+            false);
         break;
     case DeletionTimeRole:
         groupA = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->values.value("deletiontime").toDateTime();
-                     },
-                     a,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->values.value("deletiontime").toDateTime();
+            },
+            a,
+            false);
         groupB = timeRoleGroup(
-                     [](const ItemData *item) {
-                         return item->values.value("deletiontime").toDateTime();
-                     },
-                     b,
-                     false)
-                     .comparable;
+            [](const ItemData *item) {
+                return item->values.value("deletiontime").toDateTime();
+            },
+            b,
+            false);
         break;
     case PermissionsRole:
-        groupA = permissionRoleGroup(a, false).comparable;
-        groupB = permissionRoleGroup(b, false).comparable;
+        groupA = permissionRoleGroup(a, false);
+        groupB = permissionRoleGroup(b, false);
         break;
     case RatingRole:
-        groupA = ratingRoleGroup(a, false).comparable;
-        groupB = ratingRoleGroup(b, false).comparable;
+        groupA = ratingRoleGroup(a, false);
+        groupB = ratingRoleGroup(b, false);
+        break;
+    case TypeRole:
+        groupA = typeRoleGroup(a);
+        groupB = typeRoleGroup(b);
         break;
     default: {
-        QString strGroupA = genericStringRoleGroup(groupRole(), a);
-        QString strGroupB = genericStringRoleGroup(groupRole(), b);
-        result = stringCompare(strGroupA, strGroupB, collator);
+        groupA = genericStringRoleGroup(groupRole(), a);
+        groupB = genericStringRoleGroup(groupRole(), b);
         break;
     }
     }
-    if (result == 0) {
-        if (groupA < groupB) {
-            result = -1;
-        } else if (groupA > groupB) {
-            result = 1;
-        }
+    if (groupA.comparable < groupB.comparable) {
+        result = -1;
+    } else if (groupA.comparable > groupB.comparable) {
+        result = 1;
+    } else {
+        result = stringCompare(groupA.text, groupB.text, collator);
     }
     return result;
 }
@@ -2866,9 +2861,9 @@ KFileItemModel::ItemGroupInfo KFileItemModel::ratingRoleGroup(const ItemData *it
     return groupInfo;
 }
 
-QString KFileItemModel::genericStringRoleGroup(const QByteArray &role, const ItemData *itemData) const
+KFileItemModel::ItemGroupInfo KFileItemModel::genericStringRoleGroup(const QByteArray &role, const ItemData *itemData) const
 {
-    return itemData->values.value(role).toString();
+    return {0, itemData->values.value(role).toString()};
 }
 
 QList<QPair<int, QVariant>> KFileItemModel::nameRoleGroups() const
@@ -2917,6 +2912,20 @@ QList<QPair<int, QVariant>> KFileItemModel::sizeRoleGroups() const
     return groups;
 }
 
+KFileItemModel::ItemGroupInfo KFileItemModel::typeRoleGroup(const ItemData *itemData) const
+{
+    int priority = 0;
+    if (itemData->item.isDir() && m_sortDirsFirst) {
+        // Ensure folders stay first regardless of grouping order
+        if (groupOrder() == Qt::AscendingOrder) {
+            priority = -1;
+        } else {
+            priority = 1;
+        }
+    }
+    return {priority, itemData->values.value("type").toString()};
+}
+
 QList<QPair<int, QVariant>> KFileItemModel::timeRoleGroups(const std::function<QDateTime(const ItemData *)> &fileTimeCb) const
 {
     Q_ASSERT(!m_itemData.isEmpty());
@@ -2995,17 +3004,17 @@ QList<QPair<int, QVariant>> KFileItemModel::genericStringRoleGroups(const QByteA
     const int maxIndex = count() - 1;
     QList<QPair<int, QVariant>> groups;
 
-    QString groupText;
+    ItemGroupInfo groupInfo;
     for (int i = 0; i <= maxIndex; ++i) {
         if (isChildItem(i)) {
             continue;
         }
 
-        QString newGroupText = genericStringRoleGroup(role, m_itemData.at(i));
+        ItemGroupInfo newGroupInfo = genericStringRoleGroup(role, m_itemData.at(i));
 
-        if (newGroupText != groupText) {
-            groupText = newGroupText;
-            groups.append(QPair<int, QVariant>(i, newGroupText));
+        if (newGroupInfo != groupInfo) {
+            groupInfo = newGroupInfo;
+            groups.append(QPair<int, QVariant>(i, newGroupInfo.text));
         }
     }
     return groups;

diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index ea72a48a4660f3de930bf82c62f65a676d087a41..f89eae9469976709aab6a79b62fa0e00a9d4e004 100644 (file)
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -477,13 +477,15 @@ private:
     ItemGroupInfo timeRoleGroup(const std::function<QDateTime(const ItemData *)> &fileTimeCb, const ItemData *itemData, bool withString = true) const;
     ItemGroupInfo permissionRoleGroup(const ItemData *itemData, bool withString = true) const;
     ItemGroupInfo ratingRoleGroup(const ItemData *itemData, bool withString = true) const;
-    QString genericStringRoleGroup(const QByteArray &role, const ItemData *itemData) const;
+    ItemGroupInfo typeRoleGroup(const ItemData *itemData) const;
+    ItemGroupInfo genericStringRoleGroup(const QByteArray &role, const ItemData *itemData) const;
 
     QList<QPair<int, QVariant>> nameRoleGroups() const;
     QList<QPair<int, QVariant>> sizeRoleGroups() const;
     QList<QPair<int, QVariant>> timeRoleGroups(const std::function<QDateTime(const ItemData *)> &fileTimeCb) const;
     QList<QPair<int, QVariant>> permissionRoleGroups() const;
     QList<QPair<int, QVariant>> ratingRoleGroups() const;
+    QList<QPair<int, QVariant>> typeRoleGroups() const;
     QList<QPair<int, QVariant>> genericStringRoleGroups(const QByteArray &typeForRole) const;
 
     /**
@@ -646,13 +648,4 @@ inline bool KFileItemModel::ItemGroupInfo::operator!=(const ItemGroupInfo &other
     return comparable != other.comparable || text != other.text;
 }
 
-inline bool KFileItemModel::ItemGroupInfo::operator<(const ItemGroupInfo &other) const
-{
-    if (comparable == other.comparable) {
-        return text < other.text;
-    } else {
-        return comparable < other.comparable;
-    }
-}
-
 #endif