Sort directory listings with directories before files #27

Russtopia · 2021-08-11T21:09:29Z

Russtopia commented

2021-08-11 21:09:29 +00:00

First-time contributor

This patch improves the readability of directory listings, by grouping all directories together at the top, then files. The other sort criteria are otherwise still applied (ie., all dirs sorted by Date, then files sorted by Date, or whatever according to the config file).

Perhaps it could be refactored to avoid the almost-repeated logic in each sort case; I can look at that if you think it's gross 😃

This patch improves the readability of directory listings, by grouping all directories together at the top, then files. The other sort criteria are otherwise still applied (ie., all dirs sorted by Date, then files sorted by Date, or whatever according to the config file). Perhaps it could be refactored to avoid the almost-repeated logic in each sort case; I can look at that if you think it's gross 😃

Russtopia added 1 commit 2021-08-11 21:09:30 +00:00

a4873da190 Sort directory listings with directories before files

Russtopia commented

2021-09-29 05:50:53 +00:00

First-time contributor

Bump (promise I won't do it again, just wondering if this repo is alive at all)

Ghost commented

2021-09-29 06:00:13 +00:00

First-time contributor

I prefer files and dirs mixed.
Am I really the last living creature with this preference?

I prefer files and dirs mixed. Am I really the last living creature with this preference?

Russtopia commented

2021-10-10 05:42:17 +00:00

First-time contributor

I prefer files and dirs mixed.
Am I really the last living creature with this preference?

Fair enough :) I can rework the patch to make it a preference when the server is launched, to allow either behaviour.

> I prefer files and dirs mixed. > Am I really the last living creature with this preference? Fair enough :) I can rework the patch to make it a preference when the server is launched, to allow either behaviour.

👍 1

Russtopia force-pushed sort-dirs-first from a4873da190 to 2dcaf941e4

2021-10-10 06:04:13 +00:00

Compare

Russtopia commented

2021-10-10 06:07:03 +00:00

First-time contributor

Added new DirectoriesFirst config flag, defaults to false to preserve current behaviour on master

Added new `DirectoriesFirst` config flag, defaults to `false` to preserve current behaviour on `master`

Russtopia referenced this pull request

2022-03-13 03:20:15 +00:00

Directory listings mix files and subdirectory items #30

Russtopia commented

2022-06-20 09:11:53 +00:00

First-time contributor

Bump. 8 months, anyone have an opinion on this config feature?

solderpunk commented

2023-01-28 16:31:34 +00:00

Sorry for the phenomenally slow response time on this, and thanks for the help!

I'm happy to include this functionality, as something which can be toggled on or off by the user. I do think it could be done a little better. If, after over a year, you don't really care anymore and want to tell me to take a hike, then that's perfectly reasonable (really!), and I'll accept the PR as it is and do some tidying myself.

But if you are still willing to work on this: yeah, I think the repeated logic is gross, like you said, but thankfully also easily avoidable: sort.SliceStable() is a stable sort, i.e. if two items compare as equal it will leave them in their original order. So you should be able to just do one sort by name/size/time and then a second sort after that which puts directories before files. Much neater!

Also, all the other config options related to the directory list generation begin with Directory and it'd be kinda nice to keep that consistency. Maybe this option could be DirectoryDirsFirst?

Sorry for the phenomenally slow response time on this, and thanks for the help! I'm happy to include this functionality, as something which can be toggled on or off by the user. I do think it could be done a little better. If, after over a year, you don't really care anymore and want to tell me to take a hike, then that's perfectly reasonable (really!), and I'll accept the PR as it is and do some tidying myself. But if you are still willing to work on this: yeah, I think the repeated logic is gross, like you said, but thankfully also easily avoidable: `sort.SliceStable()` is a stable sort, i.e. if two items compare as equal it will leave them in their original order. So you should be able to just do one sort by name/size/time and then a second sort after that which puts directories before files. Much neater! Also, all the other config options related to the directory list generation begin with `Directory` and it'd be kinda nice to keep that consistency. Maybe this option could be `DirectoryDirsFirst`?

solderpunk commented

2023-02-05 14:13:35 +00:00

Hi there. I went ahead and merged this PR (didn't use the Tildegit interface to do it as I had pushed other changes which prevented an automatic merge, so I will close this manually), and then refactored it as above to do the sorting in two passes. It seems to work nicely and is a lot more concise.

Sorry if this seems impatient, I know that I made you wait years and then only gave you weeks. I'm just trying to give Molly Brown some much needed love and move through the long list of neglected issues quickly.

Thanks again for this contribution, I agree that turning this option on improves the readability of the directory listings!

Hi there. I went ahead and merged this PR (didn't use the Tildegit interface to do it as I had pushed other changes which prevented an automatic merge, so I will close this manually), and then refactored it as above to do the sorting in two passes. It seems to work nicely and is a lot more concise. Sorry if this seems impatient, I know that I made you wait years and then only gave you weeks. I'm just trying to give Molly Brown some much needed love and move through the long list of neglected issues quickly. Thanks again for this contribution, I agree that turning this option on improves the readability of the directory listings!

solderpunk closed this pull request

2023-02-05 14:13:40 +00:00

Russtopia commented

2023-09-07 01:58:39 +00:00

First-time contributor

Hi there. I went ahead and merged this PR (didn't use the Tildegit interface to do it as I had pushed other changes which prevented an automatic merge, so I will close this manually), and then refactored it as above to do the sorting in two passes. It seems to work nicely and is a lot more concise.

Sorry if this seems impatient, I know that I made you wait years and then only gave you weeks. I'm just trying to give Molly Brown some much needed love and move through the long list of neglected issues quickly.

Thanks again for this contribution, I agree that turning this option on improves the readability of the directory listings!

Hi, hah I am just as guilty as you I suppose for not paying attention 😃 - glad to be able to contribute in a minor way. I see there was a recent refactor on the dirlist code; looks like that repetitive code was nicely done away with.

Cheers.

> Hi there. I went ahead and merged this PR (didn't use the Tildegit interface to do it as I had pushed other changes which prevented an automatic merge, so I will close this manually), and then refactored it as above to do the sorting in two passes. It seems to work nicely and is a lot more concise. > > Sorry if this seems impatient, I know that I made you wait years and then only gave you weeks. I'm just trying to give Molly Brown some much needed love and move through the long list of neglected issues quickly. > > Thanks again for this contribution, I agree that turning this option on improves the readability of the directory listings! Hi, hah I am just as guilty as you I suppose for not paying attention :smiley: - glad to be able to contribute in a minor way. I see there was a recent refactor on the dirlist code; looks like that repetitive code was nicely done away with. Cheers.

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: solderpunk/molly-brown#27