Skip to content

fix: use locale-aware collation for language name sorting#13141

Open
okarachidera wants to merge 3 commits into
internetarchive:masterfrom
okarachidera:fix/language-sort-collation
Open

fix: use locale-aware collation for language name sorting#13141
okarachidera wants to merge 3 commits into
internetarchive:masterfrom
okarachidera:fix/language-sort-collation

Conversation

@okarachidera

Copy link
Copy Markdown

Summary

Fixes #11962: Language names are now sorted using locale-aware Unicode collation so that characters with diacritics (Č, Ć, É, etc.) sort alongside their base characters rather than at the end of the alphabet.

What was done

  • Added _sort_key_name(name) helper in openlibrary/plugins/worksearch/languages.py that uses locale.strxfrm(name.casefold()) for Unicode-aware collation with graceful fallback to casefold() when locale transformation fails
  • Updated get_top_languages() to use the new sort key when sorting by name
  • Added openlibrary/plugins/worksearch/tests/test_language_sort.py with 11 tests covering: case-insensitivity, diacritic ordering, mixed-language sorting, stability, empty strings, surrogate fallback, and locale-aware collation

Testing

The sort key logic was verified locally:

Sorted: ['Català', 'Čeština', 'Cymraeg', 'Deutsch', 'Zulu']
All C-family languages sort before Deutsch ✓

Unit tests are included in the PR.

Before / After

Before: .casefold() only — characters with diacritics sort at the end.
After: locale.strxfrm(.casefold()) — Č sorts next to C, É sorts next to E, etc.

Closes #11962

Replace casefold()-only sort key with locale.strxfrm(name.casefold())
so that diacritics (Č, Ć, É, etc.) sort alongside their base characters
rather than at the end of the alphabet.

Adds _sort_key_name() helper with graceful fallback to casefold() if
locale transformation fails (e.g. lone surrogates).

Fixes internetarchive#11962
@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project Jul 9, 2026
@openlibrary-bot

Copy link
Copy Markdown
Collaborator

Thanks for opening this PR, @okarachidera — and welcome, this looks like your first contribution to Open Library! 🎉

@RayBB is assigned to this PR and currently has:

  • 8 open PR(s) of equal or higher priority to review first

Note for reviewers: PR #12802 was opened earlier for the same issue (#11962) and is still open. Maintainers may want to compare both approaches before deciding which to move forward with.

PR triage checklist (maintainers / Richy)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — checks still running as of this comment
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting.

— Richy, via PAM (Open Library's Project AI Manager)

@okarachidera

Copy link
Copy Markdown
Author

Thanks for the triage, @openlibrary-bot.

I've looked at #12802 and believe this PR offers a simpler approach:

  • No new system dependency — uses Python stdlib locale.strxfrm instead of PyICU (no libicu-dev Dockerfile change needed)
  • Minimal diff — changes only the sort key, no unrelated formatting
  • Tested — 11 unit tests: diacritics, case-insensitivity, fallback, stability
  • Maintainer-aligned@cdrini suggested a simpler approach first on List languages in correct alphabetical order #11962: "if you want to open a PR that sorts the lowercase version of the language name, that will fix a good chunk of the issue, and we can merge that in"

Happy to adjust if maintainers prefer the PyICU path in #12802.

- Remove blank line between import groups
- Use tuple in startswith() call
- Add fmt: skip on multi-except clause to prevent formatter from
  producing Python-3-invalid comma-separated except syntax
@okarachidera
okarachidera force-pushed the fix/language-sort-collation branch from 2bda992 to b302e9e Compare July 9, 2026 16:09
@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Response Issues which require feedback from lead

Projects

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

List languages in correct alphabetical order

3 participants