----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128574/#review101143 -----------------------------------------------------------
I can confirm that the patch indeed resolves the natural sorting issue in KDirSortFilterProxyModel, which is a logical bug (an omission) introduced at some point during the KF5 kio port. It's been there since the initial git import of KF5 kio (as per [1]). However, the patch is a half-measure as it doesn't fully address the issue at hand. Essentially it enforces natural sorting in the KDirSortFilterProxyModel class, as the relevant configuration option (kdeglobals config file -> [KDE] config group -> "NaturalSorting" config entry) is not accessible (i.e. editable) to the user by means of a GUI. It used to, but this is no longer case (as per commit [2]). See, back in KDE 4.x, Dolphin and Gwenview (and essentially any code relying on KDirSortFilterProxyModel) shared the same natural sorting configuration (by means of KGlobalSettings::naturalSorting() [3], which btw has been moved to kdelibs4support in KF5 [4]) and this was, for better or worse, configurable through Dolphin. Per the above mentioned commit though ([2]), Dolphin went a different way, so the "NaturalSorting" config entry in kdeglobals is only configurable by hand editing the config file. On a side note, it's also worth mentioning that Dolphin never made use of KDirSortFilterProxyModel, it performs its own natural sorting. tl;dr: In KDE 4.x everyone relied on KGlobalSettings::naturalSorting() for natural sorting configuration, which allowed for uniform sorting behaviour in different applications. This is no longer the case, so from a casual user's perspective, this patch would only hardcode natural sorting in Gwenview (the main user of KDirSortFilterProxyModel natural sorting). The patch as it stands is not wrong, but the underlying issue is broader. I hope I didn't omit anything, I have been juggling with pre- and post-KF5 code trying to find out what went wrong in the way. A design decision has to be made on the issue. For more info check the related bug report. [1] https://cgit.kde.org/kio.git/log/src/filewidgets/kdirsortfilterproxymodel.cpp [2] https://cgit.kde.org/dolphin.git/commit/?id=fdb5c0d33e38e9d5fedc821c586bbb5ca8a0d2a5 [3] https://lxr.kde.org/source/kde/kdelibs/kdeui/kernel/kglobalsettings.cpp?v=stable-qt4#0775 [4] https://lxr.kde.org/source/frameworks/kdelibs4support/src/kdeui/kglobalsettings.cpp#0552 - Konstantinos Smanis On Aug. 2, 2016, 1:08 p.m., Mathis Beer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128574/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2016, 1:08 p.m.) > > > Review request for KDE Frameworks, Pieter David, David Faure, and Harald > Sitter. > > > Bugs: 343452 > https://bugs.kde.org/show_bug.cgi?id=343452 > > > Repository: kio > > > Description > ------- > > KDirSortFilterProxyModel is advertised in the header as performing a "natural > sort", ie. "7 8 9 10 11", instead of a lexical "10 11 7 8 9". > However, as far as I can tell this was never true from the start, since the > collator responsible for the actual sorting did not have the requisite > numeric mode enabled, and this setting has always been off by default as far > back as I can find docs for it (Qt 5.2). > > (Dolphin, which offers "natural sort", did not run into this issue because it > does not actually use KDirSortFilterProxyModel.) > > > Diffs > ----- > > src/filewidgets/kdirsortfilterproxymodel.cpp 89505ac > > Diff: https://git.reviewboard.kde.org/r/128574/diff/ > > > Testing > ------- > > Create a folder with three images, "File 1.png", "File 10.png", "File 2.png". > View the folder in Gwenview with sort order set to "Name". The sort order is > "File 1.png, File 10.png, File 2.png". > Apply patch. > View the folder in Gwenview with sort order set to "Name". The sort order is > "File 1.png, File 2.png, File 10.png". > > > Thanks, > > Mathis Beer > >
