Skip to content

Conversation

@StanFromIreland
Copy link
Member

@StanFromIrelandStanFromIreland 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

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

@StanFromIreland
Copy link
MemberAuthor

@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
Copy link
Member

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.

Copy link
Member

@malemburgmalemburg 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
MemberAuthor

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

@StanFromIreland
Copy link
MemberAuthor

@malemburg Anything else left to do here?

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@malemburgmalemburg merged commit ffd7f2f into python:mainJul 8, 2025
25 checks passed
@github-project-automationgithub-project-automationbot moved this from Todo to Done in Docs PRsJul 8, 2025
@malemburg
Copy link
Member

I'm not sure whether we should backport this change. @vstinner ?

Thanks @StanFromIreland and @sobolevn .

@malemburgmalemburg added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 9, 2025
@miss-islington-app
Copy link

Thanks @StanFromIreland for the PR, and @malemburg for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @StanFromIreland for the PR, and @malemburg for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 9, 2025
) ClosespythonGH-136162. (cherry picked from commit ffd7f2f) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@bedevere-app
Copy link

GH-136453 is a backport of this pull request to the 3.13 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 9, 2025
) ClosespythonGH-136162. (cherry picked from commit ffd7f2f) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Jul 9, 2025
@bedevere-app
Copy link

GH-136454 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14 bugs and security fixes label Jul 9, 2025
@malemburg
Copy link
Member

I'll add backports as well. This is documenting existing code, after all.

malemburg pushed a commit that referenced this pull request Jul 9, 2025
…136453) gh-136162: Document `encodings` package functions (GH-136164) ClosesGH-136162. (cherry picked from commit ffd7f2f) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
malemburg pushed a commit that referenced this pull request Jul 9, 2025
…136454) gh-136162: Document `encodings` package functions (GH-136164) ClosesGH-136162. (cherry picked from commit ffd7f2f) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
picnixz pushed a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
…nGH-136164) (python#136454) pythongh-136162: Document `encodings` package functions (pythonGH-136164) ClosespythonGH-136162. (cherry picked from commit ffd7f2f) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docsDocumentation in the Doc dirskip news

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@StanFromIreland@sobolevn@malemburg@vstinner