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

Issue #455 getCurrentLocation() incorrect at end of input #557

Merged
merged 4 commits into from Oct 2, 2019

Conversation

toddobryan
Copy link

@toddobryan toddobryan commented Sep 5, 2019

Fixes the getCurrentLocation() issue (#455) by moving changes to _currInputProcessed,
_currInputRowStart, and _nameStartOffset inside successful branch. Also includes
checks for getTokenLocation() that make sure correct beginning location of tokens is
reported and that getCurrentLocation() correctly reports the end of
stream when reading is finished.

Tests would be much nicer with parameterized tests from Junit5 since it's
hard to tell which test case fails if one does. That's what the commented
out print statements are for. Let me know if you'd like something different,
but I originally had them separated out and forgot to add methods when I
added to the enum.

Also, the idea for the enum and much of the test data is from
https://github.com/leadpony/jsonp-test-suite, which is also Apache 2
licensed. I had to rework the expected results because the JsonP
getLocation() works very differently. Let me know if you need a
CLA from the author of that project and I'll approach him.

… string at end of input

Fixes the getCurrentLocation() issue by moving changes to _currInputProcessed
and _currInputRowStart inside successful branch. Also includes checks for
getTokenLocation() that make sure correct beginning location of tokens is
reported and that getCurrentLocation() correctly reports the end of
stream when reading is finished.

Tests would be much nicer with parameterized tests from Junit5 since it's
hard to tell which test case fails if one does. That's what the commented
out print statements are for. Let me know if you'd like something different,
but I originally had them separated out and forgot to add methods when I
added to the enum.

Also, the idea for the enum and much of the test data is from
https://github.com/leadpony/jsonp-test-suite, which is also Apache 2
licensed. I had to rework the expected results because the JsonP
getLocation() works very differently. Let me know if you need a
CLA from the author of that project and I'll approach him.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants