Add support of multi-dim C-style array member of struct. #4262
Conversation
tests/src/unit-inspection.cpp
Outdated
| } | ||
|
|
||
| SECTION("indent=4") | ||
| { | ||
| CHECK(j.dump(4) == | ||
| "{\n \"array\": [\n 1,\n 2,\n 3,\n 4\n ],\n \"boolean\": false,\n \"null\": null,\n \"number\": 42,\n \"object\": {},\n \"string\": \"Hello world\"\n}"); |
There was a problem hiding this comment.
You changed the previous dump output here. Please revert this.
There was a problem hiding this comment.
If the one-line array print is going to be accepted, existing tests for array print checks must also be changed. Honestly I am also afraid that changing default array print formats will incur backward compatibility issues. Maybe we should use a switch to control the one-line array format? By default the switch is off so that existing formats are maintained.
There was a problem hiding this comment.
The thing is: another switch to dump() won't be helping - it already has too many options. We rather need an approach for a second overload of dump() that takes a proper configuration. For now, I would rather split the support for multi-dimensional arrays (this PR) and their formatting (later PR).
There was a problem hiding this comment.
OK. I'll remove the dump formatting stuff from this PR. Thanks for the suggestion.
| @@ -193,7 +193,10 @@ class serializer | |||
| return; | |||
| } | |||
|
|
|||
| if (pretty_print) | |||
| // For multi-dim number arrays, print the inner most dimension in one line to improve readability. | |||
There was a problem hiding this comment.
I think the comment is misleading: you also change the behavior for 1-dimensional arrays.
* Support up to 4 dimensional array.
dd0625f
to
68c0097
Compare
Using docker image tuxmake/clang-nightly:20240109, ci_test_clang will pass. Can we update the CI config to use images from tuxmake/clang-nightly? @nlohmann |
Support up to 4 dimensional array.
Modify pretty-print logic to print multi-dim array's inner most dimension in the same line for better readability.
Refer to discussion #4248 .