Skip to content

gh-136162: Document encodings #136164

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Jul 1, 2025

search_function has possibly the longest single if statement in the entirety of CPython...

we should add a note that these functions should normally not be used directly

I added a .. note::, but switching to a .. warning:: seems like a good idea.

I am split on documenting win32_code_page_search_function, but I have a patch ready. To me it seems as more of a helper function, which could be made private.


📚 Documentation preview 📚: https://cpython-previews--136164.org.readthedocs.build/en/136164/library/codecs.html#module-encodings

@sobolevn
Copy link
Member

sobolevn commented Jul 1, 2025

@StanFromIreland please, coordinate your work with others :)
I also had a PR for this, but I wanted to discuss it first.

@StanFromIreland
Copy link
Member Author

@sobolevn Oops... I must have missed it in your post. I believed that no one else had started work on it so I went ahead. I didn't see value in saying I was working on it if I am to open PR ten minutes later. Apologies!

@sobolevn sobolevn reopened this Jul 1, 2025
@sobolevn
Copy link
Member

sobolevn commented Jul 1, 2025

We can reopen it, since yours was the first one :)
Just something to keep in mind.

@malemburg
Copy link
Member

Looks good, but you should also document the win32_code_page_search_function() function available on Windows for completeness.

@StanFromIreland StanFromIreland requested a review from sobolevn July 1, 2025 11:26
Copy link
Member

@malemburg malemburg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @StanFromIreland

@StanFromIreland
Copy link
Member Author

@malemburg thanks for the well commented code, which is the basis for my doc:-)

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

Successfully merging this pull request may close these issues.

3 participants