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

[iOS] Set CurrentCulture locale to specific format based on system default when possible #111032

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+16 −9
Open

[iOS] Set CurrentCulture locale to specific format based on system default when possible #111032

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
858ff2f
Fix CurrentCulture to be in specific format
matouskozak Jan 2, 2025
009b11c
use parent culture if retrieved culture is custom
matouskozak Jan 3, 2025
1d59b46
update test scenario
matouskozak Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../System.Runtime/tests/System.Globalization.Tests/CultureInfo/CultureInfoCurrentCulture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public void CurrentCulture_Default_Not_Invariant()
// culture.
Assert.NotEqual(CultureInfo.CurrentCulture, CultureInfo.InvariantCulture);
Assert.NotEqual(CultureInfo.CurrentUICulture, CultureInfo.InvariantCulture);

// The current culture taken from default system culture should be specific
Assert.False(CultureInfo.CurrentCulture.IsNeutralCulture);
}

[Fact]
Expand Down
11 changes: 2 additions & 9 deletions src/native/libs/System.Globalization.Native/pal_locale.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,21 +786,14 @@ int32_t GlobalizationNative_GetLocalesNative(UChar* value, int32_t length)
}
}

static NSString* GetBaseName(NSString *localeIdentifier)
{
NSLocale *locale = [[NSLocale alloc] initWithLocaleIdentifier:localeIdentifier];
NSString *languageCode = [locale objectForKey:NSLocaleLanguageCode];
return languageCode;
}

Comment on lines -789 to -795
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know what this was done before?
The PR description should be updated to describe this - what we did before (has that) and why, and what we're changing it to (and why it's OK to change given the previous reasoning).

Copy link
Member Author

@matouskozak matouskozak Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I was trying this PR to see which test suit breaks if we switch to specific name because there are other dependencies such as SqlStrings that were previously affected. I'll update the issue description when I finalize the code changes.

Brief history what I found:

I'll try to make a middle way out of this, where we use system locale if it's recognized by .NET, otherwise we use parent culture which is neutral.

const char* GlobalizationNative_GetDefaultLocaleNameNative(void)
{
@autoreleasepool
{
if (NSLocale.preferredLanguages.count > 0)
{
NSString *preferredLanguage = [NSLocale.preferredLanguages objectAtIndex:0];
return strdup([GetBaseName(preferredLanguage) UTF8String]);
return strdup([preferredLanguage UTF8String]);
}
else
{
Expand All @@ -821,7 +814,7 @@ int32_t GlobalizationNative_GetLocalesNative(UChar* value, int32_t length)
localeName = currentLocale.localeIdentifier;
}

return strdup([GetBaseName(localeName) UTF8String]);
return strdup([localeName UTF8String]);
}
}
}
Expand Down
Loading