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

[browser][non-icu] HybridGlobalization change case #84019

Merged
merged 25 commits into from Apr 4, 2023

Conversation

ilonatommy
Copy link
Member

Implements a chunk of web-api based globalization. Is a part of HybridGlobalization feature and contributes to #79989.

Old, icu-based private API: GlobalizationNative_ChangeCase

New, non-icu private API: ChangeCaseInvariantJS, ChangeCaseJS.

Affected public API (see: tests in TextInfoTests.cs):

  • TextInfo.ToLower - no changes in behavior,
  • TextInfo.ToUpper - no changes in behavior,
  • TextInfo.ToTitleCase - no changes in behavior.

@ghost
Copy link

ghost commented Mar 28, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

Implements a chunk of web-api based globalization. Is a part of HybridGlobalization feature and contributes to #79989.

Old, icu-based private API: GlobalizationNative_ChangeCase

New, non-icu private API: ChangeCaseInvariantJS, ChangeCaseJS.

Affected public API (see: tests in TextInfoTests.cs):

  • TextInfo.ToLower - no changes in behavior,
  • TextInfo.ToUpper - no changes in behavior,
  • TextInfo.ToTitleCase - no changes in behavior.
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization

Milestone: -

@ilonatommy ilonatommy marked this pull request as draft March 28, 2023 13:55
Copy link
Contributor

@kg kg left a comment

Choose a reason for hiding this comment

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

Looks good aside from comments

@ilonatommy
Copy link
Member Author

I am merging but if there are any new ideas later, feel free to let me know and we will solve them in a follow-up, cc @pavelsavara, @radical.

@ilonatommy ilonatommy merged commit 9afb7d4 into dotnet:main Apr 4, 2023
171 of 175 checks passed

### WASM

For WebAssembly, both on Browser and WASI, we are using Web API instead of some ICU data.
Copy link
Member

Choose a reason for hiding this comment

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

There is no Web API in WASI. It probably means that the whole Hybrid Globalization can't be implemented there.

@@ -13,7 +13,7 @@ internal static partial class GlobalizationMode
private static partial class Settings
{
internal static bool Invariant { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.Invariant", "DOTNET_SYSTEM_GLOBALIZATION_INVARIANT");
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS || TARGET_BROWSER || TARGET_WASI
Copy link
Member

Choose a reason for hiding this comment

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

no WASI ?

@@ -23,7 +23,7 @@ private static partial class Settings
// This allows for the whole Settings nested class to be trimmed when Invariant=true, and allows for the Settings
// static cctor (on Unix) to be preserved when Invariant=false.
internal static bool Invariant => Settings.Invariant;
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS || TARGET_BROWSER || TARGET_WASI
Copy link
Member

Choose a reason for hiding this comment

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

no WASI ?

@dotnet dotnet locked as resolved and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants