Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove console encoding support from TextIOWrapper. #91526

Closed
methane opened this issue Apr 14, 2022 · 3 comments · Fixed by #91529
Closed

Remove console encoding support from TextIOWrapper. #91526

methane opened this issue Apr 14, 2022 · 3 comments · Fixed by #91529
Labels
OS-windows topic-IO

Comments

@methane
Copy link
Member

methane commented Apr 14, 2022

Currently, TextIOWrapper.__init__ calls os.device_encoding(file.fileno()) when encoding is not specified and fileno is 0-2 (e.g. open(0)).

However, sys.stdin, stdout, and stderr don't use it even when PYTHONLEGACYWINDOWSSTDIO=1.
config->stdio_encoding is initialized by GetACP(), and create_stdio() passes config->stdio_encoding to TextIOWrapper.

How about removing os.device_encoding(file.fileno()) from TextIOWrapper.__init__()?

  • Thanks to WindowsConsoleIO, most user don't use the console encoding at all.
  • WindowsConsoleIO require UTF-8. So TextIOWrapper(sys.stdout) cause mojibake when sys.stdout is WindowsConsoleIO
  • The only use case of PYTHONLEGACYWINDOWSSTDIO I know is this. But this use case don't create TextIOWrapper for fd=0,1,2.
  • This TextIOWrapper behavior is never documented. It makes TextIOWrapper.__init__ complicated and inconsistent.
  • If we want to use console encoding for stdio when PYTHONLEGACYWINDOWSSTDIO is set, we can fix it in create_stdio(). create_stdio() has special case for WindowsConsoleIO already. (here)

Ping: @vstinner @eryksun @zooba
methane added a commit to methane/cpython that referenced this issue Apr 14, 2022
@methane
pythongh-91526: io: Remove device encoding support from TextIOWrapper
cf94a29 
`TextIOWrapper.__init__` calls `os.device_encoding(file.fileno())` if
fileno is 0-2. But it is very rerely works, and never documented
behavior.
@methane methane linked a pull request Apr 14, 2022 that will close this issue
gh-91526: io: Remove device encoding support from TextIOWrapper #91529
Merged
Copy link
Member

Shouldn't TextIOWrapper take the encoding from the underlying stream if it has a .encoding attribute? That will handle the WindowsConsoleIO case.

What happens if you just call TextIOWrapper(0) (or whichever number is stdout)? I've definitely seen that pattern used occasionally, though I can't remember whether it was meant to be portable or not.

@methane
Copy link
Member Author

methane commented Apr 14, 2022

What file except WindowsConsoleIO has .encoding attribute?

Copy link
Member

I thought it already looked for one, but no, it doesn't.

methane added a commit that referenced this issue Apr 19, 2022
@methane
gh-91526: io: Remove device encoding support from TextIOWrapper (GH-9…
6fdb62b 
…1529)

`TextIOWrapper.__init__()` called `os.device_encoding(file.fileno())` if fileno is 0-2 and encoding=None.
But it is very rarely works, and never documented behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
OS-windows topic-IO
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-91526: io: Remove device encoding support from TextIOWrapper methane/cpython
2 participants
@methane