Skip to content

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

@StanFromIreland StanFromIreland requested a review from vstinner July 2, 2025 14:53
@StanFromIreland
Copy link
Member Author

@malemburg Anything else left to do here?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@malemburg malemburg merged commit ffd7f2f into python:main Jul 8, 2025
25 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Docs PRs Jul 8, 2025
@malemburg
Copy link
Member

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

Thanks @StanFromIreland and @sobolevn .

@malemburg malemburg 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
)

Closes pythonGH-136162.
(cherry picked from commit ffd7f2f)

Co-authored-by: Stan Ulbrych <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 9, 2025

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
)

Closes pythonGH-136162.
(cherry picked from commit ffd7f2f)

Co-authored-by: Stan Ulbrych <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 9, 2025
@bedevere-app
Copy link

bedevere-app bot commented Jul 9, 2025

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

@bedevere-app bedevere-app bot 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)

Closes GH-136162.
(cherry picked from commit ffd7f2f)

Co-authored-by: Stan Ulbrych <[email protected]>
malemburg pushed a commit that referenced this pull request Jul 9, 2025
…136454)

gh-136162: Document `encodings` package functions (GH-136164)

Closes GH-136162.
(cherry picked from commit ffd7f2f)

Co-authored-by: Stan Ulbrych <[email protected]>
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)

Closes pythonGH-136162.
(cherry picked from commit ffd7f2f)

Co-authored-by: Stan Ulbrych <[email protected]>
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: Done
Development

Successfully merging this pull request may close these issues.

4 participants