GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-33601: Document for Py_UTF8Mode by y43ren · Pull Request #7143 · python/cpython

Conversation

y43ren commented May 27, 2018 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Adding documentation for Py_UTF8Mode.

https://bugs.python.org/issue33601

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

the-knights-who-say-ni commented May 27, 2018

Copy link

Copy Markdown

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

the-knights-who-say-ni added the CLA not signed label

May 27, 2018

bedevere-bot added the awaiting review label

May 27, 2018

y43ren force-pushed the fix-issue-33601 branch from ff42709 to be62928 Compare

May 27, 2018 21:27


          remove .ds_store

31e197e

y43ren force-pushed the fix-issue-33601 branch from be62928 to 31e197e Compare

May 27, 2018 21:40

vstinner requested changes

May 28, 2018

View reviewed changes

Comment thread

Doc/c-api/init.rst

		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.

vstinner May 28, 2018

Copy link

Copy Markdown

Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/c-api/init.rst

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

		See :pep:`540` for more details.

vstinner May 28, 2018

Copy link

Copy Markdown

Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-bot added awaiting changes and removed awaiting review labels

May 28, 2018

bedevere-bot commented May 28, 2018

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner added the skip news label

May 28, 2018

vstinner commented May 28, 2018

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mariatta removed the CLA not signed label

Jul 13, 2018

the-knights-who-say-ni added the CLA not signed label

Jul 13, 2018

vstinner commented Jul 30, 2018

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

brettcannon removed the CLA not signed label

Mar 26, 2019

the-knights-who-say-ni added the CLA not signed label

Mar 26, 2019

brettcannon commented Mar 26, 2019

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

brettcannon closed this

Mar 26, 2019

serhiy-storchaka mentioned this pull request

May 23, 2022

[doc] Py_UTF8Mode is not documented #77782

Closed

Reviewers

vstinner

vstinner requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

y43ren commented May 27, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented May 27, 2018

Uh oh!

vstinner May 28, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner May 28, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented May 28, 2018

Uh oh!

vstinner commented May 28, 2018

Uh oh!

vstinner commented Jul 30, 2018

Uh oh!

brettcannon commented Mar 26, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

y43ren commented May 27, 2018 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.