search

nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
41.46k stars 6.59k forks source link

feat: Rebase `feature/optional` to `develop` #4036

Open fsandhei opened 1 year ago

fsandhei commented 1 year ago

Rebasing feature/optional in attempt to revive this PR, referring to https://github.com/nlohmann/json/pull/2117.

Sorry for long time between responses.

I may have been going forward with this the "wrong" way, so I apologize in advance.

Please let me know if this should be handled differently.

TODO

There is one test that is failing: Conversions from a default initialized nlohmann::json to std::optional<T> seems to fail. In the test case below, the last assertion fails with an exception. This is tested on Arch Linux with GCC 13.1.1.

I was not able to figure out how to fix this so I need some assistance here.

/home/fredrik/dev/github/cpp/fredriksd/json/tests/src/unit-conversions.cpp:1573:
TEST CASE:  std::optional
  null

/home/fredrik/dev/github/cpp/fredriksd/json/tests/src/unit-conversions.cpp:1573: ERROR: test case THREW exception: exception thrown in subcase - will translate later when the whole test case has been exited (cannot translate while there is an active exception)

===============================================================================
/home/fredrik/dev/github/cpp/fredriksd/json/tests/src/unit-conversions.cpp:1573:
TEST CASE:  std::optional

DEEPEST SUBCASE STACK REACHED (DIFFERENT FROM THE CURRENT ONE):
  null

/home/fredrik/dev/github/cpp/fredriksd/json/tests/src/unit-conversions.cpp:1573: ERROR: test case THREW exception: [json.exception.type_error.302] type must be string, but is null
fsandhei commented 1 year ago

Changed the base branch to develop.

I'm sorry if I'm making it a bit difficult here; I forked the repository and rebased feature/optional to develop and created this PR against nlohmann:feature/develop but I noticed that included 590 commits (which makes sense) but it cluttered the actual change here, so I instead changed it towards nlohmann:develop.

lightyear15 commented 4 months ago

I am also interested in having std::optional support landing in future versions of this library. I see there was a lot of discussion on how to properly support and convert c++ optional fields vs json nullable fields. Is there a final decision on the matter? How can we revive this PR and push forward support of std::optional fields? @nlohmann

coveralls commented 3 months ago

Coverage Status

coverage: 100.0%. remained the same when pulling 1a8d427a3d8187131cd3d93a7db3299ce6e187e5 on fsandhei:feature/optional into 199dea11b17c533721b26249e2dcaee6ca1d51d3 on nlohmann:develop.

fsandhei commented 3 months ago

See that clang-tidy is failing on some checks. Apparently this is on develop, too. I see that it is related to a supposed new check from clang-tidy v19: https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-designated-initializers.html.

nlohmann commented 3 months ago

See that clang-tidy is failing on some checks. Apparently this is on develop, too. I see that it is related to a supposed new check from clang-tidy v19: clang.llvm.org/extra/clang-tidy/checks/modernize/use-designated-initializers.html.

I'm on it. May take some more time.

lightyear15 commented 2 months ago

has this PR stalled again?

fsandhei commented 2 months ago

I'm currently blocked by https://github.com/nlohmann/json/pull/4311.