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

[nlohmann-json] Add port feature to control implicit conversions behaviour #22408

Closed
thomasgt opened this issue Jan 7, 2022 · 0 comments · Fixed by #22409
Closed

[nlohmann-json] Add port feature to control implicit conversions behaviour #22408

thomasgt opened this issue Jan 7, 2022 · 0 comments · Fixed by #22409
Assignees
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist

Comments

@thomasgt
Copy link
Contributor

thomasgt commented Jan 7, 2022


Is your feature request related to a problem? Please describe.
The nlohmann json library is gradually backtracking on providing implicit conversions to their types via operator T(). They've added a CMake option (JSON_ImplicitConversions) to control this behaviour. I think we should expose this option as a feature on the nlohmann-json port.

For context, here are some relevant issues and PRs in the nlohmann json project:

Proposed solution
Add a new port version that exposes this option. The library currently defaults to enabling implicit conversions, but I wouldn't be surprised if they change the default in the future. I think we should ensure the default behaviour in vcpkg matches that of the library, so perhaps it's best to frame it as an inverted feature (e.g. "disable-implicit-conversions") and leave it out of the port's default features. When the library changes their default from JSON_ImplicitConversions=ON to JSON_ImplicitConversions=OFF, we should add disable-implicit-conversions to the default features for the port.

Describe alternatives you've considered
Alternatively, we could make a non-inverted feature (e.g. implicit-conversions or enable-implicit-conversions), include it in the default features now, and then remove it from the default features when/if the library changes its defaults.

@LilyWangLL LilyWangLL added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants