Bug 226599 - [PATCH] Wrong sorting by name in file browser: files and folders are mixed
Summary: [PATCH] Wrong sorting by name in file browser: files and folders are mixed
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: File Browser (show other bugs)
Version: 2.3-GIT
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 236188 237509 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-02-12 22:35 UTC by Kostya Sha
Modified: 2010-05-14 15:34 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.1


Attachments
Proposal for ordering folders first, the file, both case insensitive (1.71 KB, patch)
2010-05-06 16:43 UTC, dystoptic
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kostya Sha 2010-02-12 22:35:06 UTC
All time was sorting like "all catalogs by name", then "all files by name", now  files and catalogs are combined and sorted by name.
It looks like:
 a_folder_1
 b_folder_2
 c_file_1
 d_folder_3
 e_file_2
But should be:
 a_folder_1
 b_folder_2
 d_folder_3
 c_file_1
 e_file_2
Comment 1 Myriam Schweingruber 2010-02-12 23:28:45 UTC
Well, the letter i precedes the letter o, so the sorting is certainly correct:
a_fo...
b_fo...
c_fi...
d_fo...
e_fi...

think again :)
Comment 2 Myriam Schweingruber 2010-02-12 23:30:11 UTC
Oops, I should not triage bugs late in the evening...
Comment 3 Kostya Sha 2010-02-13 19:10:10 UTC
Please change to the right version.
Reproduced for 74aacb939d28d7089f671b46d2384f2b50f1280b 13 Feb 2010 03:22:03
Comment 4 Myriam Schweingruber 2010-02-13 20:46:55 UTC
It is already set to the correct version.
Comment 5 Sven Krohlas 2010-05-03 17:40:34 UTC
*** Bug 236188 has been marked as a duplicate of this bug. ***
Comment 6 dystoptic 2010-05-06 16:43:10 UTC
Created attachment 43311 [details]
Proposal for ordering folders first, the file, both case insensitive
Comment 7 dystoptic 2010-05-06 16:45:14 UTC
Comment on attachment 43311 [details]
Proposal for ordering folders first, the file, both case insensitive

I have created a patch, that would fulfill the requirement. Would be nice if it would be integrated soon.
Comment 8 Sven Krohlas 2010-05-07 13:20:33 UTC
<nhn> I could not replicate the issue on my system
<nhn> so I never even got around to actually applying/testing the patch

I can see the issue here.

Atm I'm running a patched version of Amarok including this proposed patch and it seems to fix the problem. I don't know the file browser code at all, but if the patch looks good to you I'd say we should check it in.
Comment 9 Sven Krohlas 2010-05-07 15:10:24 UTC
commit 0a559dc4029fcfc59ac2a881d3d7f14d81303d01
Author: Sven Krohlas <sven@getamarok.com>
Date:   Fri May 7 15:03:43 2010 +0200

    File browser: show folders first, then files.
    Patch by <dystopticus@gmx.net>,  reviewed by nhn, tested by me.
    
    BUG: 226599

diff --git a/ChangeLog b/ChangeLog
index 4cf3f64..c87ffec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,6 +7,8 @@ VERSION 2.3.1
   CHANGES:
 
   BUGFIXES:
+    * File browser: show folders first, files afterwards. Patch by
+      <dystopticus@gmx.net>. (BR 226599)
     * Queued track's contextual menu entry about dequeueing was written wrong
       and misleading. (BR 235047)
     * Custom color setting in the On Screen Display was only applied after
diff --git a/src/browsers/filebrowser/MimeTypeFilterProxyModel.cpp b/src/browsers/filebrowser/MimeTypeFilterProxyModel.cpp
index 448ad85..92ae889 100644
--- a/src/browsers/filebrowser/MimeTypeFilterProxyModel.cpp
+++ b/src/browsers/filebrowser/MimeTypeFilterProxyModel.cpp
@@ -46,3 +46,27 @@ MimeTypeFilterProxyModel::filterAcceptsRow( int source_row, const QModelIndex& s
     return false;
 }
 
+bool
+MimeTypeFilterProxyModel::lessThan( const QModelIndex& left, const QModelIndex& right ) const
+{
+    // get the KFileItems
+    QVariant qvarLeft = left.data( KDirModel::FileItemRole );
+    QVariant qvarRight = right.data( KDirModel::FileItemRole );
+
+    KFileItem itemLeft = qvarLeft.value<KFileItem>();
+    KFileItem itemRight = qvarRight.value<KFileItem>();
+
+    // if both items are directories or files, then compare (case insensitive) on name
+    if( itemLeft.isDir() == itemRight.isDir()) {
+        return itemLeft.name(true) < itemRight.name(true);
+    }
+
+    // directories come before files
+    if (itemLeft.isDir()) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+
diff --git a/src/browsers/filebrowser/MimeTypeFilterProxyModel.h b/src/browsers/filebrowser/MimeTypeFilterProxyModel.h
index 6309f47..938b0c2 100644
--- a/src/browsers/filebrowser/MimeTypeFilterProxyModel.h
+++ b/src/browsers/filebrowser/MimeTypeFilterProxyModel.h
@@ -39,6 +39,7 @@ public:
 
 protected:
     virtual bool filterAcceptsRow( int source_row, const QModelIndex& source_parent ) const;
+    virtual bool lessThan( const QModelIndex& left, const QModelIndex& right ) const;
 
 private:
     QStringList m_mimeList;
Comment 10 Sven Krohlas 2010-05-14 00:47:18 UTC
*** Bug 237509 has been marked as a duplicate of this bug. ***
Comment 11 Sven-Erik Petermann 2010-05-14 13:10:29 UTC
I applied the patch to 2.3.0.90, now files and directories are separated but they are sorted in reverse alphabetical order (Z-A), also the sorting order doesn't change when clicking on column headers.
Comment 12 Sven Krohlas 2010-05-14 13:24:50 UTC
Works fine here. Anyway, this would be a new issue -> new bug report (if it is a problem your you in git), no bug recycling.
Comment 13 Sven-Erik Petermann 2010-05-14 15:34:25 UTC
Sorry for my last post, the problem is not caused by this patch, indeed.