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

Macro for localeconv call. #202

Closed
Casperinous opened this issue Oct 7, 2017 · 4 comments
Closed

Macro for localeconv call. #202

Casperinous opened this issue Oct 7, 2017 · 4 comments

Comments

@Casperinous
Copy link
Contributor

Casperinous commented Oct 7, 2017

During the compilation of your library for a native application in android the following error was produced: error: undefined reference to 'localeconv'. After googling, it came up to be a common error produced by the NDKs. A solution utilizing macros was proposed to another json-related library having the same issue. I forked your project and implement the proposed solution, resulting in a successful compilation of the code.
I can make a PR but i would like to know if you prefer another name for the macro, or in general a different solution.
Current name of the macro is NO_LOCALE_SUPPORT.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 7, 2017

Yes, please make a PR to the master branch. Call the macro ENABLE_LOCALES and, if you don't mind, also add a CMake option of the same name that defaults to on.

Also: Note that get_decimal_point should return '.', not '\0' if no locales are used.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 7, 2017

I will probably make a new minor release that also includes backports of #49 and #184 from develop.

Casperinous added a commit to Casperinous/cJSON that referenced this issue Oct 8, 2017
… the localeconv method in some SDK's.

A macro named `ENABLE_LOCALES` was added and an option with the same name too in the CMakeLists.txt
@Casperinous
Copy link
Contributor Author

Also, one question regarding the CONTRIBUTION.md:
I am a bit confused there, I dug up in the discussion and checked the code and it looks like 4 spaces per indentation.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 8, 2017

Yes, 4 Spaces. This is a bug. Interesting that nobody noticed this so far, including me.

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

No branches or pull requests

2 participants