[KDirOperator] Use more human-readable sort order descriptions
ClosedPublic

Authored by meven on Jul 26 2019, 2:15 PM.

Details

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
meven created this revision.Jul 26 2019, 2:15 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJul 26 2019, 2:15 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Jul 26 2019, 2:15 PM
meven edited the test plan for this revision. (Show Details)Jul 26 2019, 2:19 PM

Code looks sensible, haven't tried it yet

src/filewidgets/kdiroperator.cpp
1979

Remove commented code

meven updated this revision to Diff 62611.Jul 26 2019, 2:40 PM

Remove dead commented code

meven updated this revision to Diff 62612.Jul 26 2019, 2:41 PM
meven marked an inline comment as done.

Small formatting correction

meven added inline comments.Jul 26 2019, 2:41 PM
src/filewidgets/kdiroperator.cpp
1979

Thanks ;)

meven marked an inline comment as done.Jul 26 2019, 2:41 PM

I tried this out but I'm not actually seeing the menu items in the open/save dialogs change at all. :/

meven added a comment.Jul 26 2019, 3:58 PM

I tried this out but I'm not actually seeing the menu items in the open/save dialogs change at all. :/

Have you tried with gwenview ?
For some reason, it does not work in kate but works in gwenview after installing kio, not event recompiling gwenview.

I have the same issue, I use the test app kfilewidgettest_saving_gui in kio tests dir for my tests usually.

I guess the old kio is kept in memory somewhere in kdeinit somewhere since kate uses it but not gwenview.
I have tried relaunching kdeinit5 but this does not seem to work.

ngraham accepted this revision.Jul 26 2019, 4:36 PM

Yes, it does work in Gwenview. How odd.

Anyway, not an issue with your patch, for sure. Patch LGTM!

This revision is now accepted and ready to land.Jul 26 2019, 4:36 PM
meven updated this revision to Diff 62628.Jul 27 2019, 7:12 AM

Remove unecessary call

ngraham added inline comments.Jul 27 2019, 2:44 PM
src/filewidgets/kdiroperator.cpp
1992

Why change this? kdiroperator should definitely be ported to use the new connect syntax, but it should be done all at once in its own patch, not one at a time.

meven marked an inline comment as done.Jul 27 2019, 5:06 PM
meven added inline comments.
src/filewidgets/kdiroperator.cpp
1992

_k_slotSortReversed parameter means whether or not the sorting is reversed.
Before this change, the descending action was a check box meaning its value should be the parameter to _k_slotSortReversed.
But now the descending action is a radio button meaning when triggered it should always pass false to _k_slotSortReversed.

A closure was the simple course of action to implement this, and could be reused as is for the ascending action as well.
Onyl the new syntax allows closures to be used as slots.

meven marked 2 inline comments as done.Jul 27 2019, 5:07 PM

Shouldn't it be "Newest First" instead of "Newest first", to be consistent with other menu items (i.e. "Folders First", etc.)?

meven added a comment.Jul 29 2019, 6:51 AM

Shouldn't it be "Newest First" instead of "Newest first", to be consistent with other menu items (i.e. "Folders First", etc.)?

Now that you mention it, I think so. I took the code from D22006

meven added a comment.Aug 1 2019, 5:32 PM

I will merge this beginning of next week, if no one objects.

This revision was automatically updated to reflect the committed changes.