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.

Sign up for GitHub

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

Jump to bottom

Strings: unstable output for keys that differ just by case #891

Open
grigorye opened this issue Oct 18, 2021 · 2 comments · May be fixed by #890
Open

Strings: unstable output for keys that differ just by case #891

grigorye opened this issue Oct 18, 2021 · 2 comments · May be fixed by #890

Comments

@grigorye
Copy link

grigorye commented Oct 18, 2021 •
edited

If .strings contains two keys that differ just by case, the order of entries in the generated .swift file is not stable from invocation to invocation.

This is quite edge case, given that (as far as I understand) SwiftGen does not support multiple entries that differ just by case, but it still happens in our case, as we have to entries that differ just by case of keys and number of parameters, like below.

Localizable.strings:

"A" = "Foo";
"a" = "Bar %s";

The other rationale for this change is that the current case-insensitive sorting does not make that much sense in case camelCase is used for the keys: capital letters in camelCase typically introduce a grouping/another "word", but lowercase letters just introduce word variation. From that perspective I believe that word variations should better belong to one group.

Demo of the issue: https://github.com/grigorye/SwiftGen-Case-Insensitive
@grigorye grigorye linked a pull request Oct 18, 2021 that will close this issue
Open
@djbe
Copy link
Member

djbe commented Nov 26, 2021

I honestly had to dig for quite a bit (because of the repo split -> back to one repo), until I found the change that added this:
d774a7c

From the discussion in that PR, I can't seem to find any mention WHY we did it, nor are there any issues around that time related to this. It was probably done because it's "nicer" to have things alphabetically. Sorting case-insensitively is usually better/more readable 🤷

@djbe
Copy link
Member

djbe commented Nov 26, 2021 •
edited

Note that at in that same PR eventually switched to $0.key.caseInsensitiveCompare($1.key) == .orderedAscending
9a70cd2

During the repo split, that got reverted to comparison using lowercased(), maybe because I thought the former is an NSString method... Does using caseInsensitiveCompare have stable sorting for your issue? If so, I'd prefer to use that, just so we can keep the nice(r) sorting 😄

@djbe djbe changed the title Output is not stable in the presence of the .strings keys that differ just by case. Strings: unstable output for keys that differ just by case Nov 26, 2021
@djbe djbe linked a pull request Jul 22, 2022 that will close this issue
Strings: reverted to case-sensitive sorting for keys #890
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Strings: reverted to case-sensitive sorting for keys grigorye/SwiftGen
2 participants
@djbe @grigorye