-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
C#: Always decode dotnet
output as UTF-8
#74065
Conversation
modules/mono/editor/GodotTools/GodotTools/Build/DotNetFinder.cs
Outdated
Show resolved
Hide resolved
e9f8f3c
to
16eb4fd
Compare
Since the issues seem to be Windows only, I made the encoding changes only when running on windows. I did a bunch more source diving and found that the output of After having spent way too much time on this and still not understanding the whole system:
|
It looks like you found the biggest source: https://github.com/dotnet/sdk/blob/af67718a9fce6ba01796ed06df44299fe17da67b/src/Cli/Microsoft.DotNet.Cli.Utils/UILanguageOverride.cs#L27 but yes,
|
16eb4fd
to
f2b6bbd
Compare
f2b6bbd
to
e0efa3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. TIWAGOS as I'm not a .NET maintainer, but I don't see any blocker for merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks reasonable to me too, so I'm also in favor of merging.
Thanks! |
Cherry-picked for 4.0.4. |
Even tho this seems like a straight forward fix and testing shows that this appears correct, but console handling on Windows is complicated and as far as I can tell this shouldn't work as is, so I am not sure what I am missing here and if this can cause issues in some cases.
SPOILER: One quick run through the Windows console complications
dotnet
always writes UTF8 output, and thus Godot decodes things incorrectly (unless the system default encoding also is UTF8)If anyone has an idea what causes the
dotnet
output to be UTF-8 encoded, that would be great to know, so that we can ensure that this doesn't cause any regressions anywhere.The upstream change is not relevant for the issues, as it it only include in the .NET 8 previews.
If not, changing the windows default code page to utf-8 also fixes the issues, both without the fix and any potential regressions with this fix. (also it's probably a good things to do regardless)