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

Added support for non-ASCII characters (UNIX/Linux) #95

Merged
merged 7 commits into from
Aug 9, 2021

Conversation

uruha-komachin
Copy link
Contributor

Non-ASCII characters are allowed in C++ identifiers so I added support for them.

tbh, I have to admit that we barely see anyone write code with non-ASCII identifiers. Nevertheless, they're still legal C++ code and they compiles. IMO we should add support for this case.

@uruha-komachin
Copy link
Contributor Author

Also, this pr should solve #94 . Please advise if you think there are more necessary changes need to be made. Thank you 😃

@Neargye
Copy link
Owner

Neargye commented Aug 8, 2021

@uruha-komachin could you please add some test too?

There is a [known bug](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67224) in GCC 9's UTF-8 support, therefore we exclude non-ASCII example and test on GCC 9.
Config with `-DMAGIC_ENUM_OPT_ENABLE_NONASCII=OFF` to disable non-ASCII support. (default on)
@uruha-komachin
Copy link
Contributor Author

Hi, I've added an option, MAGIC_ENUM_OPT_ENABLE_NONASCII, and some test cases for the non-ASCII support.

However, as there is a known bug in GCC 9's UTF-8 support, we will skip the non-ASCII example and test for GCC version below 10.0.

As for Windows, the codepage thingy and unicode support on Windows is a PAIN. Therefore I didn't port this feature to Windows and decided to stop there. This feature will need some future help (of course, if anyone wants to implement this).

@uruha-komachin uruha-komachin changed the title Added support for non-ASCII characters Added support for non-ASCII characters (UNIX/Linux) Aug 8, 2021
@uruha-komachin
Copy link
Contributor Author

Sorry, I saw the workflow failed, and it was me forgetting to skip the non-ASCII test in test/CMakeLists.txt. I've updated the file. It should be fine now.

@Neargye
Copy link
Owner

Neargye commented Aug 9, 2021

@uruha-komachin I had the nerve to tweak the style a bit. If everything is ok, I'm ready to merge

@uruha-komachin
Copy link
Contributor Author

@Neargye Everything looks great to me. Please feel free to merge this pr 🎉

@Neargye Neargye self-requested a review August 9, 2021 14:43
@Neargye Neargye merged commit 38f86e4 into Neargye:master Aug 9, 2021
jamek pushed a commit to jamek/magic_enum that referenced this pull request Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants