json
json copied to clipboard
Allow custom base class as node customization point
This PR adds an additional template parameter which allows to set a custom base class for nlohmann::json. This class serves as an extension point and allows to add functionality to json node. Examples for such functionality might be metadata or additional member functions (e.g. visitors) or other application specific code.
By default the parameter is set to void and an empty base class is used. In this case the library behaves as it already did.
Pull request checklist
Read the Contribution Guidelines for detailed information.
- [x] Changes are described in the pull request, or an existing issue is referenced.
- [x] The test suite compiles and runs without error.
- [x] Code coverage is 100%. Test cases can be added by editing the test suite. (should be. will update state aver coverall is done)
- [x] The source code is amalgamated; that is, after making changes to the sources in the
include/nlohmanndirectory, runmake amalgamateto create the single-header filesingle_include/nlohmann/json.hpp. The whole process is described here.
Just saw i used the wrong base branch and fixed it.
Coverage remained the same at 100.0% when pulling 46597f0b9c1b8e9b017c4d5ca7b0d77810e7cbf3 on barcode:allow_custom_base_class_as_node_customization_point into 8fcdbf2e771f481d988cb36847d6af6b17e30a99 on nlohmann:develop.
Not sure about the cppcheck error, but there are also issues with MSVC.
Great! Windows seem to work now. The amalgamation check failed though. Please try again after calling make amalgamate.
For some strange reason make amalgamate did not do anything (make: Nothing to be done for 'amalgamate'.). After deleting the single include and running make amalgamate again, it also did some formatting. Not sure if that was supposed to happen.
The cppcheck test was skipped this time, so it did not fail. I guess it will fail again. Not sure why it is complaining about, but it had the same issue on my old PR
Check code with Cppcheck
Checking /__w/json/json/single_include/nlohmann/json.hpp ...
/__w/json/json/single_include/nlohmann/json.hpp:3448:54: error: syntax error [syntaxError]
class BinaryType = std::vector<std::uint8_t>,
^
The ci uses cppcheck 2.7. This seems to be some version of the current main branch. If i build cppcheck main and use it, i get the same error. It looks like a false positive. I could suppress it using // cppcheck-suppress syntaxError.
For some strange reason
make amalgamatedid not do anything (make: Nothing to be done for 'amalgamate'.). After deleting the single include and runningmake amalgamateagain, it also did some formatting. Not sure if that was supposed to happen.
Unfortunately, this can happen. You fixed it the right way. Sorry for the inconvenience.
The ci uses cppcheck 2.7. This seems to be some version of the current main branch. If i build cppcheck main and use it, i get the same error. It looks like a false positive. I could suppress it using
// cppcheck-suppress syntaxError.
Yes, the CI uses the bleeding-edge version of cppcheck. If you can suppress the issue, then I guess this is the way.
~Finally all checks went green.~ All but the first commit were fixes for the CI and do not carry important information. I think merge+squash would be a good way to keep the commit history in a readable form.
Not sure why it displayed a green tick earlier...
std::make_unique is not available in C++11 - I guess that's why the CI fails.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
@barcode @nlohmann I think this just needs some documentation.
@nlohmann @gregmarr
I have rebased the branch on develop and added some API documentation.
Let me know, if there is anything else stopping this PR from getting merged.
@nlohmann Please approve the workflow run.
Strange... not sure why the previous run of cppcheck accepted the code without the suppress comment. I guess it is a sporadic error. Hence i added the comment again.
@barcode This needs to be updated for the new type safety namespace introduced in 3.11.0 through 3.11.2.
So what is the Plan on this PR? Is there something still to be done by me? If not, what is the plan for merging this PR? i just want to know, since i will have to rebase #3165 at some point and #3165 requires this PR to work.
Looks like it just needs another review (which I can do either a little later today or over the weekend) and some attention from @nlohmann.
I'll check during the weekend.
Some notes for a follow-up PR:
- Make specifying a base class easier/less verbose.
- Warn users about potential conflicts with newer versions, i.e., base members should avoid generic names to avoid clashes with future member additions to
basic_json. - Doxygen comments should conform to the "new" convention.
- (Run reuse.)
🔴 Amalgamation check failed! 🔴
The source code has not been amalgamated.
Will merge once the CI is green.
🔴 Amalgamation check failed! 🔴
The source code has not been amalgamated.
Thanks a lot!!!
Thanks to you guys, too. I really like this library and know it is a lot of work to maintain libraries.