-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Ensures activate.bat can handle Unicode contents #2416
Conversation
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.
Can we add test for this? 🤔
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.
Can you add a changelog entry?
I think we now don't have to change the character code page to 65001 in _get_test_lines function. We can replace this: def _get_test_lines(self, activate_script):
# for BATCH utf-8 support need change the character code page to 650001
return ["@echo off", "", "chcp 65001 1>NUL"] + super()._get_test_lines(activate_script) to this: def _get_test_lines(self, activate_script):
return ["@echo off", ""] + super()._get_test_lines(activate_script) |
Actually, this is wrong. Because a file called |
I think we should use - encoding = "ascii" if IS_WIN else sys.getfilesystemencoding()
+ encoding = sys.getfilesystemencoding() But, when I run tests, it fails in Powershell tests. That can be because of PowerShell file does not handle Unicode contents also. So I mark this pull request as draft until there is a solution to this problem. |
You'll need to come up with a solution for this to land 🤔 |
Closing dut to being stalled, we can reopen if someone picks it up again. |
activate.bat is UTF-8 encoded but uses current console codepage.
This issue was encountered first in venv.
You can see the issue created for the venv module on: python/cpython#76590
And the original pull request for the venv module: python/cpython#5757
I just added some code that applies the original PR that is made for the venv module to solve this issue. The code I added is almost the same as the original commit except added
@
signs.Closes #2414