Fix sorting
ClosedPublic

Authored by rthomsen on Dec 19 2016, 7:37 PM.

Details

Summary

Commit 7781d6ef794a broke sorting in the sense that folders are no longer sorted before files. This is due to KRecursiveFilterProxyModel's implementation of sort() being used instead of ArchiveModel's.

We now subclass KRecursiveFilterProxyModel into ArchiveFilterSortModel so we can re-implement lessThan() and move the relevant code from ArchiveModel::sort().

Test Plan

Try opening archives of various types and check that sorting for all columns works as expected.

Diff Detail

Repository
R36 Ark
Lint
Lint Skipped
Unit
Unit Tests Skipped
rthomsen updated this revision to Diff 9181.Dec 19 2016, 7:37 PM
rthomsen retitled this revision from to Fix sorting.
rthomsen updated this object.
rthomsen edited the test plan for this revision. (Show Details)
rthomsen added a reviewer: elvisangelaccio.
rthomsen set the repository for this revision to R36 Ark.
rthomsen added a project: Ark.
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptDec 19 2016, 7:37 PM
elvisangelaccio requested changes to this revision.Dec 19 2016, 8:08 PM
elvisangelaccio edited edge metadata.
elvisangelaccio added inline comments.
part/archivesortfiltermodel.cpp
34–46 ↗(On Diff #9181)

Add a getter in ArchiveModel for this map? So we don't have to keep it in sync if we change it

part/archivesortfiltermodel.h
39 ↗(On Diff #9181)

No need to store the model I think, using sourceModel() + qobject_cast should be enough

This revision now requires changes to proceed.Dec 19 2016, 8:08 PM
rthomsen updated this revision to Diff 9185.Dec 19 2016, 8:34 PM
rthomsen edited edge metadata.

Fix bug

rthomsen updated this revision to Diff 9188.Dec 19 2016, 8:49 PM
rthomsen edited edge metadata.

Implement Elvis' suggestions.

rthomsen marked 2 inline comments as done.Dec 19 2016, 8:49 PM
elvisangelaccio requested changes to this revision.Dec 19 2016, 8:59 PM
elvisangelaccio edited edge metadata.

Please also drop ArchiveEntry::returnDirEntries(), it was only used in sort() and now we don't need it anymore

part/archivesortfiltermodel.cpp
24 ↗(On Diff #9181)

Unused include

part/archivesortfiltermodel.h
27 ↗(On Diff #9181)

Unused forward declaration

This revision now requires changes to proceed.Dec 19 2016, 8:59 PM
rthomsen updated this revision to Diff 9189.Dec 19 2016, 9:11 PM
rthomsen edited edge metadata.

Fix more of Elvis' comments.

rthomsen marked 2 inline comments as done.Dec 19 2016, 9:11 PM
elvisangelaccio accepted this revision.Dec 19 2016, 9:13 PM
elvisangelaccio edited edge metadata.
This revision is now accepted and ready to land.Dec 19 2016, 9:13 PM
This revision was automatically updated to reflect the committed changes.