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

Add CP_UTF8 support to GetConsoleLangId #18565

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Feb 12, 2025

The reasons why the console code page has an effect on the default
thread locale on Windows is not noted anywhere. If I had to take a
guess, I'd say it's related to the history of the han unification.

In any case, CP_UTF8 is not a "code page" and so it should not have
an effect on the thread locale either. This issue goes all the way
back to the introduction of CP_UTF8 in XP, over 20 years ago.

Closes MSFT-56188683

Validation Steps Performed

  • Switch OS display language to Japanese
  • Switch user default codepage to Japanese (932)
  • Run the modded OpenConsole
  • chcp 65001
  • netsh interface ipv4 show interface
    has Japanese table headers ❌

@lhecker lhecker added Product-Conhost For issues in the Console codebase Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. zInbox-Bug Ignore me! labels Feb 12, 2025
@lhecker lhecker requested review from DHowett and miniksa February 12, 2025 12:57
@DHowett
Copy link
Member

DHowett commented Feb 12, 2025

Wow, this one is intricate for how simple the code looks

@miniksa
Copy link
Member

miniksa commented Feb 12, 2025

The only question I really have is "what happens to FormatMessage calls inside a client process on the other end when you return this?"

I fully believe that.... 10 years ago... I didn't think through that it would make sense for UTF-8... which had never really been supported in the past... to return the STATUS_NOT_SUPPORTED. But I'm also wondering whether FormatMessage is now gonna be broken and other less enlightened tools are going to have a really bad time.

@lhecker
Copy link
Member Author

lhecker commented Feb 12, 2025

I'll of course test this properly before we merge this, but in theory when this returns the unsupported status, the per-thread locale will remain undefined. In that case FormatMessage is documented to use the user locale. So theoretically it should just use Japanese or whatever, as expected, and just how it behaves in non-East-Asian locales.

@lhecker lhecker marked this pull request as ready for review February 26, 2025 20:01
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

I just love1 functions that have 6 paragraphs of comments spanning a decade to understand just how load bearing they are

Footnotes

  1. is love the right word?

@lhecker
Copy link
Member Author

lhecker commented Mar 11, 2025

Don't merge this PR yet though. While I do I think it's doing the correct thing, it's not exactly working with netsh yet. (I just started testing it.)

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

you asked for this

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 11, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Mar 18, 2025
@lhecker lhecker reopened this Mar 26, 2025
@lhecker lhecker removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Mar 26, 2025
@lhecker lhecker marked this pull request as draft March 26, 2025 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase zInbox-Bug Ignore me!
Projects
Status: To Consider
Status: To Cherry Pick
Development

Successfully merging this pull request may close these issues.

4 participants