fileserver: Implement natural sort for browse templates #7290
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For files with regions of numbers and other characters, it's intuitive to have the numbers sorted by increasing numeric value rather than by ASCII code. (Jeff Atwood has a nice article here: https://blog.codinghorror.com/sorting-for-humans-natural-sort-order/) For example, the listing for a directory containing `foo1`, `foo2`, and `foo10` would sort in that order, rather than putting `foo10` between `foo1` and `foo2`. Closes caddyserver#7226
|
I've tested the implementation for my own web/fileserver. With normal version number style filenames, things look good, but when numbers are real decimal numbers, it doesn't quite work. I think it is "how it should be" in that the algorithm assumes more digits means larger numbers. But in the context of a real decimal number it is not correct. Version number sort means
Decimal number sort should be:
Perhaps there ought to be a Caddyfile option for specifying what type of sort that should be applied for specific directory trees? Good example:https://mirrors.tnonline.net/btrfs/btrfs-progs/sources/kernel.org/ Bad example: |
francislavoie
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For files with regions of numbers and other characters, it's intuitive to have the numbers sorted by increasing numeric value rather than by ASCII code. (Jeff Atwood has a nice article here:
https://blog.codinghorror.com/sorting-for-humans-natural-sort-order/)
For example, the listing for a directory containing
foo1,foo2, andfoo10would sort in that order, rather than puttingfoo10betweenfoo1andfoo2.Closes #7226
Since this library is so little code, I opted to copy the code into the repo rather than adding it as a dependency. (Per the Go Proverb, "A little copying is better than a little dependency" 🙂)
Responding to comments from the issue that this PR closes:
I tried (twice!) to switch this PR over to use that package instead, but wasn't able to convince myself it was correct. (I have no reason to believe it's not correct either! I just couldn't quite internalize it enough to be able to vouch for its correctness. Not sure why—it seems well written!—but I guess that implementation just doesn't quite mesh with the way I think about the algorithm.)
Because of that, I stuck with the implementation from the https://github.com/fvbommel/sortorder package, for which I can say I agree with its correctness. That said, if you'd rather I swap over to the other implementation, I'm definitely happy to do that.
Right now, the code makes the assumption that the output will be the same regardless of the format it is presented in. If I were to special-case the HTML output, I'd have to pass some additional context into
applySortAndLimit(and maybe a level further?), too, which would balloon the scope of this change.I'd personally find it surprising if the sort implementation varied depending on what format the output is returned in, so I'd advocate for natural sort regardless of output format
The included test was drafted by a large language model, but largely rewritten (and validated) by me.