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

Use CurrentCulture rather than CurrentUICulture #795

Closed
wants to merge 3 commits into from
Closed

Use CurrentCulture rather than CurrentUICulture #795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a55d4b
Use CurrentCulture rather than CurrentUICulture
tmilnthorp May 26, 2020
ef68ef0
Merge branch 'master' into Culture
tmilnthorp May 29, 2020
b3a784d
Merge fix
tmilnthorp May 29, 2020
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
26 changes: 13 additions & 13 deletions CodeGen/Generators/UnitsNetGen/QuantityGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public static string GetAbbreviation({_unitEnumName} unit)
/// </summary>
/// <param name=""unit"">Unit to get abbreviation for.</param>
/// <returns>Unit abbreviation string.</returns>
/// <param name=""provider"">Format to use for localization. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use for localization. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
public static string GetAbbreviation({_unitEnumName} unit, IFormatProvider? provider)
{{
return UnitAbbreviationsCache.Default.GetDefaultAbbreviation(unit, provider);
Expand Down Expand Up @@ -442,7 +442,7 @@ private void GenerateStaticParseMethods()
/// We wrap exceptions in <see cref=""UnitsNetException"" /> to allow you to distinguish
/// Units.NET exceptions from other exceptions.
/// </exception>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
public static {_quantity.Name} Parse(string str, IFormatProvider? provider)
{{
return QuantityParser.Default.Parse<{_quantity.Name}, {_unitEnumName}>(
Expand Down Expand Up @@ -473,7 +473,7 @@ public static bool TryParse(string? str, out {_quantity.Name} result)
/// <example>
/// Length.Parse(""5.5 m"", new CultureInfo(""en-US""));
/// </example>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
public static bool TryParse(string? str, IFormatProvider? provider, out {_quantity.Name} result)
{{
return QuantityParser.Default.TryParse<{_quantity.Name}, {_unitEnumName}>(
Expand Down Expand Up @@ -501,7 +501,7 @@ public static bool TryParse(string? str, IFormatProvider? provider, out {_quanti
/// Parse a unit string.
/// </summary>
/// <param name=""str"">String to parse. Typically in the form: {{number}} {{unit}}</param>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
/// <example>
/// Length.ParseUnit(""m"", new CultureInfo(""en-US""));
/// </example>
Expand All @@ -527,7 +527,7 @@ public static bool TryParseUnit(string str, out {_unitEnumName} unit)
/// <example>
/// Length.TryParseUnit(""m"", new CultureInfo(""en-US""));
/// </example>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use when parsing number and unit. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
public static bool TryParseUnit(string str, IFormatProvider? provider, out {_unitEnumName} unit)
{{
return UnitParser.Default.TryParse<{_unitEnumName}>(str, provider, out unit);
Expand Down Expand Up @@ -961,7 +961,7 @@ public override string ToString()
/// Gets the default string representation of value and unit using the given format provider.
/// </summary>
/// <returns>String representation.</returns>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
public string ToString(IFormatProvider? provider)
{{
return ToString(""g"", provider);
Expand All @@ -972,7 +972,7 @@ public string ToString(IFormatProvider? provider)
/// </summary>
/// <param name=""significantDigitsAfterRadix"">The number of significant digits after the radix point.</param>
/// <returns>String representation.</returns>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
[Obsolete(@""This method is deprecated and will be removed at a future release. Please use ToString(""""s2"""") or ToString(""""s2"""", provider) where 2 is an example of the number passed to significantDigitsAfterRadix."")]
public string ToString(IFormatProvider? provider, int significantDigitsAfterRadix)
{{
Expand All @@ -987,14 +987,14 @@ public string ToString(IFormatProvider? provider, int significantDigitsAfterRadi
/// <param name=""format"">String format to use. Default: ""{{0:0.##}} {{1}} for value and unit abbreviation respectively.""</param>
/// <param name=""args"">Arguments for string format. Value and unit are implicitly included as arguments 0 and 1.</param>
/// <returns>String representation.</returns>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
[Obsolete(""This method is deprecated and will be removed at a future release. Please use string.Format()."")]
public string ToString(IFormatProvider? provider, [NotNull] string format, [NotNull] params object[] args)
{{
if (format == null) throw new ArgumentNullException(nameof(format));
if (args == null) throw new ArgumentNullException(nameof(args));

provider = provider ?? CultureInfo.CurrentUICulture;
provider = provider ?? CultureInfo.CurrentCulture;

var value = Convert.ToDouble(Value);
var formatArgs = UnitFormatter.GetFormatArgs(Unit, value, provider, args);
Expand All @@ -1003,21 +1003,21 @@ public string ToString(IFormatProvider? provider, [NotNull] string format, [NotN

/// <inheritdoc cref=""QuantityFormatter.Format{{TUnitType}}(IQuantity{{TUnitType}}, string, IFormatProvider)""/>
/// <summary>
/// Gets the string representation of this instance in the specified format string using <see cref=""CultureInfo.CurrentUICulture"" />.
/// Gets the string representation of this instance in the specified format string using <see cref=""CultureInfo.CurrentCulture"" />.
/// </summary>
/// <param name=""format"">The format string.</param>
/// <returns>The string representation.</returns>
public string ToString(string format)
{{
return ToString(format, CultureInfo.CurrentUICulture);
return ToString(format, CultureInfo.CurrentCulture);
}}

/// <inheritdoc cref=""QuantityFormatter.Format{{TUnitType}}(IQuantity{{TUnitType}}, string, IFormatProvider)""/>
/// <summary>
/// Gets the string representation of this instance in the specified format string using the specified format provider, or <see cref=""CultureInfo.CurrentUICulture"" /> if null.
/// Gets the string representation of this instance in the specified format string using the specified format provider, or <see cref=""CultureInfo.CurrentCulture"" /> if null.
/// </summary>
/// <param name=""format"">The format string.</param>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""provider"">Format to use for localization and number formatting. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
/// <returns>The string representation.</returns>
public string ToString(string format, IFormatProvider? provider)
{{
Expand Down
4 changes: 2 additions & 2 deletions CodeGen/Generators/UnitsNetGen/StaticQuantityGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using CodeGen.Helpers;
using CodeGen.Helpers;
using CodeGen.JsonTypes;

namespace CodeGen.Generators.UnitsNetGen
Expand Down Expand Up @@ -89,7 +89,7 @@ public static bool TryFrom(QuantityValue value, Enum unit, out IQuantity? quanti
/// <summary>
/// Try to dynamically parse a quantity string representation.
/// </summary>
/// <param name=""formatProvider"">The format provider to use for lookup. Defaults to <see cref=""CultureInfo.CurrentUICulture"" /> if null.</param>
/// <param name=""formatProvider"">The format provider to use for lookup. Defaults to <see cref=""CultureInfo.CurrentCulture"" /> if null.</param>
Copy link
Owner

@angularsen angularsen May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hm. What is correct here, if we can only specify a single culture?

My understanding:
CurrentUICulture is used for display language in the UI, which affects our abbreviation parsing.
CurrentCulture is used for date/number formatting, which affects our number parsing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From my understanding, CurrentUICulture is for use with resource manager only. It's an odd one, as CurrentUICulture would be en-US even for someone in Britain (where I'd expect en-GB). Important if we ever want to distinguish meter from metre for example?

Copy link
Owner

Choose a reason for hiding this comment

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

Well. Thought experiment:

  • I am Norwegian.
  • I configure my Windows region to Norway, so that Windows Store shows me content available in my region. I believe this is not exposed in .NET.
  • I configure my Windows regional format to English (Sweden) to get ISO dates instead of Norwegian dates, because I'm weird that way. I believe this configures CurrentCulture to en-SE.
  • I configure Windows display language to English (United States), because I prefer reading English. I believe this configures CurrentUICulture to en-US.

This is my actual current setup:
image

image

image

Interestingly, my country/region Norway is not visible in RegionInfo. In fact, it seems this choice is not available in .NET framework, only via WinSDK or P/Invoke.

So. In my particular Windows configuration. I want to read English, but I want date/numbers formatted in ISO (Sweden).

How should UnitsNet format the number and what language should it choose abbreviations from? I would prefer Swedish numbers and English abbreviations, but it seems I may be stuck with all Swedish in this PR proposal, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting. They had to make it difficult didn't they 😂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What does DateTime.Now.ToString("F") give you?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From MSDN:

The ToString(String) method returns the string representation of a date and time value in a specific format that uses the formatting conventions of the current culture; for more information, see CultureInfo.CurrentCulture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Result:

Thursday, May 28, 2020 9:30:26 PM
Thursday, 28 May 2020 21:30:26
den 28 maj 2020 21:30:26

That looks like what I'd expect. So everything is formatted according to the given culture. This suggests to always use CurrentCulture, and I think we can agree that this is what should be used for formatting the number part. For the units, it's a bit more complicated and the docs are a bit less clear.

Now technically, the unit names are resource-lookups and should use CurrentUiCulture, but apparently, its not used that way (otherwise, the day and month names in the above example would have been english, regardless of the explicitly provided culture, because @tmilnthorp uses en-US as CurrentUiCulture [I assume]). It's not possible to provide two cultures to a ToString call. And it would be confusing to use one language's string formatting with another languages unit names.

IMHO, the correct behavior is to always use the provided culture for the whole formatting operation (number and unit name) and use CurrentCulture as the default.

Copy link
Owner

Choose a reason for hiding this comment

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

I think you are right. We can only pass a single culture, this is the norm in .NET.

Seeing that .NET DateTime also uses the passed in culture, instead of trying to use CurrentUICulture for the text parts, we should probably do the same.
In a way, it makes sense that whatever culture you pass in, is used consistently for all the parts of the string formatting. It is easier to reason about, even if it leaves my Windows configuration edge case hanging in the cold. I'm fine with that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this has any relevance- but for my localized WPF applications - I tend to setup 4 things on login (culture is part of the user profile):

        CultureInfo.DefaultThreadCurrentCulture = cultureInfo;
        CultureInfo.DefaultThreadCurrentUICulture = cultureInfo;

        Thread.CurrentThread.CurrentCulture = cultureInfo;
        Thread.CurrentThread.CurrentUICulture = cultureInfo;

..thinking that the UI sets the default culture for the UI threads- with the other one concerning the culture of any background threads. And from what I understand- you're saying that actually one is for the language-localization and the other for the formatting?
PS I'm in the exact same situation as @angularsen in terms of the regional settings (only the input is different)
image

Copy link
Contributor

Choose a reason for hiding this comment

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

@lipchev : You would normally do that, if your application supports localization. If it does not, you could still use localized number formatting. The CurrentUiCulture will have a fallback (usually english) if it doesn't exist, while the CurrentCulture is always completely defined. Using applications (Windows itself, but also Office or Visual Studio) in english while still using the locale's preferred number/date formatting is a very common scenario.

/// <param name=""quantityType"">Type of quantity, such as <see cref=""Length""/>.</param>
/// <param name=""quantityString"">Quantity string representation, such as ""1.5 kg"". Must be compatible with given quantity type.</param>
/// <param name=""quantity"">The resulting quantity if successful, otherwise <c>default</c>.</param>
Expand Down
16 changes: 8 additions & 8 deletions CodeGen/Generators/UnitsNetGen/UnitTestBaseClassGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,8 @@ public void BaseDimensionsShouldNeverBeNull()
[Fact]
public void ToString_ReturnsValueAndUnitAbbreviationInCurrentCulture()
{{
var prevCulture = Thread.CurrentThread.CurrentUICulture;
Thread.CurrentThread.CurrentUICulture = CultureInfo.GetCultureInfo(""en-US"");
var prevCulture = Thread.CurrentThread.CurrentCulture;
Thread.CurrentThread.CurrentCulture = CultureInfo.GetCultureInfo(""en-US"");
try {{");
foreach (var unit in _quantity.Units)
{
Expand All @@ -428,7 +428,7 @@ public void ToString_ReturnsValueAndUnitAbbreviationInCurrentCulture()
}}
finally
{{
Thread.CurrentThread.CurrentUICulture = prevCulture;
Thread.CurrentThread.CurrentCulture = prevCulture;
}}
}}

Expand All @@ -449,18 +449,18 @@ public void ToString_WithSwedishCulture_ReturnsUnitAbbreviationForEnglishCulture
[Fact]
public void ToString_SFormat_FormatsNumberWithGivenDigitsAfterRadixForCurrentCulture()
{{
var oldCulture = CultureInfo.CurrentUICulture;
var oldCulture = CultureInfo.CurrentCulture;
try
{{
CultureInfo.CurrentUICulture = CultureInfo.InvariantCulture;
CultureInfo.CurrentCulture = CultureInfo.InvariantCulture;
Assert.Equal(""0.1 {_baseUnitEnglishAbbreviation}"", new {_quantity.Name}(0.123456{_numberSuffix}, {_baseUnitFullName}).ToString(""s1""));
Assert.Equal(""0.12 {_baseUnitEnglishAbbreviation}"", new {_quantity.Name}(0.123456{_numberSuffix}, {_baseUnitFullName}).ToString(""s2""));
Assert.Equal(""0.123 {_baseUnitEnglishAbbreviation}"", new {_quantity.Name}(0.123456{_numberSuffix}, {_baseUnitFullName}).ToString(""s3""));
Assert.Equal(""0.1235 {_baseUnitEnglishAbbreviation}"", new {_quantity.Name}(0.123456{_numberSuffix}, {_baseUnitFullName}).ToString(""s4""));
}}
finally
{{
CultureInfo.CurrentUICulture = oldCulture;
CultureInfo.CurrentCulture = oldCulture;
}}
}}

Expand Down Expand Up @@ -491,10 +491,10 @@ public void ToString_NullArgs_ThrowsArgumentNullException()
}}

[Fact]
public void ToString_NullProvider_EqualsCurrentUICulture()
public void ToString_NullProvider_EqualsCurrentCulture()
{{
var quantity = {_quantity.Name}.From{_baseUnit.PluralName}(1.0);
Assert.Equal(quantity.ToString(CultureInfo.CurrentUICulture, ""g""), quantity.ToString(null, ""g""));
Assert.Equal(quantity.ToString(CultureInfo.CurrentCulture, ""g""), quantity.ToString(null, ""g""));
}}

#pragma warning restore 612, 618
Expand Down
2 changes: 1 addition & 1 deletion UnitsNet.Tests/CustomCode/StonePoundsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void StonePoundsToString_FormatsNumberInDefaultCulture()
{
Mass m = Mass.FromStonePounds(3500, 1);
StonePounds stonePounds = m.StonePounds;
string numberInCurrentCulture = 3500.ToString("n0", CultureInfo.CurrentUICulture); // Varies between machines, can't hard code it
string numberInCurrentCulture = 3500.ToString("n0", CultureInfo.CurrentCulture); // Varies between machines, can't hard code it

Assert.Equal($"{numberInCurrentCulture} st 1 lb", stonePounds.ToString());
}
Expand Down
16 changes: 8 additions & 8 deletions UnitsNet.Tests/GeneratedCode/TestsBase/AccelerationTestsBase.g.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions UnitsNet.Tests/GeneratedCode/TestsBase/AmountOfSubstanceTestsBase.g.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading