Skip to content

bpo-25416: add aliases for cp874 and mac_cyrillic encodings #10237

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

Closed
wants to merge 1 commit into from

Conversation

fbidu
Copy link
Contributor

@fbidu fbidu commented Oct 30, 2018

Hi all!

This PR is about issue 25416 that is also closely related with with bpo-18624, both deal with missing aliases in the encondings module.

In this changeset I only tackled the lack of aliases reported on 25416 but I'm still missing tests on it. Reading through 18624, I noted that the file suggested by @bitdancer - test_encodings.py - is missing. The author of 18624's patch added a test on test_codecs.py but that test seems incomplete.

I'm considering add a test that checks if all the aliases are implemented and another one that checks if they are mapped to correct encodings. I'd be glad if a core dev could give me a guidance in how to proceed with that.

Thank you!

https://bugs.python.org/issue25416

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Neither cp874 nor mac_cyrillic are mentioned on bpo-18624.

@@ -439,6 +443,7 @@

# mac_cyrillic codec
'maccyrillic' : 'mac_cyrillic',
'x_mac_cyrillic' : 'mac_cyrillic',
Copy link
Member

Choose a reason for hiding this comment

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

Why x_mac_cyrillic, but not x_mac_greek etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the issue was only about cyrillic. I think that we should implement all the aliases but I'm not sure if I should do it in a PR related to a issue that cited only those two. I can implement the others if you think that's ok to do it in this PR

@fbidu
Copy link
Contributor Author

fbidu commented Nov 2, 2018

Neither cp874 nor mac_cyrillic are mentioned on bpo-18624.

@serhiy-storchaka:
Yes, because this PR is about bpo-25416 I cited 18624 just because I think they are related

@fbidu
Copy link
Contributor Author

fbidu commented Aug 22, 2020

Hi @serhiy-storchaka I just ran into a discussion about encoding and remembered of this PR. Do you think it still makes sense? Thanks

Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 13, 2025
@fbidu fbidu closed this Apr 15, 2025
@fbidu fbidu deleted the bpo-25416 branch April 15, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants