D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. Done. :) REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D15598 To: ngraham, apol, #plasma Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

wbauer added a comment. In D15598#328967 , @ngraham wrote: > Darn, looks like arc didn't notice my rebase on 5.14. Oh well, I guess this'll go into 5.15. Well, you could cherry-pick it into the 5.14 branch (using `git cherry-pick xxx`).

D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. Darn, looks like arc didn't notice my rebase on 5.14. Oh well, I guess this'll go into 5.15. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D15598 To: ngraham, apol, #plasma Cc: wbauer, cfeck, broulik, plasma-devel, ragreen

D15598: Sort icon and cursor themes case-insensitively

This revision was automatically updated to reflect the committed changes. Closed by commit R99:b943d4ac6e2c: Sort icon and cursor themes case-insensitively (authored by ngraham). REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15598?vs=41983&i

D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. I'll keep `setSortRole(Qt::DisplayRole)` for explicitness if nobody objects. REPOSITORY R99 KDE Gtk Configuration Tool BRANCH sort-icon-and-cursor-themes-case-insensitively (branched from master) REVISION DETAIL https://phabricator.kde.org/D15598 To: ngraham,

D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. @wbauer, thanks for the hint. I've tested this with `xcursor-themes` and verified that it works for me too. REPOSITORY R99 KDE Gtk Configuration Tool BRANCH sort-icon-and-cursor-themes-case-insensitively (branched from master) REVISION DETAIL https://phabricat

D15598: Sort icon and cursor themes case-insensitively

wbauer added a comment. Works (and looks) fine for me now. Btw, it wouldn't really be necessary to call `setSortRole(Qt::DisplayRole)`, as the default value is `Qt::DisplayRole` anyway. (but of course doing it is not wrong either...) REPOSITORY R99 KDE Gtk Configuration Tool BRANCH

D15598: Sort icon and cursor themes case-insensitively

apol accepted this revision. apol added a comment. This revision is now accepted and ready to land. LGTM, not sure why sort() doesn't work though. REPOSITORY R99 KDE Gtk Configuration Tool BRANCH sort-icon-and-cursor-themes-case-insensitively (branched from master) REVISION DETAIL http

D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. In D15598#328774 , @wbauer wrote: > Maybe it would be better to derive CursorThemesModel and IconThemesModel from QSortFilterProxyModel instead of QStandardItemModel, and keep the previous fix? I had that idea to

D15598: Sort icon and cursor themes case-insensitively

ngraham updated this revision to Diff 41983. ngraham added a comment. Address review comments REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15598?vs=41956&id=41983 BRANCH sort-icon-and-cursor-themes-case-insensitively (branched from ma

D15598: Sort icon and cursor themes case-insensitively

wbauer added a comment. In D15598#328791 , @broulik wrote: > > Although, I think that would need to be done every time the data changes. > > `QSortFilterProxyModel` does that automatically You mean it will keep the data sorted after ch

D15598: Sort icon and cursor themes case-insensitively

broulik added a comment. > Although, I think that would need to be done every time the data changes. `QSortFilterProxyModel` does that automatically REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D15598 To: ngraham, apol, #plasma Cc: wbauer, cf

D15598: Sort icon and cursor themes case-insensitively

wbauer added a comment. In D15598#328747 , @wbauer wrote: > I suppose that's because, according to the QSortFilterProxyModel docs, `At this point, neither sorting nor filtering is enabled; the original data is displayed in the view.` > Appare

D15598: Sort icon and cursor themes case-insensitively

wbauer added a comment. Well, I tried this patch (applied upon 4.13.90a) and it doesn't seem to change anything. I still get icon and cursor themes starting with a lower-case character sorted after all those with an upper-case character. I suppose that's because, according to the QSort

D15598: Sort icon and cursor themes case-insensitively

ngraham updated this revision to Diff 41956. ngraham added a comment. Rebase on Plasma/5.14 branch REPOSITORY R99 KDE Gtk Configuration Tool CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15598?vs=41925&id=41956 BRANCH sort-icon-and-cursor-themes-case-insensitively (branched fr

D15598: Sort icon and cursor themes case-insensitively

cfeck added a comment. If you would need to process the data sorted, you would already sort the model. But here, it is only sorted for display, and that's exactly the use case for the proxy model. REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D155

D15598: Sort icon and cursor themes case-insensitively

ngraham added a comment. Data structures are not really my forte, but I had a look. It seems possible but rather complicated due to the way the models populate and update themselves and return data. I'd like to see what @apol has to say. Is there some big disadvantage with this approach

D15598: Sort icon and cursor themes case-insensitively

broulik added a comment. Can't we fix the `CursorThemesModel` and `IconThemesModel` to already populate themselves correctly? REPOSITORY R99 KDE Gtk Configuration Tool REVISION DETAIL https://phabricator.kde.org/D15598 To: ngraham, apol, #plasma Cc: broulik, plasma-devel, ragreen, Pitel

D15598: Sort icon and cursor themes case-insensitively

ngraham created this revision. ngraham added reviewers: apol, Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY Stick a QSortFilterProxyModel between the model and view and tel it to sort case-insensitive