Repository navigation
fix(core): pin PowerShell output encoding to UTF-8 on Windows - #58968
fake-monkey wants to merge 2 commits into
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
There was a problem hiding this comment.
I couldn't verify this. The bug only happens on Windows, and this sandbox is Linux with no PowerShell installed. I ran the nearby tests and type-checked packages/core with tsc. The public API is unchanged.
Main concerns:
- The approach is disputed. Two people on #56292 (pwsh 7.6.7 on es-* Windows, and Windows 10 zh-CN with CP936) say that assigning
[Console]::OutputEncodinginside the session is ignored or comes too late when the process is spawned like this. Only setting the code page before PowerShell starts worked for them. Your notes say it works on 5.1 and 7.5. Could you post a capture from pwsh 7.6.x run through theshelltool (not an interactive console)? Please includeWrite-Output 中文and a native child such ascmd /c echo 中文. - The new
shell.test.tsassertion fails on Linux and macOS (see inline comment). - Commands that start with
usingbreak with the setup line in front of them (see inline comment).
The new tool-shell test can't catch this bug outside Windows, because pwsh already writes UTF-8 there. It passes with or without the fix, so it can be dropped unless it runs on Windows CI.
Related: #56292
| "Write-Output hi", | ||
| process.platform === "win32" ? `${ShellSelect.PS_UTF8} Write-Output hi` : "Write-Output hi", | ||
| ]) | ||
| expect(ShellSelect.args("powershell.exe", "Write-Output hi")).toEqual([ |
There was a problem hiding this comment.
This fails on Linux and macOS. ShellSelect.name keeps the .exe off Windows, so args("powershell.exe", ...) returns ["-c", "Write-Output hi"] there. I ran bun test test/shell.test.ts and got shell > builds command args per shell family failing. Either drop this assertion or limit it to win32.
| "-NoProfile", | ||
| "-NonInteractive", | ||
| "-Command", | ||
| process.platform === "win32" ? `${PS_UTF8} ${command}` : command, |
There was a problem hiding this comment.
Putting statements in front of the user's command breaks commands that must start the script. using namespace System.Text; [StringBuilder]::new() (and using module/using assembly) becomes a parse error once [Console]::OutputEncoding = ...; comes first. The same problem came up on #54511. Skipping the setup when the command starts with using would avoid it.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Updated:
On pwsh 7.6.x: not verified locally. #56292 says the in-session assignment is ignored there; if that is confirmed, the fallback is to set the code page before PowerShell starts. |
There was a problem hiding this comment.
Both earlier points are fixed. The assertion that failed on Linux is gone, and commands that start with using namespace|module|assembly (after blank or comment lines) now skip the setup line. The test that couldn't catch the bug outside Windows was removed. At this commit, test/shell.test.ts and test/tool-shell.test.ts pass on Linux (73 pass, 42 skip) and tsc is clean.
I still couldn't verify the fix, because it only applies on Windows. Whether the in-session assignment works on pwsh 7.6.x (the version in #56292) needs a check on a Windows machine before merging. The public API is unchanged.
Issue for this PR
Closes #56292
Type of change
What does this PR do?
On Windows,
shelldecodes command output as UTF-8, but PowerShell encodes stdout with the console code page. Commands are spawned withwindowsHide, so there is no console and PowerShell falls back to the system ANSI code page (936 on zh-CN Windows). Every non-ASCII character comes back as U+FFFD.This PR pins the encoding before the command runs:
Assigning
OutputEncodingalso sets the console code page, so native children (git,cmd /c ...) emit UTF-8 too.chcpalone is not enough here because PowerShell's own writer keeps the code page it started with.using namespace/using module/using assemblymust be the first statement in a script, so commands that start with one skip the prefix.For the PTY path the same setup is passed as
-NoExit -Commandwhen the caller does not provide explicit args.How did you verify your code works?
Windows 11 25H2 (build 26200), system ANSI/OEM code page 936. The harness has the same shape as the shell tool:
ShellSelect.args(...)+spawnSync(..., { windowsHide: true }).Write-Output 中文/Write-Host 中文/cmd /c echo 中文PTY: ConPTY through
@lydell/node-ptywith-NoExit -Command <prefix>; both versions renderWrite-Output PTY-中文correctly.Runs on the final version of the prefix (no
chcp):bun test test/tool-shell.test.ts— 46 pass, 63 skip, 0 failbun test test/shell.test.ts -t "builds command args"— passbun typecheckinpackages/core— cleanNot verified on pwsh 7.6.x. #56292 reports that the in-session assignment is ignored there; if that is confirmed, the fallback is to set the code page before PowerShell starts, which needs a launcher. This PR keeps the smaller change until that is confirmed.
Screenshots / recordings
N/A
Checklist
Added by the opencode bot (AI-generated).
Related: #56292