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

feat: add support for alternative ordering strategies #424

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

iiian
Copy link
Contributor

@iiian iiian commented Jan 1, 2023

Addresses #341.

Adds a default hotkey of ctrl+o to flip between A-Z sort order and numerical sort-by-size-desc on the right-side pane.

The approach to implementing this feature was to find all of the places in code that enforce the order in which file-system nodes in the selected layer are displayed in the buffer window. Three methods were identified: FileTree.StringBetween, FileNode.VisitDepthParentFirst and FileNode.VisitDepthChildFirst. These locations were taking the FileNode's property Children map[string]*FileNode and performing sort.Strings(keys) in order to implement alphanumeric sorting.

These locations have been unified with OrderStrategy.orderKeys(map[string]*FileNode). A factory function, GetSortOrderStrategy, tees up the corresponding ordering strategy given an enum, which it then propagates down to the three mentioned methods.

Additionally, a Size int64 field has been attached to FileNode in order to cache information on directory/file size for use in the sort-by-size-desc strategy (with a simple extraction refactor of the FileNode.MetadataString method.

Happy New Year! 🥂

@wagoodman wagoodman deleted the branch wagoodman:master July 6, 2023 15:24
@wagoodman wagoodman closed this Jul 6, 2023
@wagoodman
Copy link
Owner

Sorry! I didn't mean to close this, I renamed the master branch to main... I should have switched the target here first.

@wagoodman
Copy link
Owner

@iiian if you'd like to re-open the PR against main I'd be happy to take a look

@wagoodman wagoodman reopened this Jul 7, 2023
@wagoodman
Copy link
Owner

@iiian I recreated the master branch temporarily to get this in (I didn't think of that earlier). Just force pushed to rebase.

@wagoodman wagoodman merged commit 6f20438 into wagoodman:master Jul 7, 2023
6 checks passed
@iiian
Copy link
Contributor Author

iiian commented Jul 7, 2023

Rad!

@Potherca
Copy link

FYI: This closes #89

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.

None yet

3 participants