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

Make value types replacable (by providing type traits) #52

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kazuho
Copy link
Owner

@kazuho kazuho commented Oct 22, 2014

Until now, picojson has fixed the types used to represent various types of JSON (e.g. number, string, boolean, ...). But there has been a constant request to replace how the numbers are represented (which has been handled to some extent by adding the PICOJSON_USE_INT64 flag).

The ideal way to answer such requests is to templatize the value type so that the types used for storing the JSON values via type traits (using #ifdefs to switch between the types is not an answer, since it would cause compile or link-time errors when more than one module uses picojson with different set of types).

This PR does that. It implements a type trait (only for numbers at the moment), so that users can define their own set of traits and use the templatized value type (value_t<TraitsT>).

I am going to merge this branch to master if I anybody actually starts using the feature (and if he/she provides the source code that can be used for testing the feature).

@kazuho
Copy link
Owner Author

kazuho commented Oct 22, 2014

This PR has been written as a response to #51.

@jamesjer
Copy link

I do not know why Christopher Jefferson never spoke up, but he did indeed use this code in a forked version of picojson.h that is now included in the GAP package "json": https://gap-packages.github.io/json/. I am looking at adding that to the collection of GAP packages in the Fedora Linux distribution, but would much prefer to use the official released version of picojson.h. Is there any chance that this could be merged? Thank you.

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