erase_at_pointer #1138
erase_at_pointer #1138
Conversation
|
An automated preview of the documentation is available at https://1138.json.prtest2.cppalliance.org/libs/json/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-06 06:22:58 UTC |
|
GCOVR code coverage report https://1138.json.prtest2.cppalliance.org/gcovr/index.html Build time: 2026-01-06 06:35:15 UTC |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1138 +/- ##
===========================================
- Coverage 93.67% 93.59% -0.08%
===========================================
Files 91 91
Lines 9153 9217 +64
===========================================
+ Hits 8574 8627 +53
- Misses 579 590 +11
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
|
grisumbras
left a comment
grisumbras
left a comment
There was a problem hiding this comment.
In addition to other changes, I expect the tests to have the full code coverage of the added lines.
| { | ||
| ec.clear(); | ||
| if(sv.empty()){ | ||
| BOOST_JSON_FAIL(ec, error::syntax); |
There was a problem hiding this comment.
error::syntax is for JSON syntax errors. Arguably, error::missing_slash is fitting (you require at least one pointer token, and a pointer token starts with a /. But there's an interesting question if we want to allow zero tokens, and what to do in that case. For example, we can set the root object to null.
| doc_uses_allocator.cpp | ||
| doc_using_numbers.cpp | ||
| double.cpp | ||
| erase_at_pointer.cpp |
There was a problem hiding this comment.
I don't think this feature warrants a separate test file. Just use pointer.cpp.
| */ | ||
| BOOST_JSON_DECL | ||
| bool | ||
| erase_at_pointer( |
There was a problem hiding this comment.
There should also be a non-throwing overload.
| result = arr.if_contains(index); | ||
| if( !result ) | ||
| { | ||
| BOOST_JSON_FAIL(ec, error::past_the_end); |
There was a problem hiding this comment.
error::past_the_end is for when the current token is the past-the-end token (-). This is just error::not_found.
| #include <boost/json/parse.hpp> | ||
| #include <boost/json/serialize.hpp> | ||
| #include "test_suite.hpp" | ||
| #include <iostream> |
| value json = testValueArray(); | ||
|
|
||
| value target = boost::json::parse(R"( | ||
| [ |
There was a problem hiding this comment.
Please, here and elsewhere indent according to the surrounding code.
| )"; | ||
| return boost::json::parse(raw); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you configure your editor/IDE to not have stray whitespace at the end of lines?
|
|
||
| bool | ||
| value::erase_at_pointer( | ||
| string_view sv, |
There was a problem hiding this comment.
Please, reformat here and elsewhere. The project doesn't do such token alignment.
| auto result = this; | ||
| auto previous_result = this; | ||
|
|
||
| while (true) |
There was a problem hiding this comment.
Looking at this loop, this is pretty much walk_pointer used with find_pointer lambdas, followed by element erasure, if the search was successful. The only difference is that you need to store the parent of the discovered element, which can be achieved with lambdas with captures.
So, why not do that instead of duplicating walk_pointer?
There was a problem hiding this comment.
I will check that part of the code, thank you
3 participants
Hopefully a more clear and decent version of #1071