-
Notifications
You must be signed in to change notification settings - Fork 6.9k
feat(windows) start powershell in utf-8 mode #7902
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
base: main
Are you sure you want to change the base?
Conversation
e5750bd to
a623f60
Compare
maxj-oai
left a comment
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.
seems reasonable to me
a623f60 to
e67b964
Compare
e2b078c to
2a24d64
Compare
ae69170 to
77f84e0
Compare
ff57428 to
072c7a6
Compare
jif-oai
left a comment
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.
Ok on the principle but a lot of the wiring can be simplified
| const POWERSHELL_FLAGS: &[&str] = &["-nologo", "-noprofile", "-command", "-c"]; | ||
|
|
||
| /// Prefixed command for powershell shell calls to force UTF-8 console output. | ||
| pub(crate) const UTF8_OUTPUT_PREFIX: &str = |
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.
TIL, I would never have guesses that this is sufficient.
|
|
||
| /// Prefixed command for powershell shell calls to force UTF-8 console output. | ||
| pub(crate) const UTF8_OUTPUT_PREFIX: &str = | ||
| "[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;\n"; |
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.
Is it stable for every windows version?
Summary
Adds a FeatureFlag to enforce UTF8 encoding in powershell, particularly Windows Powershell v5. This should help address issues like #7290.
Notably, this PR does not include the ability to parse
apply_patchinvocations within UTF8 shell commands (calls to the freeform tool should not be impacted). I am leaving this out of scope for now. We should address before this feature becomes Stable, but those cases are not the default behavior at this time so we're okay for experimentation phase. We should continue cleaning up theapply_patch::invocationlogic and then can handle it more cleanly.Testing