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

Add unit test to make sure iterator_input_adapter advances iterators correctly #3548

Merged
merged 1 commit into from
Jul 23, 2022

Conversation

falbrechtskirchinger
Copy link
Contributor

When parsing a string containing two JSON values using iterators, after parsing, iterator_input_adapter should have advanced to the first character of the second value.

The current API doesn't return the first iterator after parsing. To achieve the same effect a proxy_iterator is added that advances the iterator passed by reference, even if the proxy is moved.

@falbrechtskirchinger
Copy link
Contributor Author

@nlohmann @gregmarr This PR is a proactive step to make sure the iterator position after parsing reflects expectations [1]. I hope the parse API can be changed or amended in the future to return the iterator to where parsing stopped. This is in clear conflict with #3442, which aims to change this to point to the last processed character.

[1] To be fair, those are my expectations but I believe they're reasonable.

When parsing a string containing two JSON values using iterators, after
parsing, iterator_input_adapter should have advanced to the first
character of the second value.
Add a unit test to check that this is true.
@coveralls
Copy link

coveralls commented Jun 20, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 3458089 on falbrechtskirchinger:iterator-unit-test into 87cda1d on nlohmann:develop.

@falbrechtskirchinger
Copy link
Contributor Author

@nlohmann Ping. Do you need me to do anything here?

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann self-assigned this Jul 23, 2022
@nlohmann nlohmann added this to the Release 3.11.0 milestone Jul 23, 2022
@nlohmann nlohmann merged commit e91686c into nlohmann:develop Jul 23, 2022
@nlohmann
Copy link
Owner

Thanks for reminding me.

@falbrechtskirchinger falbrechtskirchinger deleted the iterator-unit-test branch July 23, 2022 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants