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

Implement sorting using twitter_cldr #32

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aried3r
Copy link
Contributor

@aried3r aried3r commented Jun 15, 2021

This is a draft PR since it does not necessarily overlap with the goals of this gem.

Using twitter_cldr we can solve #20 and partly #18. #20, because we can now pass a German collator and the result will be correct.
Case insensitive sorting is only partly solved as can be seen in the new specs. Lower case letters are sorted before upper case letters, however this might give surprising results, depending on your use case. As you can see, "öl3" is not after "Öl2", because aforementioned rule.

Before implementing self.sort_with_collator I tried using the implementation using a collator as the general case and all specs passed, so it seems that it could potentially be extended to have all methods take an optional collator. I didn't go that far for this PR since it might collide with the goals of this gem as it introduces an optional gem dependency.

Maybe it can also serve others as a pointer on how to solve similar issues.

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.

1 participant