Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds keybinding for 'c' to toggle sorting by number of items #188

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

gosuwachu
Copy link
Contributor

Adds keybinding for 'c' to toggle sorting by number of items. I have used human_format to convert numbers to more human readable form.

Number of items in each directory is collected the same way as the size was done before during file system traversal. I have updated the tests to ensure the counts are calculated correctly.

Sorting by number of items + current sorting mode in the footer:
Screenshot 2023-12-08 at 22 16 47

* copy when possible
* mild function renames
* restore previous sorting direction precedence
Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, it's a very nice feature to have!

I have made a few changes if you want to take a look.

Memory consumption doesn't change significantly compared to the version without counts, so tracking it seems mostly free.

MTimeAscending => MTimeDescending,
MTimeDescending => MTimeAscending,
_ => MTimeAscending,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the default sort-direction without commenting on it seems like an accident to me.

*self = match self {
CountAscending => CountDescending,
CountDescending => CountAscending,
_ => CountAscending,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's because Size starts descending, even though the default value suggests otherwise?

I set it to be Descending now consistently, which is nice from a UX point of view (as it's what feels natural).

Those don't have a count in that sense, and that needs to be visible
when sorting.

Now files, which naturally have no count, will sort alphabetically instead
which seems most natural, as they are technically out of scope.
@Byron Byron merged commit 45ccb7c into Byron:main Dec 9, 2023
2 checks passed
@gosuwachu gosuwachu deleted the count_sort branch December 9, 2023 14:01
@gosuwachu
Copy link
Contributor Author

Thanks, looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants