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

Implement subclass of CultureInfo (UCultureInfo) to replace ULocale #3

Closed
NightOwl888 opened this issue Sep 30, 2019 · 0 comments · Fixed by #27
Closed

Implement subclass of CultureInfo (UCultureInfo) to replace ULocale #3

NightOwl888 opened this issue Sep 30, 2019 · 0 comments · Fixed by #27

Comments

@NightOwl888
Copy link
Owner

This issue is blocking ICU4N from going to beta.

In Java, the ULocale class was created as a separate entity in ICU4J because:

  1. The Java Locale class is sealed/final
  2. In Java, the Locale is stored in a static field, not associated with the current thread like CultureInfo is in .NET

To make ICU4N compatible with .NET, a subclass of CultureInfo should be created (named UCultureInfo, for consistency) to replace ULocale.

This subclass should have all of the functionality of ULocale, but subclassing CultureInfo makes the class automatically compatible with the properties that allow it to be set to the current thread.

Compatibility

Gaps

There are several gaps in functionality between Java/.NET that need to be addressed. Including:

  1. Conversion nn-NO-NY to/from nn-NO
  2. NumberFormat differences (for example, in Java there is min and max number of decimals, but in .NET there is a single property indicating the number of decimals).

There may be others, more analysis is needed to get the complete list. Perhaps a conversation with the ICU team and/or Microsoft is required to work out how to account for these gaps.

Constructor

The constructor of UCultureInfo needs to accept both standard .NET cultures (in which case, we will just wrap the default culture), and it also should accept ICU-style parameters. A decision needs to be made whether to keep the ICU4N underscores (en_US@collation=phonebook), or to make it more like the .NET style (en-US@collation=phonebook). In the JDK, there are no underscores, the values are passed as separate parameters to the constructor of Locale, so there is no definite precedence to follow.

We may need to take the ICU data into consideration when making this decision.

Calendars

.NET actually has more calendars than ICU, so it is unclear exactly how to deal with this just yet. ICU is meant to be an "up to date" version of Unicode that is more recent than .NET, but we need to check whether the calendars in .NET are being kept up with the standard.

Conversion

Unlike Java, we won't need to convert UCultureInfo > CultureInfo as we are a subclass so UCultureInfo is a CultureInfo already.

To convert the other way, my thought is that we should use the same pattern as was done on the System.Type class in .NET.

var name = this.GetType().GetTypeInfo().Name;

So, we could do a similar thing with CultureInfo by creating an extension method:

var ucultureInfo = CultureInfo.CurrentCulture.GetUCultureInfo();

This would allow us to add additional properties to the subclass for use by ICU4N and/or its users.

Miscellaneous

There are probably additional things that need to be considered. More analysis is required.

@NightOwl888 NightOwl888 added this to the Beta Launch milestone Sep 30, 2019
@NightOwl888 NightOwl888 modified the milestones: beta, 60.1.0-alpha.300 May 8, 2020
@NightOwl888 NightOwl888 self-assigned this May 8, 2020
@NightOwl888 NightOwl888 linked a pull request Jun 25, 2020 that will close this issue
NightOwl888 added a commit that referenced this issue Jun 25, 2020
* ICU4N.Support.Globalization: Added UCultureInfo class and reworked GetLanguage(), GetCountry(), GetVariant(), and GetScript() into properties

* ICU4N.Impl.BaseLocale: Refactored getters into properties: GetLanguage() > Language, GetScript() > Script, GetRegion() > Region, GetVariant() > Variant

* ICU4N.Impl.Locale.BaseLocale: Use pattern matching rather than multiple casts for type conversion

* ICU4N.Impl.LocaleIDParser: Refactored GetKeywordMap() into Keywords property

* BUG: ICU4N.Globalization.UChar::IsWhiteSpace(char) - method was calling itself, added a cast to int

* ICU4N.Globalization.LocaleDisplayNames: De-nested and refactored DataTable/DataTables to allow external libraries to supply their own data distributions.

* ICU4N.Globalization: Refactored DisplayContext enums into a 4 separate enums and a DisplayContextOptions class.

* region/language fixup

* ICU4N.Globalization: Ported LocaleDisplayNames for use with UCultureInfo

* ICU4N.Impl.LocaleIDs: Renamed GetISO3Country() > GetThreeLetterISOCountryName() to match .NET

* ICU4N.Globalization: Ported ULocale tests for UCultureInfo

* ICU4N.Impl: Added comments about relocating locale related types

* ICU4N.Globalization.UCultureInfo: Cleaned up API of AcceptLanguage overloads

* BUG: ICU4N.Impl.ICUResourceBundle: Fixed issue with null ulocales list

* BUG: ICU4N.Collation.Text.CollatorServiceShim: Fixed issue with referencing the wrong assembly when requesting available locales

* ICU4N.Impl.Locale.LanguageTag: Changed static fields to constants

* ICU4N.Globalization.CultureInfoExtensions: Fixed equality checking to account for cultures with custom calendars defined.

