Skip to content

gh-68168: Improve the docs for format units starting with 'e' #19610

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Apr 19, 2020

@ZackerySpytz
Copy link
Contributor Author

I believe this is a "skip news" PR.

@@ -191,7 +191,7 @@ which disallows mutable objects such as :class:`bytearray`.
The buffer may contain embedded null bytes. The caller have to call
:c:func:`PyBuffer_Release` when it is done with the buffer.

``es`` (:class:`str`) [const char \*encoding, char \*\*buffer]
``es`` (:class:`str`) [*encoding*, char \*buffer]
Copy link
Member

Choose a reason for hiding this comment

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

Why do you remove "const char*" type on encoding: the type is correct, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that this is following Martin's comment on the issue.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but Martin's comment also says:

The text description should explain what encoding is, but it appears it may already do that well enough.

Copy link
Member

Choose a reason for hiding this comment

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

@erlend-aasland: Do you want to address this issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I can take care of that later today.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the arguments are explained pretty good already:

This format requires two arguments. The first is only used as input, and
must be a :c:expr:const char* which points to the name of an encoding as a
NUL-terminated string, or NULL, in which case 'utf-8' encoding is used.
An exception is raised if the named encoding is not known to Python. The
second argument must be a :c:expr:char**; the value of the pointer it
references will be set to a buffer with the contents of the argument text.
The text will be encoded in the encoding specified by the first argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

But, I find it strange that the type is removed. Passing a NULL argument to a const char * parameter is perfectly allowed in C.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reading again the OP by Larry, I'm not so sure he meant the data types to be removed: #68168 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

See also the description of the table at the top of arg.rst:

In the following description, the quoted form is the format
unit; the entry in (round) parentheses is the Python object type that matches
the format unit; and the entry in [square] brackets is the type of the C
variable(s) whose address should be passed.

@erlend-aasland erlend-aasland changed the title bpo-23980: Improve the docs for format units starting with 'e' gh-68168: Improve the docs for format units starting with 'e' Jan 5, 2024
@erlend-aasland erlend-aasland marked this pull request as draft January 9, 2024 22:03
@erlend-aasland
Copy link
Contributor

This PR needs more work, so I'm marking it as a draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

8 participants