-
Notifications
You must be signed in to change notification settings - Fork 965
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
Fix CultureNotFoundException when InvariantGlobalization is enabled #1213
Conversation
…ion-invariant mode"
Any reason for not accepting this useful PR? |
@clairernovotny @MehdiK @hazzik can we please have this merged? |
@clairernovotny @MehdiK @hazzik Would you please take a look at this PR if you've got the time? |
@ptupitsyn sorry for the delayed reply. can you rebase. and also add a test for this scenario |
@SimonCropp invariant globalization can only be enabled before the runtime starts: https://learn.microsoft.com/en-us/dotnet/core/runtime-config/globalization I'm not sure how to test this scenario. Is it ok to spawn a new process? |
# Conflicts: # src/Humanizer/Configuration/Configurator.cs # src/Humanizer/Localisation/Formatters/DefaultFormatter.cs
@ptupitsyn i think the best way is to add a new test project with the following
and add a few tests to it. enough to verify the change |
Just a question: how to differentiate between culture not found due, for ex, misspelling, and globalization invariant? |
@hazzik perhaps it throws a different exception? |
{ | ||
try | ||
{ | ||
return new FormatterRegistry(new DefaultFormatter("en-US"), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This try-catch does not guarantee that exception is expected at exactly new DefaultFormatter("en-US")
and in particular at _culture = new(localeCode)
part. It can be thrown anywhere inside the FormatterRegistry
constructor which creates many more cultures.
I think it need to be localized to something like this:
CultureInfo GetDefaultCulture() {
try {
return new CultureInfo("en-US");
} catch (CultureNotFoundException) {
return CultureInfo.InvariantCulture;
}
}
And then check if default culture is invariant and if so then skip all other initializers.
Fixes #1126 .
Starting with .NET 6, when
InvariantGlobalization
is enabled,CultureInfo
constructor throwsCultureNotFoundException
for all cultures except invariant: https://docs.microsoft.com/en-us/dotnet/core/compatibility/globalization/6.0/culture-creation-invariant-modeCultureNotFoundException
when creating the default formatter.en-US
.