Skip to content

gh-124665: add _PyCodec_UnregisterError and _codecs._unregister_error - #124677

Merged
vstinner merged 29 commits into
python:mainfrom
picnixz:capi/codecs-unregister-error-124665
Sep 29, 2024
Merged

gh-124665: add _PyCodec_UnregisterError and _codecs._unregister_error#124677
vstinner merged 29 commits into
python:mainfrom
picnixz:capi/codecs-unregister-error-124665

Conversation

@picnixz

@picnixzpicnixz commented Sep 27, 2024•
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot mentioned this pull request Sep 27, 2024
Closed
vstinner
vstinner reviewed Sep 27, 2024
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think that we should start with a public function. I would prefer to start with a private function instead, it's simpler.

Comment threadInclude/codecs.h Outdated
Comment threadModules/_codecsmodule.c Outdated
Comment threadPython/codecs.c Outdated
Comment threadPython/codecs.c Outdated
Comment threadPython/codecs.c Outdated
@picnixz

picnixz commented Sep 28, 2024

Copy link
Copy Markdown
MemberAuthor

@vstinner I followed your advice and only introduced the new array. We'll just need to keep in mind that if we add a new policy, then we should also add it to the array and test it.

@picnixzpicnixz changed the title gh-124665: add PyCodec_UnregisterError and codecs.unregister_errorgh-124665: add _PyCodec_UnregisterError and _codecs._unregister_errorSep 28, 2024
vstinner
vstinner reviewed Sep 28, 2024
View reviewed changes
Comment threadInclude/internal/pycore_codecs.h Outdated
vstinner
vstinner reviewed Sep 28, 2024
View reviewed changes
Comment threadLib/test/test_codeccallbacks.py Outdated
Comment threadPython/codecs.c Outdated
Comment threadLib/test/test_codeccallbacks.py Outdated
Comment threadInclude/internal/pycore_codecs.h Outdated

/*
* Un-register the error handling callback function error under the given
* name. Only non-built-in error handlers can be un-registered.

@vstinnervstinnerSep 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Below you use "custom". Maybe it's better name than "non-built-in". I don't know.

@picnixzpicnixzSep 28, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I reworded the docs. WDYT?

vstinner
vstinner approved these changes Sep 29, 2024
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@vstinner
vstinner merged commit c00964e into python:mainSep 29, 2024
@vstinner

vstinner commented Sep 29, 2024

Copy link
Copy Markdown
Member

Merged, thank you.

@picnixz
picnixz deleted the capi/codecs-unregister-error-124665 branch September 29, 2024 06:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes
@encukouencukouAwaiting requested review from encukou
@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@picnixz@vstinner