-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
lib: Attempted to fix issue #57780, child_process.js #57956
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
{ | ||
"dependencies": { | ||
"iconv-lite": "^0.6.3" | ||
} | ||
} |
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.
As you can imagine, we're not going to want to have this file
} else { | ||
// macOS: Test UTF-8 in bash |
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.
There are many more OSes which are neither Windows nor macOS
// Enhanced ChildProcess class with Windows encoding support | ||
class ChildProcess extends OriginalChildProcess { |
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.
Introducing a class just to "highjack" spawnings of cmd.exe
on Windows doesn't feel right – it's unclear whether we'd want to fix #57780 in the first place tbh
Fixes: #57780