Skip to content
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

[no sq.] Iconv fixes and improvements #14464

Merged
merged 3 commits into from Mar 30, 2024
Merged

[no sq.] Iconv fixes and improvements #14464

merged 3 commits into from Mar 30, 2024

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Mar 16, 2024

first: fixes #14463 and also cleaner in general
second: after seeing iconv_open appear in my flamegraph I checked the docs and realized we can reuse it.

To do

This PR is Ready for Review.

How to test

  1. check that it compiles
  2. check that string conversion works (we have an unittest)

@sfan5 sfan5 added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Bugfix 🐛 PRs that fix a bug labels Mar 16, 2024
@JurajVajdaIRobot
Copy link

JurajVajdaIRobot commented Mar 16, 2024

Opens fine for me (OSx)

Before:
5.6.0, 5.7.0, 5.8.0 open up the same (broken) way, 5.6.1 opens up without issue (all downloaded app files)
image

After:
image

NOTE:
opening minetest worked fine for all of them when opening through terminal, e.g. open /Applications/Minetest-5.8.0.app

@sfan5 sfan5 added Code quality and removed Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements labels Mar 17, 2024
@sfan5 sfan5 changed the title Rework iconv encoding detection [no sq.] Iconv fixes and improvements Mar 24, 2024
WCHAR_T doesn't seem as portable as we thought, so it's just easier
to detect the right encoding using macros at this point.
@sfan5 sfan5 merged commit 5df60d8 into minetest:master Mar 30, 2024
13 checks passed
@sfan5 sfan5 deleted the iconv branch March 30, 2024 10:07
@celeron55 celeron55 added the 1 approval rule testing https://gist.github.com/celeron55/bf93a47442e418a629181908f68ffb0f label Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 approval rule testing https://gist.github.com/celeron55/bf93a47442e418a629181908f68ffb0f Bugfix 🐛 PRs that fix a bug Code quality Linux macOS Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid UTF-8 string after upgrade macOS to 14.4
3 participants