Add sortable columns to session manager dialog, and remove previous sorting code
AcceptedPublic

Authored by rrosch on Wed, Feb 5, 9:57 PM.

Details

Summary

I really like the new session manager dialog (session chooser) but when I went to click on the column headers to sort, it didn't work. So I wondered how hard it would be to implement and just went for it.

I know the diff is supposed to be done with arc, but I don't yet have that all set up, and I wanted to get you the patch over as soon as possible. I hope that's ok.

One thing that does not work properly is the sorting by number of open files, since that requires numeric sort which was beyond my current abilities, maybe someone else can take that up for the next round, once this simple patch is in.

I also corrected a couple spelling mistakes, which accounts for the extra lines.

Test Plan

Apply patch (should apply cleanly to most recent master).
Compile.
Run kate.

Diff Detail

Repository
R40 Kate
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rrosch created this revision.Wed, Feb 5, 9:57 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald TranscriptWed, Feb 5, 9:57 PM
rrosch requested review of this revision.Wed, Feb 5, 9:57 PM
rrosch added reviewers: dhaumann, cullmann.

I like the idea of the patch, could you add a screenshot?

rrosch added a comment.Thu, Feb 6, 9:55 PM

As requested. You will notice that the button to sort has been removed and sorting is done only by clicking the header bars.

I'd be fine with committing this. Still, there very clearly is large room to improve this further.

kate/session/katesessionchooseritem.h
38

I believe that seconds are a bit over engineered. In fact, it would be nice to do the sorting based on the timestamp as now, but visually the English format does not fit very well in other languages. So this should be translated via i18n. But then we loose the correct sorting.

So for now this patch is ok, but in the long run we should do better.

For instance, I think it would be much more user friendly to have "Yesterday", "3 days ago", ... instead of the datetime right now.

Would you be interested to look into this as well?

I agree that sorting needs to be improved, however I don't have the skills right now (or time) to implement them.
Ideally the number of open files should sort numerically, and the last opened should sort via timestamp, to allow for locale-based display. But both of these require the same intrinsic change that I don't know how to do yet (the class is chosen by the .ui file, and a new overloaded class with the proper sorting functions would need to be implemented). But with this first set of changes in this patch we can get something out there for people to use now. Someone else might come later to do that, just as I did for this part.

Thanks again for taking a look, I hope it gets accepted, I love kate as a text editor, so much to love about it!

cullmann accepted this revision.Tue, Feb 11, 7:04 PM

Let's to with this then for the moment.
If you find time to improve on this, further patches are welcome!
Thanks for taking care! (and that you like Kate ;=)

This revision is now accepted and ready to land.Tue, Feb 11, 7:04 PM

Btw., for future patches, better use merge requests at https://invent.kde.org/kde/kate, thanks.

This revision was automatically updated to reflect the committed changes.
cullmann reopened this revision.Tue, Feb 11, 8:42 PM

Hmm, this doesn't build due to use of an obsolete function.

I quick-fixed this now via:

https://invent.kde.org/kde/kate/commit/54b3fd74f74f604932b93f46aeaba4e8f5e9a2aa

Can you check if that still works as intended?

This revision is now accepted and ready to land.Tue, Feb 11, 8:42 PM