Bug 154408

This works now with new versions of collectionbrowser/CollectionSortFilterProxyModel.h and collectionbrowser/CollectionSortFilterProxyModel.cpp.  (Against SVN 29/12/07)

collectionbrowser/CollectionSortFilterProxyModel.h:
---------------------------------------------------

class CollectionSortFilterProxyModel : public QSortFilterProxyModel
{
    public:
        CollectionSortFilterProxyModel( QObject * parent = 0 ); 
         virtual ~CollectionSortFilterProxyModel();
 
        bool hasChildren(const QModelIndex &parent) const;

    protected:
        virtual bool lessThan( const QModelIndex &left, const QModelIndex &right ) const;
        virtual bool lessThanString( const QString a, const QString b ) const;
};

collectionbrowser/CollectionSortFilterProxyModel.cpp:
-----------------------------------------------------

bool
CollectionSortFilterProxyModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const
{
    CollectionTreeItem *leftItem = static_cast<CollectionTreeItem*>( left.internalPointer() );
    CollectionTreeItem *rightItem = static_cast<CollectionTreeItem*>( right.internalPointer() );

    //Here we catch the bottom 'track' level
    if( leftItem->level() == rightItem->level() )
    {
        const Meta::TrackPtr leftTrack = Meta::TrackPtr::dynamicCast( leftItem->data() );
        const Meta::TrackPtr rightTrack = Meta::TrackPtr::dynamicCast( rightItem->data() );
        if( !leftTrack.isNull()  && !rightTrack.isNull() )
            return leftTrack->trackNumber() < rightTrack->trackNumber();
    }

    //This should catch everything else
    QVariant leftData = left.data();
    QVariant rightData = right.data();
    if( leftData.canConvert( QVariant::String ) && rightData.canConvert( QVariant::String ) ) {
        return lessThanString( leftData.toString().toLower(), rightData.toString().toLower() );
    }

    warning() << "failed: an unexpected comparison was made";

    //Just in case
    return QSortFilterProxyModel::lessThan( left, right );
}

bool
CollectionSortFilterProxyModel::lessThanString( const QString a, const QString b ) const
{
    int compareIndices[2];
    compareIndices[0] = a.indexOf(QRegExp("\\d"));
    if ( compareIndices[0] == -1 || (compareIndices[1] = b.indexOf(QRegExp("\\d"))) == -1
            || compareIndices[0] != compareIndices[1])
        return QString::localeAwareCompare( a, b ) < 0;

    QString toCompare[2];
    int intToCompare[2];
    toCompare[0] = a.left( compareIndices[0] );
    toCompare[1] = b.left( compareIndices[1] );
    int rv = QString::localeAwareCompare( toCompare[0], toCompare[1] );
    if( rv != 0 ) return rv < 0;

    toCompare[0] = a.mid( compareIndices[0] );
    toCompare[1] = b.mid( compareIndices[1] );
    for( int i = 0; i < 2; ++i )
    {
        compareIndices[i] = toCompare[i].indexOf( QRegExp("\\D") );
        if ( compareIndices[i] == -1 ) compareIndices[i] = toCompare[i].length();
        intToCompare[i] = toCompare[i].left( compareIndices[i] ).toInt();
    }

    rv = intToCompare[0] - intToCompare[1];
    if( rv != 0 ) return rv < 0;

    for( int i = 0; i < 2; ++i )
        toCompare[i] =  toCompare[i].mid( compareIndices[i] );

    return CollectionSortFilterProxyModel::lessThanString( toCompare[0], toCompare[1] );
}

Better:

Index: src/collectionbrowser/CollectionSortFilterProxyModel.h
===================================================================
--- src/collectionbrowser/CollectionSortFilterProxyModel.h      (revision 754278)
+++ src/collectionbrowser/CollectionSortFilterProxyModel.h      (working copy)
@@ -38,6 +38,7 @@

     protected:
         virtual bool lessThan( const QModelIndex &left, const QModelIndex &right ) const;
+        virtual bool lessThanString( const QString a, const QString b ) const;


 };
Index: src/collectionbrowser/CollectionSortFilterProxyModel.cpp
===================================================================
--- src/collectionbrowser/CollectionSortFilterProxyModel.cpp    (revision 754278)
+++ src/collectionbrowser/CollectionSortFilterProxyModel.cpp    (working copy)
@@ -19,11 +19,16 @@

 #include "CollectionSortFilterProxyModel.h"
 #include "CollectionTreeItem.h"
+#include "debug.h"
+#include "QVariant"
+#include "QString"

 CollectionSortFilterProxyModel::CollectionSortFilterProxyModel(  QObject * parent )
  : QSortFilterProxyModel( parent )
 {
     setDynamicSortFilter( true );
+    setSortLocaleAware( true );
+    setSortCaseSensitivity( Qt::CaseInsensitive );
 }


@@ -44,6 +49,7 @@
     CollectionTreeItem *leftItem = static_cast<CollectionTreeItem*>( left.internalPointer() );
     CollectionTreeItem *rightItem = static_cast<CollectionTreeItem*>( right.internalPointer() );

+    //Here we catch the bottom 'track' level
     if( leftItem->level() == rightItem->level() )
     {
         const Meta::TrackPtr leftTrack = Meta::TrackPtr::dynamicCast( leftItem->data() );
@@ -51,7 +57,50 @@
         if( !leftTrack.isNull()  && !rightTrack.isNull() )
             return leftTrack->trackNumber() < rightTrack->trackNumber();
     }
-    return QSortFilterProxyModel::lessThan( left, right ); //Bad idea fallthrough
+
+    //This should catch everything else
+    QVariant leftData = left.data();
+    QVariant rightData = right.data();
+    if( leftData.canConvert( QVariant::String ) && rightData.canConvert( QVariant::String ) ) {
+        return lessThanString( leftData.toString().toLower(), rightData.toString().toLower() );
+    }
+
+    warning() << "failed: an unexpected comparison was made";
+
+    //Just in case
+    return QSortFilterProxyModel::lessThan( left, right );
 }

+bool
+CollectionSortFilterProxyModel::lessThanString( const QString a, const QString b ) const
+{
+    int compareIndices[2];
+    compareIndices[0] = a.indexOf(QRegExp("\\d"));
+    if ( compareIndices[0] == -1 || (compareIndices[1] = b.indexOf(QRegExp("\\d"))) == -1
+            || compareIndices[0] != compareIndices[1])
+        return QString::localeAwareCompare( a, b ) < 0;

+    QString toCompare[2];
+    int intToCompare[2];
+    toCompare[0] = a.left( compareIndices[0] );
+    toCompare[1] = b.left( compareIndices[1] );
+    int rv = QString::localeAwareCompare( toCompare[0], toCompare[1] );
+    if( rv != 0 ) return rv < 0;
+
+    toCompare[0] = a.mid( compareIndices[0] );
+    toCompare[1] = b.mid( compareIndices[1] );
+    for( int i = 0; i < 2; ++i )
+    {
+        compareIndices[i] = toCompare[i].indexOf( QRegExp("\\D") );
+        if ( compareIndices[i] == -1 ) compareIndices[i] = toCompare[i].length();
+        intToCompare[i] = toCompare[i].left( compareIndices[i] ).toInt();
+    }
+
+    rv = intToCompare[0] - intToCompare[1];
+    if( rv != 0 ) return rv < 0;
+
+    for( int i = 0; i < 2; ++i )
+        toCompare[i] =  toCompare[i].mid( compareIndices[i] );
+
+    return CollectionSortFilterProxyModel::lessThanString( toCompare[0], toCompare[1] );
+}

Thanks for the patch! But can you please attach it to the bug report, not as a text dump?

Use the command:
svn diff > mypatch.diff

then upload the diff file.

Cheers

Created attachment 22777 [details]
Fixes the sorting behaviour

SVN commit 876025 by seb:

Collection browser: Use an intelligent natural sort algorithm which isn't necessarily a lexicographical comparison.
Eg: "Symphony 2" < "Symphony 10"
Patch by Nicholas Wilson <nicholas@nickcwilson.co.uk>
BUG: 154408

@nick: sorry it took so long to apply this patch. a combination of new years and me being overseas directly after that obviously contributing factors to overlooking your patch :)


 M  +63 -1     CollectionSortFilterProxyModel.cpp  
 M  +1 -2      CollectionSortFilterProxyModel.h  
 M  +3 -3      CollectionTreeItemModelBase.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=876025

SVN commit 876026 by seb:

Update changelog
CCBUG: 154408


 M  +3 -0      ChangeLog  


WebSVN link: http://websvn.kde.org/?view=rev&revision=876026