Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support case variants for insensitive lexical sort #189

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

mcongrove
Copy link
Contributor

@mcongrove mcongrove commented Dec 30, 2023

Add support for keys that are duplicates (but in different cases) to the case-insensitive lexical sort.

This is especially useful for translation files that are stored as JSON, such as:

{
  "user": "user",
  "User": "User"
}

These two keys represent different translation strings, so cannot be de-duped. When running through the current case-insensitive lexical sort, because the keys are first lowercased and then sorted, the order of these keys can change from one run to the next. This causes constant changes to the JSON file, and can fail pre-commit hooks, etc.

This PR checks to see if the keys are duplicates, and if so, falls back to the case-sensitive lexical sort to maintain consistent sorting between the conflicting keys.

@Gudahtt
Copy link
Owner

Gudahtt commented Mar 18, 2024

Thanks for the contribution! I appreciate the changelog update but I've reverted it for now to get the linter passing (the changes should have gone under "Unreleased"; with the release process used here, the version isn't set until we're preparing the release). I'll consult your change entry here when preparing the changelog later.

@Gudahtt
Copy link
Owner

Gudahtt commented Mar 18, 2024

This is an interesting case because we're making a tradeoff here between having a deterministic item order (your preference) versus preserving the original item order when possible (the old behavior). At first when I read this PR, my thought was that this is no longer fully "case insensitive". But I think it's a matter of interpretation.

This version of case insensitive search seems likely to be more useful to more people than the original behavior, so I'm going to accept this. However I will be marking it as a breaking change, and leaving a note in the changelog about how to preserve the old behavior with a custom JSON sort order config, just in case anyone really wanted to ensure original order was preserved.

There was a breaking change pending release anyway, so the timing works out well.

I have added a test case showing how to preserve the old behavior with a custom JSON sort configuration. It's tedious but possible. I've updated both the new test case and your test case to have one in-order pair of keys as well, to better explain the difference between the two cases.

@Gudahtt Gudahtt merged commit 2d305da into Gudahtt:main Mar 18, 2024
15 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants