virtualenv icon indicating copy to clipboard operation
virtualenv copied to clipboard

Ensures activate.bat can handle Unicode contents

Open OmerFI opened this issue 3 years ago • 5 comments

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: https://github.com/python/cpython/issues/76590 And the original pull request for the venv module: https://github.com/python/cpython/pull/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

OmerFI avatar Sep 10 '22 22:09 OmerFI

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)

OmerFI avatar Sep 20 '22 19:09 OmerFI

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 script.bat is created during the test and this file also should handle Unicode contents.

OmerFI avatar Sep 27 '22 12:09 OmerFI

I think we should use utf-8 in windows not ascii to test this Unicode support, so I made these changes in the last commit:

- 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.

OmerFI avatar Sep 27 '22 12:09 OmerFI

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 🤔

gaborbernat avatar Sep 28 '22 18:09 gaborbernat

Closing dut to being stalled, we can reopen if someone picks it up again.

gaborbernat avatar Oct 25 '22 05:10 gaborbernat