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

Jump to bottom

Honor user setting overrides for current culture when using ICU on Windows #38372

Merged
merged 10 commits into from
Jul 3, 2020
Merged

Honor user setting overrides for current culture when using ICU on Windows #38372

merged 10 commits into from
Jul 3, 2020

Conversation

safern
Copy link
Member

@safern safern commented Jun 25, 2020

Fixes: #37121

Adding support to read user overrides from the OS when using ICU on Windows when the locale information we're getting is the user's default locale.

@tarekgh the only way I could think we could test this would be by modifying the registry key and then clearing the cache data, do you think we should do that?

@safern safern requested review from jkotas and tarekgh June 25, 2020 03:08
@ghost
Copy link

ghost commented Jun 25, 2020

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

@safern safern mentioned this pull request Jun 25, 2020
Closed
@jaredpar jaredpar mentioned this pull request Jul 1, 2020
Closed
danmoseley
danmoseley reviewed Jul 2, 2020
View reviewed changes
src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Unix.cs
private void InitUserOverride(bool useUserOverride)
{
// Unix doesn't support user overrides
_bUseOverrides = false;
Copy link
Member

@danmoseley danmoseley Jul 2, 2020 •
edited

Choose a reason for hiding this comment

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

Aside (not this PR) it would be nice at some point to remove all the "Hungarian notation" in CultureData

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree, that'd be a nice cleanup.

Copy link
Member

Choose a reason for hiding this comment

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

Hungarian notations is actually to match Windows exposed API's fields. I am not pushing again the clean up but wanted to tell why we had it in the first place.

tarekgh
tarekgh approved these changes Jul 2, 2020
View reviewed changes
Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

Thanks @safern for getting this fixed

@safern safern merged commit 3cd9172 into dotnet:master Jul 3, 2020
@safern safern deleted the HonorUserSettingsCurrentCulture branch July 3, 2020 03:57
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danmoseley danmoseley danmoseley left review comments

@jkotas jkotas jkotas left review comments

@sba923 sba923 sba923 left review comments

@iSazonov iSazonov iSazonov left review comments

@tarekgh tarekgh tarekgh approved these changes

Assignees
No one assigned
Labels
area-System.Globalization
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable Globalization to read the user setting overrides for current cultures when using ICU
7 participants
@safern @danmoseley @jkotas @tarekgh @sba923 @iSazonov @Dotnet-GitSync-Bot