* ICU4N.Globalization.UCultureInfo: Added special cases for .NET alternate collations, calendars, and cultures that .NET automatically converts that need to be converted back in ICU

* ICU4N.Tests.Support.Text.ChoiceFormatTest: Removed duplicate using directive

* ICU4N.Tests.Globalization.UCultureInfoTest: Fixed setting CurrentCulture and CurrentUICulture cross platform

* build/TestTargetFramework.props: Added FEATURE_CULTUREINFO_GETCULTURES and setup explicit list of cultures for testing on .NET Standard 1.x

* ICU4N.Suport.UCultureInfo.DotNetLocaleHelper: Fixed base name to exclude script and to use the correct delimiters for .NET

* Upgraded CurrencyDisplayNames/UResourceBundle to support UCultureInfo

* Revert "ICU4N.Suport.UCultureInfo.DotNetLocaleHelper: Fixed base name to exclude script and to use the correct delimiters for .NET"

This reverts commit 9bd751e.

* ICU4N.Tests.Globalization.UCultureInfoTest: Updated references to use CultureDisplayNames instead of LocaleDisplayNames

* BUG: ICU4N.CurrencyData.Impl.ICUCurrencyDisplayInfoProvider: Fixed caching so it will switch between ULocale and UCultureInfo (no effect once ULocale is removed).

* BUG: ICU4N.Impl.ICUResourceBundle::FindStringWithFallback(): Fixed null reference exception when passing a null path

* ICU4N.Globalization.ICUDataTable: Updated to use the ICUResourceBundle.CultureInfo property

* ICU4N.Impl.LocaleIDParser: Changed GetName() > GetFullName(). Added new GetName() implementation that returns a hyphen-delimited name compatible with .NET.

* ICU4N.Globalization.UCultureInfo: Added InvariantCulture property

* ICU4N.Globalization.UCultureInfo: Added functionality for DefaultThreadCurrentCulture, DefaultThreadCurrentUICulture, CurrentCulture, and CurrentUICulture. For now, we track the current culture and UI culture of .NET unless it is explicitly set.

* ICU4N.Globalization.UCultureInfo: Made GetInstance(), Base, and LocaleExtensions members internal.

* BUG: ICU4N.Globalization.UCultureInfo::GetInstance(): Use StringComparer.Ordinal when sorting keys

* ICU4N.Globalization: Added UCultureInfoParser (which was a nested class on ULocale in ICU4J)

* ICU4N.Globalization.DataTableCultureDisplayNames: Completed implementation

* ICU4N.Tests.Globalization.UCultureInfoTest::TestToCultureInfo_AllCultures(): Added [Ignore] attribute, as this is only for identifying gaps

* ICU4N.Collation.Impl.Coll.CollationRuleParser: Updated to use UCultureInfoBuilder class

* ICU4N.Tests.Globalization.UCultureInfoTest.CurrentContext: Fixed compile for .NET Framework 4.0

* ICU4N.Collation.Impl.Coll.CollationLoader: Passed references to the current assembly in UResourceBundle.GetBundleInstance calls

* ICU4N.Globalization: Added ThreadCultureChange for testing purposes

* Directory.Build.targets + tests/TestAssemblyInfo.cs: Added DEBUG configuration so the test timeout doesn't occur when debugging

* BREAKING: Replaced ULocale with UCultureInfo

* BUG: ICU4N.Globalization.DisplayContextOptions: Fixed issue with GetHashCode() always returning 0 and also completed IFreezable implementation to correctly prevent properties from being edited when frozen.

* BUG: ICU4N.Globalization.UiListItem: Fixed reference equality comparison issue due to overloaded == operator

* ICU4N.Globalization.UCultureInfo: Added English static field so the EnglishName doesn't have to create a UCultureInfo instance for each language

* ICU4N.Impl.ICUService: Switched from HashSet to List, since we are pulling from a unique list of keys

* ICU4N.Globalization: Added UCultureTypes enum

* ICU4N.Impl.LocaleIDParser: Converted GetLanguageScriptCountryVariant() to GetLocaleID that returns a LocaleID struct rather than an array.

* Added filtering using UCultureTypes enum

* Fixed build for .NET Framework 4.0

* ICU4N.Support.UCultureInfo: Added InstalledUICulture property and additional tests

* ICU4N.Globalization.LocaleID: Added serializable support on .NET Framework

* ICU4N.Globalization.UCultureInfo: Removed CultureInfo as a superclass, marked sealed, and made unfinished members internal

* ICU4N.Tests.Globalization.TestUCultureInfo::TestCategoryDefault(): Fixed conditional compile issue with .NET Framework

* ICU4N.Globalization.UCultureInfo: Fixed invariant equality comparison

* ICU4N.Globalization.UCultureInfo: Marked Parent property internal, as the behavior differs from .NET and investigation is needed to determine if this is the equivalent operation.

* ICU4N.Tests.Globalization.UCultureInfoTest::TestTwoLetterISOLanguageName(): Although the documentation says that this method should return empty string, the observed behavior is null. Changing to empty string breaks many tests, need to revisit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant