Some code cleanup as per suggestions

src/kitemviews/kfileitemlistview.cpp

@@ -452,8 +452,9 @@ void KFileItemListView::applyRolesToModel()
         roles.insert("expandedParentsCount");
     }
 
-    // Assure that the role that is used for sorting will be determined
+    // Assure that the roles used for sorting and grouping will be determined
     roles.insert(fileItemModel->sortRole());
+    roles.insert(fileItemModel->groupRole());
 
     fileItemModel->setRoles(roles);
     m_modelRolesUpdater->setRoles(roles);

src/kitemviews/kfileitemmodel.cpp

@@ -975,14 +975,10 @@ void KFileItemModel::onSortRoleChanged(const QByteArray &current, const QByteArr
     }
 }
 
-void KFileItemModel::onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems)
+void KFileItemModel::onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous)
 {
     Q_UNUSED(current)
     Q_UNUSED(previous)
-
-    if (resortItems) {
-        resortAllItems();
-    }
 }
 
 void KFileItemModel::onGroupRoleChanged(const QByteArray &current, const QByteArray &previous, bool resortItems)
@@ -1008,14 +1004,10 @@ void KFileItemModel::onGroupRoleChanged(const QByteArray &current, const QByteAr
     }
 }
 
-void KFileItemModel::onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems)
+void KFileItemModel::onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous)
 {
     Q_UNUSED(current)
     Q_UNUSED(previous)
-
-    if (resortItems) {
-        resortAllItems();
-    }
 }
 
 void KFileItemModel::loadSortingSettings()
@@ -1716,9 +1708,9 @@ QList<KFileItemModel::ItemData *> KFileItemModel::createItemDataList(const QUrl
     return itemDataList;
 }
 
-void KFileItemModel::prepareItemsForSorting(QList<ItemData *> &itemDataList)
+void KFileItemModel::prepareItemsWithRole(QList<ItemData *> &itemDataList, RoleType roleType)
 {
-    switch (m_sortRole) {
+    switch (roleType) {
     case ExtensionRole:
     case PermissionsRole:
     case OwnerRole:
@@ -1755,35 +1747,12 @@ void KFileItemModel::prepareItemsForSorting(QList<ItemData *> &itemDataList)
         // DateRole).
         break;
     }
-    switch (m_groupRole) {
+}
-    case ExtensionRole:
-    case PermissionsRole:
-    case OwnerRole:
-    case GroupRole:
-    case DestinationRole:
-    case PathRole:
-    case DeletionTimeRole:
-        for (ItemData *itemData : std::as_const(itemDataList)) {
-            if (itemData->values.isEmpty()) {
-                itemData->values = retrieveData(itemData->item, itemData->parent);
-            }
-        }
-        break;
 
-    case TypeRole:
+void KFileItemModel::prepareItemsForSorting(QList<ItemData *> &itemDataList)
-        for (ItemData *itemData : std::as_const(itemDataList)) {
+{
-            if (itemData->values.isEmpty()) {
+    prepareItemsWithRole(itemDataList, m_sortRole);
-                const KFileItem item = itemData->item;
+    prepareItemsWithRole(itemDataList, m_groupRole);
-                if (item.isDir() || item.isMimeTypeKnown()) {
-                    itemData->values = retrieveData(itemData->item, itemData->parent);
-                }
-            }
-        }
-        break;
-
-    default:
-        break;
-    }
 }
 
 int KFileItemModel::expandedParentsCount(const ItemData *data)

src/kitemviews/kfileitemmodel.h

@@ -293,9 +293,9 @@ Q_SIGNALS:
 protected:
     void onGroupedSortingChanged(bool current) override;
     void onSortRoleChanged(const QByteArray &current, const QByteArray &previous, bool resortItems = true) override;
-    void onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems = true) override;
+    void onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous) override;
     void onGroupRoleChanged(const QByteArray &current, const QByteArray &previous, bool resortItems = true) override;
-    void onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems = true) override;
+    void onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous) override;
 
 private Q_SLOTS:
     /**
@@ -396,6 +396,12 @@ private:
      */
     QList<ItemData *> createItemDataList(const QUrl &parentUrl, const KFileItemList &items) const;
 
+    /**
+     * Helper method for prepareItemsForSorting(). 
+     * For a set role, fills 'values' of ItemData non-lazily.
+     */
+    void prepareItemsWithRole(QList<ItemData *> &itemDataList, RoleType roleType);
+
     /**
      * Prepares the items for sorting. Normally, the hash 'values' in ItemData is filled
      * lazily to save time and memory, but for some sort roles, it is expected that the

src/kitemviews/kitemmodelbase.cpp

@@ -68,12 +68,12 @@ QByteArray KItemModelBase::sortRole() const
     return m_sortRole;
 }
 
-void KItemModelBase::setSortOrder(Qt::SortOrder order, bool resortItems)
+void KItemModelBase::setSortOrder(Qt::SortOrder order)
 {
     if (order != m_sortOrder) {
         const Qt::SortOrder previous = m_sortOrder;
         m_sortOrder = order;
-        onSortOrderChanged(order, previous, resortItems);
+        onSortOrderChanged(order, previous);
         Q_EMIT sortOrderChanged(order, previous);
     }
 }
@@ -93,12 +93,12 @@ QByteArray KItemModelBase::groupRole() const
     return m_groupRole;
 }
 
-void KItemModelBase::setGroupOrder(Qt::SortOrder order, bool resortItems)
+void KItemModelBase::setGroupOrder(Qt::SortOrder order)
 {
     if (order != m_groupOrder) {
         const Qt::SortOrder previous = m_groupOrder;
         m_groupOrder = order;
-        onGroupOrderChanged(order, previous, resortItems);
+        onGroupOrderChanged(order, previous);
         Q_EMIT groupOrderChanged(order, previous);
     }
 }
@@ -180,11 +180,10 @@ void KItemModelBase::onSortRoleChanged(const QByteArray &current, const QByteArr
     Q_UNUSED(resortItems)
 }
 
-void KItemModelBase::onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems)
+void KItemModelBase::onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous)
 {
     Q_UNUSED(current)
     Q_UNUSED(previous)
-    Q_UNUSED(resortItems)
 }
 
 void KItemModelBase::onGroupRoleChanged(const QByteArray &current, const QByteArray &previous, bool resortItems)
@@ -194,11 +193,10 @@ void KItemModelBase::onGroupRoleChanged(const QByteArray &current, const QByteAr
     Q_UNUSED(resortItems)
 }
 
-void KItemModelBase::onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems)
+void KItemModelBase::onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous)
 {
     Q_UNUSED(current)
     Q_UNUSED(previous)
-    Q_UNUSED(resortItems)
 }
 
 QUrl KItemModelBase::url(int index) const

src/kitemviews/kitemmodelbase.h

@@ -81,7 +81,7 @@ public:
      * called so that model-implementations can react on the sort order change. Afterwards the
      * signal sortOrderChanged() will be emitted.
      */
-    void setSortOrder(Qt::SortOrder order, bool resortItems = true);
+    void setSortOrder(Qt::SortOrder order);
     Qt::SortOrder sortOrder() const;
 
     /**
@@ -98,7 +98,7 @@ public:
      * called so that model-implementations can react on the group order change. Afterwards the
      * signal groupOrderChanged() will be emitted.
      */
-    void setGroupOrder(Qt::SortOrder order, bool resortItems = true);
+    void setGroupOrder(Qt::SortOrder order);
     Qt::SortOrder groupOrder() const;
 
     /**
@@ -293,7 +293,7 @@ protected:
      * itemsRemoved() signal for all items, reorder the items internally and to emit a
      * itemsInserted() signal afterwards.
      */
-    virtual void onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems = true);
+    virtual void onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous);
 
     /**
      * Is invoked if the sort role has been changed by KItemModelBase::setSortRole(). Allows
@@ -304,7 +304,7 @@ protected:
      * itemsInserted() signal afterwards.
      * The implementation should resort only if \a regroupItems is true.
      */
-    virtual void onGroupRoleChanged(const QByteArray &current, const QByteArray &previous, bool resortItems = true);
+    virtual void onGroupRoleChanged(const QByteArray &current, const QByteArray &previous, bool regroupItems = true);
 
     /**
      * Is invoked if the sort order has been changed by KItemModelBase::setSortOrder(). Allows
@@ -314,7 +314,7 @@ protected:
      * itemsRemoved() signal for all items, reorder the items internally and to emit a
      * itemsInserted() signal afterwards.
      */
-    virtual void onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous, bool resortItems = true);
+    virtual void onGroupOrderChanged(Qt::SortOrder current, Qt::SortOrder previous);
 
 private:
     bool m_groupedSorting;