Skip to content

mb_detect_encoding recognizes all letters in Hungarian alphabet #8629

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

Merged
merged 1 commit into from
May 25, 2022

Conversation

alexdowad
Copy link
Contributor

As requested in #8439.

@cmb69 @mvorisek @icetee @ramsey

@mvorisek You mentioned that historically, Czech has used Windows-1250 encoding. I have no objection to adding support for Windows-1250 to mbstring, if it is likely that Eastern European PHP users will actually use it. If nobody is likely to use it, it will be better to leave it out.

@mvorisek
Copy link
Contributor

@mvorisek You mentioned that historically, Czech has used Windows-1250 encoding. I have no objection to adding support for Windows-1250 to mbstring, if it is likely that Eastern European PHP users will actually use it. If nobody is likely to use it, it will be better to leave it out.

In some legacy Windows applications, it is still used in the Czech Republic, but I personally do not need it.

@alexdowad
Copy link
Contributor Author

I guess I can take @cmb69's "thumbs up" as approval? 😄

@alexdowad alexdowad merged commit 8b70a7d into php:master May 25, 2022
@alexdowad alexdowad deleted the hungary branch May 25, 2022 11:11
@cmb69
Copy link
Member

cmb69 commented May 25, 2022

My thumbs-up was primarily regarding your stance on Windows 1250 support, especially to add it, if it's actually needed. But anyhow, it was fine to merge this PR. :)

@ramsey
Copy link
Member

ramsey commented May 25, 2022

This says it was merged directly into master, so how did 58d0aad get into PHP-8.1? Did you cherry-pick it there after merging to this to master?

@alexdowad
Copy link
Contributor Author

This says it was merged directly into master, so how did 58d0aad get into PHP-8.1? Did you cherry-pick it there after merging to this to master?

No, I merged into PHP-8.1 first and then merged PHP-8.1 down into master.

The message displayed by GitHub is not correct.

@cmb69
Copy link
Member

cmb69 commented May 25, 2022

This is due to GH's focus on the default branch (that doesn't quite match our branching strategy).

@ramsey
Copy link
Member

ramsey commented May 25, 2022

Thanks. It was very confusing. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants