Fix size-hint calculation in KFileItemListView

The current approach works fine but should be replaced in the longterm
by a helper class shared by KFileItemListWidget and
KFileItemListView to share information that is required to
calculate the size hints in KFileItemListView and to represent
the actual data in KFileItemListWidget.

4 files changed, 60 insertions, 54 deletions

diff --git a/src/kitemviews/kfileitemlistview.cpp b/src/kitemviews/kfileitemlistview.cpp
index 48e7306fc..68bd7bde9 100644
--- a/src/kitemviews/kfileitemlistview.cpp
+++ b/src/kitemviews/kfileitemlistview.cpp
@@ -143,7 +143,7 @@ QSizeF KFileItemListView::itemSizeHint(int index) const
         qreal maximumRequiredWidth = 0.0;
 
         foreach (const QByteArray& role, visibleRoles()) {
-            const QString text = values[role].toString();
+            const QString text = KFileItemListWidget::roleText(role, values);
             const qreal requiredWidth = option.fontMetrics.width(text);
             maximumRequiredWidth = qMax(maximumRequiredWidth, requiredWidth);
         }
@@ -427,15 +427,14 @@ QSizeF KFileItemListView::visibleRoleSizeHint(int index, const QByteArray& role)
     qreal width = m_minimumRolesWidths.value(role, 0);
     const qreal height = option.margin * 2 + option.fontMetrics.height();
 
-    const QVariant value = model()->data(index).value(role);
-    const QString text = value.toString();
+    const QHash<QByteArray, QVariant> values = model()->data(index);
+    const QString text = KFileItemListWidget::roleText(role, values);
     if (!text.isEmpty()) {
         const qreal columnMargin = option.margin * 3;
         width = qMax(width, qreal(2 * columnMargin + option.fontMetrics.width(text)));
     }
 
     if (role == "name") {
-        const QHash<QByteArray, QVariant> values = model()->data(index);
         Q_ASSERT(values.contains("expansionLevel"));
 
         // Increase the width by the expansion-toggle and the current expansion level
diff --git a/src/kitemviews/kfileitemlistwidget.cpp b/src/kitemviews/kfileitemlistwidget.cpp
index 49c5db5fd..7cf1b4df1 100644
--- a/src/kitemviews/kfileitemlistwidget.cpp
+++ b/src/kitemviews/kfileitemlistwidget.cpp
@@ -155,6 +155,51 @@ QRectF KFileItemListWidget::expansionToggleRect() const
     return m_isDir ? m_expansionArea : QRectF();
 }
 
+QString KFileItemListWidget::roleText(const QByteArray& role, const QHash<QByteArray, QVariant>& values)
+{
+    QString text;
+    const QVariant roleValue = values.value(role);
+
+    switch (roleTextId(role)) {
+    case Name:
+    case Permissions:
+    case Owner:
+    case Group:
+    case Type:
+    case Destination:
+    case Path:
+        text = roleValue.toString();
+        break;
+
+    case Size: {
+        if (values.value("isDir").toBool()) {
+            // The item represents a directory. Show the number of sub directories
+            // instead of the file size of the directory.
+            if (!roleValue.isNull()) {
+                const KIO::filesize_t size = roleValue.value<KIO::filesize_t>();
+                text = i18ncp("@item:intable", "%1 item", "%1 items", size);
+            }
+        } else {
+            const KIO::filesize_t size = roleValue.value<KIO::filesize_t>();
+            text = KIO::convertSize(size);
+        }
+        break;
+    }
+
+    case Date: {
+        const QDateTime dateTime = roleValue.toDateTime();
+        text = KGlobal::locale()->formatDateTime(dateTime);
+        break;
+    }
+
+    default:
+        Q_ASSERT(false);
+        break;
+    }
+
+    return text;
+}
+
 void KFileItemListWidget::invalidateCache()
 {
     m_dirtyLayout = true;
@@ -482,7 +527,7 @@ void KFileItemListWidget::updateIconsLayoutTextCache()
             continue;
         }
 
-        const QString text = roleText(textId, values[role]);
+        const QString text = roleText(role, values);
         m_text[textId].setText(text);
 
         qreal requiredWidth = 0;
@@ -540,7 +585,7 @@ void KFileItemListWidget::updateCompactLayoutTextCache()
     foreach (const QByteArray& role, m_sortedVisibleRoles) {
         const TextId textId = roleTextId(role);
 
-        const QString text = roleText(textId, values[role]);
+        const QString text = roleText(role, values);
         m_text[textId].setText(text);
 
         qreal requiredWidth = option.fontMetrics.width(text);
@@ -586,7 +631,7 @@ void KFileItemListWidget::updateDetailsLayoutTextCache()
     foreach (const QByteArray& role, m_sortedVisibleRoles) {
         const TextId textId = roleTextId(role);
 
-        const QString text = roleText(textId, values[role]);
+        const QString text = roleText(role, values);
         m_text[textId].setText(text);
 
         const qreal requiredWidth = option.fontMetrics.width(text);
@@ -630,50 +675,6 @@ void KFileItemListWidget::updateAdditionalInfoTextColor()
                                        (c1.blue()  * p1 + c2.blue()  * p2) / 100);
 }
 
-QString KFileItemListWidget::roleText(TextId textId, const QVariant& roleValue) const
-{
-    QString text;
-
-    switch (textId) {
-    case Name:
-    case Permissions:
-    case Owner:
-    case Group:
-    case Type:
-    case Destination:
-    case Path:
-        text = roleValue.toString();
-        break;
-
-    case Size: {
-        if (data().value("isDir").toBool()) {
-            // The item represents a directory. Show the number of sub directories
-            // instead of the file size of the directory.
-            if (!roleValue.isNull()) {
-                const KIO::filesize_t size = roleValue.value<KIO::filesize_t>();
-                text = i18ncp("@item:intable", "%1 item", "%1 items", size);
-            }
-        } else {
-            const KIO::filesize_t size = roleValue.value<KIO::filesize_t>();
-            text = KIO::convertSize(size);
-        }
-        break;
-    }
-
-    case Date: {
-        const QDateTime dateTime = roleValue.toDateTime();
-        text = KGlobal::locale()->formatDateTime(dateTime);
-        break;
-    }
-
-    default:
-        Q_ASSERT(false);
-        break;
-    }
-
-    return text;
-}
-
 void KFileItemListWidget::drawPixmap(QPainter* painter, const QPixmap& pixmap)
 {
     const bool isHiddenItem = m_text[Name].text().startsWith(QLatin1Char('.'));
diff --git a/src/kitemviews/kfileitemlistwidget.h b/src/kitemviews/kfileitemlistwidget.h
index 4f1cf1133..71c57ea73 100644
--- a/src/kitemviews/kfileitemlistwidget.h
+++ b/src/kitemviews/kfileitemlistwidget.h
@@ -52,6 +52,14 @@ public:
     virtual QRectF textBoundingRect() const;
     virtual QRectF expansionToggleRect() const;
 
+    /**
+     * @return Shown string for the role \p role of the item with the values \p values.
+     */
+    // TODO: Move this method to a helper class shared by KFileItemListWidget and
+    // KFileItemListView to share information that is required to calculate the size hints
+    // in KFileItemListView and to represent the actual data in KFileItemListWidget.
+    static QString roleText(const QByteArray& role, const QHash<QByteArray, QVariant>& values);
+
 protected:
     /**
      * Invalidates the cache which results in calling KFileItemListWidget::refreshCache() as
@@ -103,8 +111,6 @@ private:
     
     void updateAdditionalInfoTextColor();
 
-    QString roleText(TextId textId, const QVariant& roleValue) const;
-
     void drawPixmap(QPainter* painter, const QPixmap& pixmap);
 
     static QPixmap pixmapForIcon(const QString& name, int size);
diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index 3c8cdba2c..4c36a2c21 100644
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -52,7 +52,7 @@ public:
 
     virtual int count() const;
     virtual QHash<QByteArray, QVariant> data(int index) const;
-    virtual bool setData(int index, const QHash<QByteArray, QVariant> &values);
+    virtual bool setData(int index, const QHash<QByteArray, QVariant>& values);
 
     /**
      * @return True