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

Restore Encoding for the .NET SDK upon Exit #30963

Merged

Conversation

nagilson
Copy link
Member

@nagilson nagilson commented Mar 2, 2023

Fixes #30170

  • The encoding before the SDK is under execution is saved and then restored upon program exit. This will make it so it doesn't affect the behavior of other programs on the console. This behavior was observed before the encoding fixes for other language support, but now that the encoding is actually being applied correctly, it would occur more often.

  • In addition, we have limited the build number to have the encoding UTF 8 fix only on windows builds that officially support UTF 8. Previous builds of Windows 10 2019 that were earlier, even in 2018, did have the support, but the behavior is technically unsupported and thus 'undefined.'

  • Added environment variable to turn on the and force the encoding even if your OS may not support it officially, or if you don't have read registry access and so we can't tell what build of windows you're on. This is 'breaking' in the sense that previously the encoding would work for those without registry access... maybe we don't want to do that? cc @baronfel for a decision.

  • This is a breaking change in that those who relied on the encoding to be changed by the SDK will no longer be able to rely on this.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@baronfel
Copy link
Member

baronfel commented Mar 2, 2023

Added environment variable to turn on the and force the encoding even if your OS may not support it officially, or if you don't have read registry access and so we can't tell what build of windows you're on. This is 'breaking' in the sense that previously the encoding would work for those without registry access... maybe we don't want to do that? cc @baronfel for a decision.

This behavior is fine to me - we just need to make sure that the environment variable is documented.

@nagilson nagilson marked this pull request as ready for review March 2, 2023 21:57
@nagilson nagilson requested a review from a team March 2, 2023 22:25
@nagilson
Copy link
Member Author

nagilson commented Mar 7, 2023

@joeloff Thanks for taking a look, I have responded to all feedback.

@marcpopMSFT marcpopMSFT requested a review from joeloff March 8, 2023 21:35
@nagilson
Copy link
Member Author

nagilson commented Mar 8, 2023

Made a breaking change doc as well. Thanks!

@marcpopMSFT
Copy link
Member

This should have targeted main. Preview 2 is closed and there is no automatic port to main from the preview branches. @nagilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants