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

Remove comments in JSON files for boost 1.59 (ptree removed support) #1689

Closed
theopolis opened this Issue Nov 30, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@theopolis
Contributor

theopolis commented Nov 30, 2015

Comments in JSON is non-standard but were still parsed by boost's property tree library. In boost 1.59 this non-standard support is removed. Homebrew already includes 1.59, so there's a little haste around removing the comments we have.

@andrew-d

This comment has been minimized.

andrew-d commented Dec 2, 2015

I will note that having comments to explain what a query does, why it's there, etc. is super-useful. My vote is actually for keeping these, possibly by including a preprocessor that strips them on load? This library does a cool thing which replaces comments with whitespace, so the error locations (if any) are still correct.

@andrew-d

This comment has been minimized.

andrew-d commented Dec 2, 2015

I ported over the code from the above library to C++ pretty quickly today - see it here: https://github.com/andrew-d/json-strip

I'm pretty busy for the rest of the week, though, so I'm not sure I'll have the time to get to actually putting this in a PR - if anyone else does before me, let me know. It probably also needs tests - the code was written pretty quickly.

@marpaia

This comment has been minimized.

Contributor

marpaia commented Dec 2, 2015

@theopolis

This comment has been minimized.

Contributor

theopolis commented Dec 2, 2015

Although we assume config instantiation and parsing is a rare and potentially expensive event-- if we "only support" hash-style comments then we can greatly simplify the comment stripping logic. It essentially becomes a strip + character compare.

@marpaia

This comment has been minimized.

Contributor

marpaia commented Dec 2, 2015

If we want comments for the sake of comments, sure, we can just support # comments.

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