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.

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

Enable Globalization to read the user setting overrides for current cultures when using ICU #37121

Closed
DmitryGaravsky opened this issue May 28, 2020 · 12 comments · Fixed by #38372

Comments

@DmitryGaravsky
Copy link
Contributor

DmitryGaravsky commented May 28, 2020

Description

The DateTimeFormatInfo properties in Net Core 5 Preview 4 return different culture-specific information about the format of date and time values compared to .Net Framework in the same environment

Our unit tests detected this issue immediately. (we maintain unit tests for all the released and preview environments like .net core 5 in our codebase). Thus it is an important breaking change for us.

Configuration

OS Version: Microsoft Windows [Version 10.0.18362.535]
.Net Framework Version: Microsoft (R) .NET CLR Version Tool Version 4.8.3928.0
.Net Core Version: SDK Version: 5.0.100-preview.4.20258.7

Regression?

Yes, this works correctly until .Net Core 5 Preview 4

Other information

Here are some tests which fail under .Net Core 5 Preview 4

[TestCase("en-US", "M/d/yyyy", "dddd, MMMM d, yyyy")]
[TestCase("ru-RU", "dd.MM.yyyy", "d MMMM yyyy 'г.'")]
public void DatePatterns(string culture, string pShort, string pLong) {
	CultureInfo c = new CultureInfo(culture);
	Assert.AreEqual(pShort, c.DateTimeFormat.ShortDatePattern);
	Assert.AreEqual(pLong, c.DateTimeFormat.LongDatePattern);
}
[TestCase("en-US", "h:mm tt", "h:mm:ss tt")]
[TestCase("ru-RU", "H:mm", "H:mm:ss")]
public void TimePatterns(string culture, string pShort, string pLong) {
	CultureInfo c = new CultureInfo(culture);
	Assert.AreEqual(pShort, c.DateTimeFormat.ShortTimePattern);
	Assert.AreEqual(pLong, c.DateTimeFormat.LongTimePattern);
}
[TestCase("en-US", "January", "Jan")]
[TestCase("ru-RU", "Январь", "янв")]
public void MonthNames(string culture, string name, string abrName) {
	CultureInfo c = new CultureInfo(culture);
	Assert.AreEqual(name, c.DateTimeFormat.MonthNames[0]);
	Assert.AreEqual(abrName, c.DateTimeFormat.AbbreviatedMonthNames[0]);
}
[TestCase("en-US", "January", "Jan")]
[TestCase("ru-RU", "января", "янв")]
public void MonthGenitiveNames(string culture, string name, string abrName) {
	CultureInfo c = new CultureInfo(culture);
	Assert.AreEqual(name, c.DateTimeFormat.MonthGenitiveNames[0]);
	Assert.AreEqual(abrName, c.DateTimeFormat.AbbreviatedMonthGenitiveNames[0]);
}
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels May 28, 2020
@stephentoub
Copy link
Member

I expect this is by-design and due to switching to using ICU instead of NLS by default for globalization.
https://github.com/safern/runtime/blob/c592559b25de2bcc2ddf8adcb94a9a37fe1d10ce/docs/design/features/globalization-icu.md
cc: @safern, @tarekgh

@tarekgh
Copy link
Member

tarekgh commented May 28, 2020

@stephentoub is right. We are now using ICU by default. the doc link that @stephentoub mentioned has the way how to switch back using NLS instead of ICU to revert back to the old behavior. Please send any questions you have and we'll be happy to help with.

@tarekgh tarekgh closed this as completed May 28, 2020
@DmitryGaravsky
Copy link
Contributor Author

Thanks, @stephentoub and @tarekgh

I clearly understand the reason - using ICU instead of NLS.
The single point I can't understand is "Why the results have taken via ICU does not match OS settings for current culture?".

Should I always switch back to NLS to create desktop applications that take OS settings into account?

image

@safern
Copy link
Member

safern commented May 28, 2020

@Dmitri-Botcharnikov maybe you're hitting this issue: #35638 ?

This was fixed for Preview5. Could you try with our latest release and see if that works as expected?

You can get the latest daily build from here: https://github.com/dotnet/installer/blob/master/README.md#installers-and-binaries

@tarekgh
Copy link
Member

tarekgh commented May 28, 2020

@DmitryGaravsky that is a good point. we may try to look at this one and figure out if it make-sense using the overrides from the System. I am not expecting users to switch back to NLS just to pick the overrides.

@stephentoub stephentoub reopened this May 28, 2020
@tarekgh tarekgh changed the title The DateTimeFormatInfo properties return different value since .Net Core5 Preview 4 in the same environment Enable Globalization to read the user settings overrides for current cultures when using ICU May 28, 2020
@tarekgh tarekgh changed the title Enable Globalization to read the user settings overrides for current cultures when using ICU Enable Globalization to read the user setting overrides for current cultures when using ICU May 28, 2020
@ghost
Copy link

ghost commented May 28, 2020

Tagging subscribers to this area: @tarekgh, @safern, @krwq
Notify danmosemsft if you want to be subscribed.

@tarekgh tarekgh added bug and removed untriaged New issue has not been triaged by the area owner labels May 28, 2020
@tarekgh tarekgh added this to the 5.0 milestone May 28, 2020
@tarekgh
Copy link
Member

tarekgh commented May 28, 2020

@DmitryGaravsky I have changed the title of the issue just to reflect what needs to be fixed.

@safern safern self-assigned this May 28, 2020
@safern
Copy link
Member

safern commented May 28, 2020

Thanks @tarekgh I've self assigned to start looking into it.

@DmitryGaravsky
Copy link
Contributor Author

Thanks, guys! I'll stay tuned

@iSazonov
Copy link
Contributor

iSazonov commented Jul 3, 2020

What .Net preview version is we get the fix in?

@safern
Copy link
Member

safern commented Jul 3, 2020

This is going to land on Preview8. I can’t point you to the first daily build that contains the fix for Powershell to try it out.

@safern
Copy link
Member

safern commented Jul 16, 2020

@iSazonov I forgot to let you know as we were having trouble to update the SDK to a new runtime, but the latest daily SDK should contain this fix if you want to try it 😄

https://aka.ms/dotnet/net5/dev/Sdk/dotnet-sdk-win-x64.exe

https://github.com/dotnet/installer/blob/master/README.md#installers-and-binaries

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants