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.

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

bpo-33601: Document for Py_UTF8Mode #7143

Closed
wants to merge 1 commit into from

Conversation

y43ren
Copy link

@y43ren y43ren commented May 27, 2018

Adding documentation for Py_UTF8Mode.

https://bugs.python.org/issue33601

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

Thanks again to your contribution and we look forward to looking at it!

@y43ren y43ren force-pushed the fix-issue-33601 branch from be62928 to 31e197e Compare May 27, 2018 21:40
Enable Cpython utf-8 mode, which ignores the locale settings, and
uses the UTF-8 encoding by default; changes `sys.stdin` and
`sys.stdout` error handlers to ``surrogateescape``. By default,
it is enabled in the POSIX locale.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation is incorrect. Extract of Python/bltinmodule.c:

/* UTF-8 mode (PEP 540): if equals to 1, use the UTF-8 encoding, and change
   stdin and stdout error handler to "surrogateescape". It is equal to
   -1 by default: unknown, will be set by Py_Main() */
int Py_UTF8Mode = -1;

I suggest:

  • If set to 1, enable the UTF-8 Mode
  • If set to 0, disable the UTF-8 Mode
  • If set to -1, look for the :envvar:PYTHONUTF8 environment variable and the :option:-X utf8 command line option

I would suggest to not document here the effects of the UTF-8 Mode.

Currently, the effects of the UTF-8 Mode are not documented. But I would prefer to not document it at 3 different places: C API doc, PYTHONUTF8 doc, -X utf8 doc.

I don't know what is the best place to document the UTF-8 Mode, but I would prefer to document it in a Python documentation (not in the C API).

Set by the :option:`-X` ``utf8`` option and the :envvar:`PYTHONUTF8`
environment variable.

See :pep:`540` for more details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add "the": See the :pep:540 ...

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@vstinner
Copy link
Member

@y43ren: Did you sign the Python CLA? See #7143 (comment)

@vstinner
Copy link
Member

@y43ren: Do you still plan to work on this PR? I don't see any change since I reviewed your first PR version.

@brettcannon
Copy link
Member

Thanks for the PR, but closing as the CLA has not been signed within the last month. If you do decide to sign the CLA we can re-open this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants