Can't we fix the CursorThemesModel and IconThemesModel to already populate themselves correctly?

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 in this patch? Negative memory/performance implications?

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.

Well, I tried this patch (applied upon 5.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 QSortFilterProxyModel docs, At this point, neither sorting nor filtering is enabled; the original data is displayed in the view.
Apparently you need to explicitly call sort() on the proxy model...
See http://doc.qt.io/qt-5/qsortfilterproxymodel.html#details

Regardless of that, using a QSortFilterProxyModel would render the previous fix (https://phabricator.kde.org/R99:7b56c23870798adbeb3f110eacae7f1020dcad6f) useless (as I see it there's no need to sort the model if it goes through QSFPM anyway), so it should be reverted here IMHO.

Btw, the lower-case cursor themes I have installed ("handhelds", "redglass", "whiteglass") come from the (openSUSE) package xcursor-themes.
From the package's README:

This is a default set of cursor themes for use with libXcursor,
originally created for the XFree86 Project, and now shipped as part
of the X.Org software distribution.
...
The master development code repository can be found at:

git://anongit.freedesktop.org/git/xorg/data/cursors
 
http://cgit.freedesktop.org/xorg/data/cursors

Although, just renaming or copying an existing cursor theme folder (they are in /usr/share/icons/) accordingly should suffice for testing as well.

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.
Apparently you need to explicitly call sort() on the proxy model...

Yes, adding cursorsProxyModel->sort(0); and iconsProxyModel->sort(0); here seems to "fix" the sorting.
Although, I think that would need to be done every time the data changes. (can that happen later on? I haven't looked at all the code...)

Maybe it would be better to derive CursorThemesModel and IconThemesModel from QSortFilterProxyModel instead of QStandardItemModel, and keep the previous fix?
Or maybe reimplement sort() in these classes, the base implementation of QStandardItemModel just does nothing if I read the docs correctly (that also means that the previous fix actually did nothing either).

Just some thoughts, though.

Although, I think that would need to be done every time the data changes.

QSortFilterProxyModel does that automatically

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 changes if you call sort() once?
It obviously doesn't sort it automatically by default.

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 too and gave it a shot, but got into the weeds quickly and abandoned the effort because it seemed too challenging. If this seems like the correct approach, I can give it another go.

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...)

@wbauer, thanks for the hint. I've tested this with xcursor-themes and verified that it works for me too.

I'll keep setSortRole(Qt::DisplayRole) for explicitness if nobody objects.

Darn, looks like arc didn't notice my rebase on 5.14. Oh well, I guess this'll go into 5.15.

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).
Not sure if it's worth it though...

Done. :)